[Config] Add ArrayNodeDefinition::acceptAndWrap() to list alternative types that should be accepted and wrapped in an array
#51273
Conversation
This file contains hidden or 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
7b1952c to
e6694e6
Compare
This comment was marked as outdated.
This comment was marked as outdated.
maxbeckers
reviewed
Aug 7, 2023
src/Symfony/Component/Config/Definition/Builder/ExprBuilder.php
Outdated
Show resolved
Hide resolved
e6694e6 to
1b84880
Compare
stof
reviewed
Sep 16, 2025
| if (isset($this->normalization)) { | ||
| $node->setNormalizationClosures($this->normalization->before); | ||
| $node->setNormalizedTypes($this->normalization->declaredTypes); | ||
| $node->setNormalizedTypes($this->allowedTypes ?? $this->normalization->declaredTypes); |
Member
There was a problem hiding this comment.
Why is $this->normalization->declaredTypes filled normally ? and why do we need to add this new $this->allowedTypes property instead of using the existing feature (which gets ignored entirely) ?
Member
Author
There was a problem hiding this comment.
I was wondering the same until I (re)discovered #44166, which is based on that: sniffing for allowed types in what I'd call an indirect approach. I'm here proposing a more explicit approach, which wins over the implicit one when used.
Member
There was a problem hiding this comment.
overriding entirely the types inferred from the normalization looks wrong to me (unless we want to deprecate that behavior).
And the suggested API does not play well with nodes supporting multiple normalization rules.
Member
Author
There was a problem hiding this comment.
It has to be this way, see L1532 in the Configuration class for an example: implicit types are loose and too many times we have to fallback to mixed (->always(), ->ifTrue($fn), etc.). Being explicit need to override the implicit collection of possible types - as not all types managed by normalizers have to be accepted at the config builder level (because of the XML loader mainly).
Which means this has to be the way to go.
I renamed the new method ->acceptAndWrap() instead of ->castToArray(). This is both more explicit and more accurate.
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php
Outdated
Show resolved
Hide resolved
1b84880 to
83c0b30
Compare
83c0b30 to
48a78ed
Compare
ArrayNodeDefinition::acceptAndWrap() to list alternative types that should be accepted and wrapped in an array
37153cf to
73b4c37
Compare
Member
Author
|
Thanks for the review @GromNaN - PR updated. |
GromNaN
approved these changes
Sep 22, 2025
| ExprBuilder::TYPE_STRING => is_string(...), | ||
| ExprBuilder::TYPE_BOOL => is_bool(...), | ||
| ExprBuilder::TYPE_NULL => is_null(...), | ||
| ExprBuilder::TYPE_BACKED_ENUM => static fn ($v) => $v instanceof \BackedEnum, |
Member
There was a problem hiding this comment.
That's right, let's keep that for a future improvement.
GromNaN
added a commit
to GromNaN/DoctrineMongoDBBundle
that referenced
this pull request
Sep 22, 2025
Enable more specific types Requires symfony/symfony#51273
fabpot
approved these changes
Sep 23, 2025
Member
|
Thank you @nicolas-grekas. |
Kocal
added a commit
to Kocal/symfony-ux
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
Kocal
added a commit
to Kocal/symfony-ux
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
Kocal
added a commit
to symfony/ux
that referenced
this pull request
Sep 25, 2025
… from Kernel for testing (Kocal) This PR was merged into the 2.x branch. Discussion ---------- Remove explicit configuration `twig.exception_controller` from Kernel for testing | Q | A | -------------- | --- | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md --> | Documentation? | no <!-- required for new features, or documentation updates --> | Issues | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT Related to symfony/symfony#51273 (comment) Fix https://github.com/symfony/ux/actions/runs/17999302594/job/51204828176?pr=3102 Commits ------- f46109a Remove explicit configuration `twig.exception_controller` from Kernel for testing
symfony-splitter
pushed a commit
to symfony/ux-notify
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
symfony-splitter
pushed a commit
to symfony/ux-chartjs
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
symfony-splitter
pushed a commit
to symfony/ux-lazy-image
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
symfony-splitter
pushed a commit
to symfony/ux-map
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
symfony-splitter
pushed a commit
to symfony/ux-vue
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
symfony-splitter
pushed a commit
to symfony/ux-cropperjs
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
symfony-splitter
pushed a commit
to symfony/ux-svelte
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
symfony-splitter
pushed a commit
to symfony/ux-react
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
symfony-splitter
pushed a commit
to symfony/ux-toggle-password
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
symfony-splitter
pushed a commit
to symfony/ux-dropzone
that referenced
this pull request
Sep 25, 2025
… for testing Related to symfony/symfony#51273 (comment)
fabpot
added a commit
that referenced
this pull request
Sep 29, 2025
…nodes that can be enabled or disabled (nicolas-grekas) This PR was merged into the 7.4 branch. Discussion ---------- [Config] Config builders should accept booleans for array nodes that can be enabled or disabled | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Continuing my journey on the config component, after #51273 That could be considered as a bugfix, but it's not worth fixing on lower branches because nobody is using this missing capability - by definition. So: this allows doing eg `->lock(false)` to disable locks, as expected when the node definition has `->treatFalseLike(['enabled' => false])` Note that I didn't turn `->treatNullLike(['enabled' => true])` into accepting `null` as I don't see the point of allowing `->lock(null)` when one can use `->lock(true)`. Commits ------- 633169f [Config] Config builders should accept booleans for array nodes that can be enabled or disabled
This was referenced Oct 27, 2025
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
With #58450, we lost some type info when generating config builders, by preventing #44166 to work correctly.
This PR improves the description of config trees by adding an
ArrayNodeDefinition::acceptAndWrap()method that allows one to declare which alternative types should be accepted at array-node levels. This declaration then triggers the wrapping of such cases into arrays, and also hints the config builder generator so that it can generate more accurate types for configurator methods.