-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: valid rule set, type templates, nested rule set #2
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This method makes it possible to create an initial valid result set for use in populating a form. Having this method makes the following possible: ```php <?php $form = $ruleSet->createValidResultSet(); ?> <!-- In a template: --> <input type="text" name="first" value="<?= $this->e($form->first->value) ?>" /> <?php if (! $form->first->isValid): ?> <p class="text-danger">The value you provided is invalid: <?= $this->e($form->first->message) ?></p> <?php endif ?> ``` To make the tests work, I also needed to provide an `__isset()` implementation for `ResultSet`.
Rounds out remaining functionality of `createValidResultSet()`: - ResultSet MUST NOT contain mappings for any `$valueMap` keys NOT in the rule set - ResultSet MUST use `null` for any OPTIONAL rules with no default value set - ResultSet MUST raise an exception if a key does not exist in the `$valueMap`, maps to a required rule, but has no default value. - Adds `RequiredRuleWithNoDefaultValueException` as the exception type for this - Method MUST create a `ResultSet` of the type provided in `$resultSetClass` if a class was provided. - This required making the `ResultSet` class non-final; all methods were marked final, however. To make this functionality more predictable and useful to developers: - Updated `Result` to add a `@template T` annotation, and assigns `@var T` to the `$value` property. - This allows the following patterns: - Extending `ResultSet` and defining `@property(-read)` mappings: ```php /** * @property-read Result<int> $count * @property-read Result<string> $title */ class CustomResultSet extends ResultSet { } ``` - Providing a template for a returned result set: ```php /** @var ResultSet{count: Result<int>, title: Result<string>} $form */ ```
- Moves the definition of a custom result set class to use with a rule set to a new named constructor, `RuleSet::createWithResultSetClass()`. - This means that for the 80% use case, you will just use the constructor, with or without rules. - When you want to specify a custom result set class to use, use the named constructor. - You can provide Psalm/PHPStan hints when creating a ResultSet or RuleSet ```php /** @var RuleSet<ResultSet> $ruleSet */ $ruleSet = new RuleSet(); /** @var ResultSet{title: string, description: string, createdDate: DateTimeImmutable} $result */ $result = $ruleSet->validate($data); ```
- Makes `RuleSet::$resultSetClass` protected, to simplify extension. - Documents how to provide types for results, result sets, and rule sets via template annotations.
Documents it and provides some examples.
- Removes `final` and `readonly` keywords from `Result`. - All properties are marked readonly - All methods are marked final - Named constructors use `static` when creating new instances, and `@return Result<T>` - constructor is made protected and marked final, to allow Psalm to validate that the named constructors will work with the class constructor - `NestedResult` extends `Result, and adds `__isset()` and `__get()` implementations that check for nested result sets and the properties it defines.
Turns out this was just the beginning of a comprehensive rewrite; will create a new PR with full rewrite. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds a number of features:
ResultSet
to allow extension; this is necessary so that developers can extend and provide@property
or@property-read
annotations mapping keys to specific result types.Result
, allowing the ability to indicate the expected type for the composed$value
.RuleSet
, allowing the ability to indicate the expectedResultSet
class fromvalidate()
.RuleSet
,$resultSetClass
.RuleSet::createWithResultSetClass()
allows creating aRuleSet
that will use the specifiedResultSet
class when creating a new result set during validation.RuleSet::createValidResultSet()
, which produces a valid result set withResult
instances containing the values provided to the method, or the associated default values from the associatedRule
s; this is generally for use with pre-populating forms in order to minimize logic.Result
to remove thefinal
andreadonly
keywordsNestedResult
; this is intended to allow composing aResultSet
as a$value
of aNestedResult
, which then will proxy property calls to the nestedResult
instances of the composedResultSet
.These features allow better extension, improved type inference, and greater flexibility around nested data sets.