[OptionsResolver] Support array of types in allowed type #23112
Conversation
| 3.4.0 | ||
| ----- | ||
|
|
||
| * added array of types support in allowed types |
There was a problem hiding this comment.
Would it not make it more clear to add a short example here? For example:
added array of types support in allowed types (e.g.
int[])
|
What if I have an array of custom items? $resolver->setAllowedType('foo', Foo::class.'[]');
// or
$resolver->setAllowedType('foo', '\App\Bar\Foo[]');This seems a bit annoying to write. |
|
The syntax follows the standard phpdoc notation, which IMO is clear what it means. Even though it can be a bit annoying to write, just looking at the code still reads easily and is clear what it means |
|
This misses the case where the passed value isn't an array, doesn't it? |
|
If the passed value isn't an array, then you'll get an exception saying, e.g, |
|
Might be a good idea to add a test case for that too. |
|
I'd greatly prefer this supported the whole test suite introduces in #17032, instead of a subset of those features. For example, nested values. |
|
The syntax for the nested values just feels a bit weird to me. Looking at it initially took a moment or two to understand exactly what it means. And the further you nest it, the weirder it becomes, E.G |
|
@pierredup I prefer the original phpdoc syntax of |
|
|
|
What about borrowing some inspiration of react proptypes, instead? Im thinking notNull(), oneOf(), oneOfType(), instanceOf() api =/ |
379de0d to
14a0707
Compare
|
Support for nested values added (E.G Status: needs review |
| } | ||
|
|
||
| if (!$invalidTypes) { | ||
| $invalidTypes[] = $this->formatTypeOf($value, null); |
There was a problem hiding this comment.
What about using the types as keys instead and then get rid of the array_unique() keys (replace it with array_keys())?
897081a to
b6ab62e
Compare
b6ab62e to
d066a23
Compare
|
Thank you @pierredup. |
…pe (pierredup) This PR was merged into the 3.4 branch. Discussion ---------- [OptionsResolver] Support array of types in allowed type | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17032, #15524 | License | MIT | Doc PR | TBD This replaces #17032 with a simpler approach to allow an array of types in the allowed types for the options resolver Note: This implementation doesn't support nested values (I.E `int[][]`), but if there is a strong need for it, I'll add it in another PR Commits ------- d066a23 Support array of types in allowed type
This replaces #17032 with a simpler approach to allow an array of types in the allowed types for the options resolver
Note: This implementation doesn't support nested values (I.E
int[][]), but if there is a strong need for it, I'll add it in another PR