Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.8k
Define TreeBuilder default generic type#62616
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.4
Are you sure you want to change the base?
Conversation
Set default template type for TreeBuilder.
Add return type declaration to getConfigTreeBuilder method
| /** | ||
| * Generates the configuration tree builder. | ||
| * | ||
| * @return TreeBuilder<'array'> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@nicolas-grekas unsure about this, is there any case where the getConfigTreeBuilder returns something else as an array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
the interface allows it 🤷
alexander-schranz commentedDec 2, 2025
Fabbot error looks unexpected and not related to the changes. |
Add PHPDoc for getConfigTreeBuilder method
| /** | ||
| * @return TreeBuilder<'array'> | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not sure why psalm wants this be added shouldn't it not just use interface return type here and not force redefinition?
| * This class provides a fluent interface for defining an array node. | ||
| * | ||
| * @template TParent of NodeParentInterface|null | ||
| * @template TParent of NodeParentInterface|null = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not sure about the default here?
alexander-schranz commentedDec 2, 2025
Stuck with psalm and the correct defaults if somebody has any hints to tackle this correctly to make it as easy as possible to create config builders without devs need to think about any generics. |
Uh oh!
There was an error while loading.Please reload this page.
| * This is the entry class for building a config tree. | ||
| * | ||
| * @template T of 'array'|'variable'|'scalar'|'string'|'boolean'|'integer'|'float'|'enum' | ||
| * @template T of 'array'|'variable'|'scalar'|'string'|'boolean'|'integer'|'float'|'enum' = 'array' |
nicolas-grekasDec 2, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this shouldn't be needed, the$type = 'array' default is enough, phpstan understands it.
If other tools don't, that should be reported to them
alexander-schranzDec 2, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
enough for usage ofnew TreeBuilder maybe but not for param type withfunction method(TreeBuilder $treeBuilder) or return typemethod(): TreeBuilder to defaults there to'array'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I ran into the exact problem today and can confirm that PHPStan is indeed reporting an issue:
Method MyBundle\DependencyInjection\Configuration::getConfigTreeBuilder() return type with generic class Symfony\Component\Config\Definition\Builder\TreeBuilder does not specify its types: T
Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>
Removed unnecessary type hint comment for treeBuilder.
Uh oh!
There was an error while loading.Please reload this page.
The generics make sense internal but are very confusing for all people not knowing about internal behaviours and hard to guess whats corrct so we should provide defaults for them. Similar to#61805 add some defaults.
Stumbled over this during upgrade StofDoctrineExtensionBundle: