NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Config] Do not generate unreachable configuration paths#58995

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

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:6.4frombobvandevijver:generated-config-docblock

Jul 26, 2025

Merged

[Config] Do not generate unreachable configuration paths#58995

fabpot merged 1 commit intosymfony:6.4frombobvandevijver:generated-config-docblock

Jul 26, 2025

Conversation

Copy link

Contributor

bobvandevijver commentedNov 26, 2024

Q	A
Branch?	6.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Issues	-
License	MIT

PHPStan was having issues correctly inferring the returned type of a configuration function.

Consider the following messages as example:

Call to an undefined method Symfony\Config\Framework\AnnotationsConfig|Symfony\Config\FrameworkConfig::enabled()

This came from the following generated config class:

/**     * @template TValue     * @param TValue $value     * annotation configuration     * @default {"enabled":false,"cache":"php_array","file_cache_dir":"%kernel.cache_dir%\/annotations","debug":true}     * @return \Symfony\Config\Framework\AnnotationsConfig|$this     * @psalm-return (TValue is array ? \Symfony\Config\Framework\AnnotationsConfig : static)     */publicfunctionannotations(array$value = []):\Symfony\Config\Framework\AnnotationsConfig|static    {if (!\is_array($value)) {$this->_usedProperties['annotations'] =true;$this->annotations =$value;return$this;        }if (!$this->annotationsinstanceof \Symfony\Config\Framework\AnnotationsConfig) {$this->_usedProperties['annotations'] =true;$this->annotations =new \Symfony\Config\Framework\AnnotationsConfig($value);        }elseif (0 <\func_num_args()) {thrownewInvalidConfigurationException('The node created by "annotations()" has already been initialized. You cannot pass values the second time you call annotations().');        }return$this->annotations;    }

When the determined parameter type isarray, only that type can be passed meaning that theis_array is unnecessary. The same holds for the generated docblock: as only an array can be passed, there is no need to define a template and psalm returns.

With the changes in this PR this method is generated more cleanly:

/**     * annotation configuration     * @default {"enabled":false,"cache":"php_array","file_cache_dir":"%kernel.cache_dir%\/annotations","debug":true}    */publicfunctionannotations(array$value = []):\Symfony\Config\Framework\AnnotationsConfig    {if (null ===$this->annotations) {$this->_usedProperties['annotations'] =true;$this->annotations =new \Symfony\Config\Framework\AnnotationsConfig($value);        }elseif (0 <\func_num_args()) {thrownewInvalidConfigurationException('The node created by "annotations()" has already been initialized. You cannot pass values the second time you call annotations().');        }return$this->annotations;    }

A similar issue happens with functions that do accept more than an array value:

Call to an undefined method Symfony\Config\Doctrine\Dbal\TypeConfig|Symfony\Config\Doctrine\DbalConfig::class()

This is caused by the following generated method:

/**     * @template TValue     * @param TValue $value     * @return \Symfony\Config\Doctrine\Dbal\TypeConfig|$this     * @psalm-return (TValue is array ? \Symfony\Config\Doctrine\Dbal\TypeConfig : static)     */publicfunctiontype(string$name,string|array$value = []):\Symfony\Config\Doctrine\Dbal\TypeConfig|static    {if (!\is_array($value)) {$this->_usedProperties['types'] =true;$this->types[$name] =$value;return$this;        }if (!isset($this->types[$name]) || !$this->types[$name]instanceof \Symfony\Config\Doctrine\Dbal\TypeConfig) {$this->_usedProperties['types'] =true;$this->types[$name] =new \Symfony\Config\Doctrine\Dbal\TypeConfig($value);        }elseif (1 <\func_num_args()) {thrownewInvalidConfigurationException('The node created by "type()" has already been initialized. You cannot pass values the second time you call type().');        }return$this->types[$name];    }

While the method seems fine, the@template definition is not correctly defined, seehttps://phpstan.org/r/09317897-4cc8-4f67-98ca-8b6da3671b31.

With the changes in this PR the template is now strictly defined so it matches the function signature:

/**     * @template TValue of string|array     * @param TValue $value     * @return \Symfony\Config\Doctrine\Dbal\TypeConfig|$this     * @psalm-return (TValue is array ? \Symfony\Config\Doctrine\Dbal\TypeConfig : static)     */

Seehttps://phpstan.org/r/986db325-9869-4a6f-8587-6af06c0612d4 for the results.

While the second change might actually be enough to fix the errors, I prefer both fixes as it no longers generates code that can not be executed anyways.

carsonbot added Status: Needs Review Bug labels

Nov 26, 2024

carsonbot added this to the6.4 milestone

Nov 26, 2024

Copy link

Member

stof commentedNov 26, 2024

Tests need to be updated to reflect the new behavior.

Copy link

Member

stof commentedNov 26, 2024

And new tests should probably be added covering the case being fixed for array parameters.

Copy link

ContributorAuthor

bobvandevijver commentedNov 26, 2024

Yeah, I am working on the updated tests 👍🏻

Copy link

ContributorAuthor

bobvandevijver commentedNov 26, 2024

@stof Tests have been added, I added an new testcase which adds a configuration sample for the cases I hit.

nicolas-grekas added the Config label

Nov 27, 2024

carsonbot changed the title~~fix: Do not generate unreachable configuration paths~~[Config] fix: Do not generate unreachable configuration paths

Nov 27, 2024

nicolas-grekas changed the title~~[Config] fix: Do not generate unreachable configuration paths~~[Config] Do not generate unreachable configuration paths

Nov 27, 2024