You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
bug #58995 [Config] Do not generate unreachable configuration paths (bobvandevijver)
This PR was squashed before being merged into the 6.4 branch.
Discussion
----------
[Config] Do not generate unreachable configuration paths
| Q | A
| ------------- | ---
| Branch? | 6.4 <!-- see below -->
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues | - <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead -->
| License | MIT
<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.
Additionally (see https://symfony.com/releases):
- Always add tests and ensure they pass.
- Bug fixes must be submitted against the lowest maintained branch where they apply
(lowest branches are regularly merged to upper ones so they get the fixes too).
- Features and deprecations must be submitted against the latest branch.
- For new features, provide some code snippets to help understand usage.
- Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
- Never break backward compatibility (see https://symfony.com/bc).
-->
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:
```php
/**
* `@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)
*/
public function annotations(array $value = []): \Symfony\Config\Framework\AnnotationsConfig|static
{
if (!\is_array($value)) {
$this->_usedProperties['annotations'] = true;
$this->annotations = $value;
return $this;
}
if (!$this->annotations instanceof \Symfony\Config\Framework\AnnotationsConfig) {
$this->_usedProperties['annotations'] = true;
$this->annotations = new \Symfony\Config\Framework\AnnotationsConfig($value);
} elseif (0 < \func_num_args()) {
throw new InvalidConfigurationException('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 is `array`, only that type can be passed meaning that the `is_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:
```php
/**
* annotation configuration
* `@default` {"enabled":false,"cache":"php_array","file_cache_dir":"%kernel.cache_dir%\/annotations","debug":true}
*/
public function annotations(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()) {
throw new InvalidConfigurationException('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:
```php
/**
* `@template` TValue
* `@param` TValue $value
* `@return` \Symfony\Config\Doctrine\Dbal\TypeConfig|$this
* `@psalm`-return (TValue is array ? \Symfony\Config\Doctrine\Dbal\TypeConfig : static)
*/
public function type(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()) {
throw new InvalidConfigurationException('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, see https://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:
```php
/**
* `@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)
*/
```
See https://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.
Commits
-------
c79c6a2 [Config] Do not generate unreachable configuration paths
0 commit comments