From 75e43f0222ac0b34fdf34e01810ed02a0e4dab8a Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 2 Jul 2026 21:35:09 +0200 Subject: [PATCH 1/2] [TypeDeclaration] Skip constructor in PropertyTypeFromStrictSetterGetterRector setter inference The setter type inferer matched any single-statement `$this->prop = ` method as a setter, including constructors. A constructor such as `$this->listPath = $coreParametersHelper->get(...)` was treated as a setter, so its first param type (CoreParametersHelper) leaked into the inferred property type, producing a bogus `CoreParametersHelper|string` union. Add `hasOnlyFirstParamPropertyAssign()` which additionally requires the assigned value to be exactly the method's first parameter variable (`$this->prop = $param`), and use it in the setter inferer. --- .../constructor_assigned_no_default.php.inc | 51 +++++++++++++++++ .../constructor_non_param_assign.php.inc | 55 +++++++++++++++++++ .../Source/SomeConfigHelper.php | 13 +++++ .../ClassMethodAndPropertyAnalyzer.php | 37 ++++++++++++- ...tterTypeDeclarationPropertyTypeInferer.php | 2 +- 5 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_assigned_no_default.php.inc create mode 100644 rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_non_param_assign.php.inc create mode 100644 rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Source/SomeConfigHelper.php diff --git a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_assigned_no_default.php.inc b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_assigned_no_default.php.inc new file mode 100644 index 00000000000..b4a3b125882 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_assigned_no_default.php.inc @@ -0,0 +1,51 @@ +listPath = $listPath; + } + + public function getListPath(): string + { + return $this->listPath; + } + + public function setListPath(string $path): void + { + $this->listPath = $path; + } +} + +?> +----- +listPath = $listPath; + } + + public function getListPath(): string + { + return $this->listPath; + } + + public function setListPath(string $path): void + { + $this->listPath = $path; + } +} + +?> diff --git a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_non_param_assign.php.inc b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_non_param_assign.php.inc new file mode 100644 index 00000000000..e941ff204b8 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_non_param_assign.php.inc @@ -0,0 +1,55 @@ +listPath = $configHelper->get('path', ''); + } + + public function getListPath(): string + { + return $this->listPath; + } + + public function setListPath(string $path): void + { + $this->listPath = $path; + } +} + +?> +----- +listPath = $configHelper->get('path', ''); + } + + public function getListPath(): string + { + return $this->listPath; + } + + public function setListPath(string $path): void + { + $this->listPath = $path; + } +} + +?> diff --git a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Source/SomeConfigHelper.php b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Source/SomeConfigHelper.php new file mode 100644 index 00000000000..19ab3c23bf4 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Source/SomeConfigHelper.php @@ -0,0 +1,13 @@ +isLocalPropertyVariableAssign($onlyClassMethodStmt, $propertyName); } + /** + * Matches a strict setter shape: single statement `$this->propertyName = $firstParam;`, + * where the assigned value is exactly the method's first parameter variable. + */ + public function hasOnlyFirstParamPropertyAssign(ClassMethod $classMethod, string $propertyName): bool + { + if (! $this->hasOnlyPropertyAssign($classMethod, $propertyName)) { + return false; + } + + $firstParam = $classMethod->params[0] ?? null; + if (! $firstParam instanceof Param) { + return false; + } + + $onlyClassMethodStmt = ((array) $classMethod->stmts)[0]; + if (! $onlyClassMethodStmt instanceof Expression) { + return false; + } + + $assign = $onlyClassMethodStmt->expr; + if (! $assign instanceof Assign) { + return false; + } + + if (! $assign->expr instanceof Variable) { + return false; + } + + return $this->nodeComparator->areNodesEqual($assign->expr, $firstParam->var); + } + public function hasPropertyAssignWithReturnThis(ClassMethod $classMethod): bool { $stmts = (array) $classMethod->stmts; diff --git a/rules/TypeDeclaration/TypeInferer/PropertyTypeInferer/SetterTypeDeclarationPropertyTypeInferer.php b/rules/TypeDeclaration/TypeInferer/PropertyTypeInferer/SetterTypeDeclarationPropertyTypeInferer.php index bc07a55b5d4..b1d2634d003 100644 --- a/rules/TypeDeclaration/TypeInferer/PropertyTypeInferer/SetterTypeDeclarationPropertyTypeInferer.php +++ b/rules/TypeDeclaration/TypeInferer/PropertyTypeInferer/SetterTypeDeclarationPropertyTypeInferer.php @@ -27,7 +27,7 @@ public function inferProperty(Property $property, Class_ $class): ?Type $propertyName = $this->nodeNameResolver->getName($property); foreach ($class->getMethods() as $classMethod) { - if (! $this->classMethodAndPropertyAnalyzer->hasOnlyPropertyAssign($classMethod, $propertyName)) { + if (! $this->classMethodAndPropertyAnalyzer->hasOnlyFirstParamPropertyAssign($classMethod, $propertyName)) { continue; } From 04bcf2f973307b3570206e9430852e61bd236a45 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 2 Jul 2026 21:49:37 +0200 Subject: [PATCH 2/2] Skip constructor-assigned property instead of guarding setter inference Simpler, more conservative: if a property is assigned anywhere in the constructor, its runtime type may differ from the setter/getter, so skip it entirely rather than inferring a type. Reverts the setter-inferer/analyzer change in favor of an isAssignedInConstructor() guard in the rule. --- .../constructor_assigned_no_default.php.inc | 51 ----------------- .../constructor_non_param_assign.php.inc | 55 ------------------- .../skip_assigned_in_constructor.php.inc | 23 ++++++++ ...kip_nested_assigned_in_constructor.php.inc | 25 +++++++++ .../Source/SomeConfigHelper.php | 13 ----- .../ClassMethodAndPropertyAnalyzer.php | 37 +------------ ...opertyTypeFromStrictSetterGetterRector.php | 40 +++++++++++++- ...tterTypeDeclarationPropertyTypeInferer.php | 2 +- 8 files changed, 89 insertions(+), 157 deletions(-) delete mode 100644 rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_assigned_no_default.php.inc delete mode 100644 rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_non_param_assign.php.inc create mode 100644 rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/skip_assigned_in_constructor.php.inc create mode 100644 rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/skip_nested_assigned_in_constructor.php.inc delete mode 100644 rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Source/SomeConfigHelper.php diff --git a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_assigned_no_default.php.inc b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_assigned_no_default.php.inc deleted file mode 100644 index b4a3b125882..00000000000 --- a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_assigned_no_default.php.inc +++ /dev/null @@ -1,51 +0,0 @@ -listPath = $listPath; - } - - public function getListPath(): string - { - return $this->listPath; - } - - public function setListPath(string $path): void - { - $this->listPath = $path; - } -} - -?> ------ -listPath = $listPath; - } - - public function getListPath(): string - { - return $this->listPath; - } - - public function setListPath(string $path): void - { - $this->listPath = $path; - } -} - -?> diff --git a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_non_param_assign.php.inc b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_non_param_assign.php.inc deleted file mode 100644 index e941ff204b8..00000000000 --- a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/constructor_non_param_assign.php.inc +++ /dev/null @@ -1,55 +0,0 @@ -listPath = $configHelper->get('path', ''); - } - - public function getListPath(): string - { - return $this->listPath; - } - - public function setListPath(string $path): void - { - $this->listPath = $path; - } -} - -?> ------ -listPath = $configHelper->get('path', ''); - } - - public function getListPath(): string - { - return $this->listPath; - } - - public function setListPath(string $path): void - { - $this->listPath = $path; - } -} - -?> diff --git a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/skip_assigned_in_constructor.php.inc b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/skip_assigned_in_constructor.php.inc new file mode 100644 index 00000000000..f2d3d675d8a --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/skip_assigned_in_constructor.php.inc @@ -0,0 +1,23 @@ +listPath = $listPath; + } + + public function getListPath(): string + { + return $this->listPath; + } + + public function setListPath(string $path): void + { + $this->listPath = $path; + } +} diff --git a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/skip_nested_assigned_in_constructor.php.inc b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/skip_nested_assigned_in_constructor.php.inc new file mode 100644 index 00000000000..d54b4057cef --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Fixture/skip_nested_assigned_in_constructor.php.inc @@ -0,0 +1,25 @@ +listPath = $listPath; + } + } + + public function getListPath(): string + { + return $this->listPath; + } + + public function setListPath(string $path): void + { + $this->listPath = $path; + } +} diff --git a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Source/SomeConfigHelper.php b/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Source/SomeConfigHelper.php deleted file mode 100644 index 19ab3c23bf4..00000000000 --- a/rules-tests/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector/Source/SomeConfigHelper.php +++ /dev/null @@ -1,13 +0,0 @@ -isLocalPropertyVariableAssign($onlyClassMethodStmt, $propertyName); } - /** - * Matches a strict setter shape: single statement `$this->propertyName = $firstParam;`, - * where the assigned value is exactly the method's first parameter variable. - */ - public function hasOnlyFirstParamPropertyAssign(ClassMethod $classMethod, string $propertyName): bool - { - if (! $this->hasOnlyPropertyAssign($classMethod, $propertyName)) { - return false; - } - - $firstParam = $classMethod->params[0] ?? null; - if (! $firstParam instanceof Param) { - return false; - } - - $onlyClassMethodStmt = ((array) $classMethod->stmts)[0]; - if (! $onlyClassMethodStmt instanceof Expression) { - return false; - } - - $assign = $onlyClassMethodStmt->expr; - if (! $assign instanceof Assign) { - return false; - } - - if (! $assign->expr instanceof Variable) { - return false; - } - - return $this->nodeComparator->areNodesEqual($assign->expr, $firstParam->var); - } - public function hasPropertyAssignWithReturnThis(ClassMethod $classMethod): bool { $stmts = (array) $classMethod->stmts; diff --git a/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php b/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php index e27356c026e..9e3e68b56c8 100644 --- a/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php +++ b/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php @@ -6,12 +6,15 @@ use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\ConstFetch; +use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Name; use PhpParser\Node\Scalar\Float_; use PhpParser\Node\Scalar\Int_; use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt\Class_; +use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Property; use PHPStan\Reflection\ClassReflection; use PHPStan\Type\FloatType; @@ -21,12 +24,14 @@ use PHPStan\Type\TypeCombinator; use PHPStan\Type\UnionType; use Rector\Php74\Guard\MakePropertyTypedGuard; +use Rector\PhpParser\Node\BetterNodeFinder; use Rector\PHPStanStaticTypeMapper\Enum\TypeKind; use Rector\Rector\AbstractRector; use Rector\Reflection\ReflectionResolver; use Rector\StaticTypeMapper\StaticTypeMapper; use Rector\TypeDeclaration\TypeInferer\PropertyTypeInferer\GetterTypeDeclarationPropertyTypeInferer; use Rector\TypeDeclaration\TypeInferer\PropertyTypeInferer\SetterTypeDeclarationPropertyTypeInferer; +use Rector\ValueObject\MethodName; use Rector\ValueObject\PhpVersionFeature; use Rector\VersionBonding\Contract\MinPhpVersionInterface; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; @@ -42,7 +47,8 @@ public function __construct( private readonly SetterTypeDeclarationPropertyTypeInferer $setterTypeDeclarationPropertyTypeInferer, private readonly MakePropertyTypedGuard $makePropertyTypedGuard, private readonly ReflectionResolver $reflectionResolver, - private readonly StaticTypeMapper $staticTypeMapper + private readonly StaticTypeMapper $staticTypeMapper, + private readonly BetterNodeFinder $betterNodeFinder ) { } @@ -113,6 +119,11 @@ public function refactor(Node $node): ?Node continue; } + // constructor assignment can set a different type than the setter/getter → skip + if ($this->isAssignedInConstructor($node, $property)) { + continue; + } + $getterSetterPropertyType = $this->matchGetterSetterIdenticalType($property, $node); if (! $getterSetterPropertyType instanceof Type) { continue; @@ -267,6 +278,33 @@ private function decorateDefaultExpr( $propertyProperty->default = new ConstFetch(new Name('null')); } + private function isAssignedInConstructor(Class_ $class, Property $property): bool + { + $constructClassMethod = $class->getMethod(MethodName::CONSTRUCT); + if (! $constructClassMethod instanceof ClassMethod) { + return false; + } + + $propertyName = $this->getName($property); + + foreach ($this->betterNodeFinder->findInstanceOf($constructClassMethod, Assign::class) as $assign) { + if (! $assign->var instanceof PropertyFetch) { + continue; + } + + $propertyFetch = $assign->var; + if (! $this->isName($propertyFetch->var, 'this')) { + continue; + } + + if ($this->isName($propertyFetch->name, $propertyName)) { + return true; + } + } + + return false; + } + private function hasPropertyDefaultNull(Property $property): bool { $defaultExpr = $property->props[0]->default ?? null; diff --git a/rules/TypeDeclaration/TypeInferer/PropertyTypeInferer/SetterTypeDeclarationPropertyTypeInferer.php b/rules/TypeDeclaration/TypeInferer/PropertyTypeInferer/SetterTypeDeclarationPropertyTypeInferer.php index b1d2634d003..bc07a55b5d4 100644 --- a/rules/TypeDeclaration/TypeInferer/PropertyTypeInferer/SetterTypeDeclarationPropertyTypeInferer.php +++ b/rules/TypeDeclaration/TypeInferer/PropertyTypeInferer/SetterTypeDeclarationPropertyTypeInferer.php @@ -27,7 +27,7 @@ public function inferProperty(Property $property, Class_ $class): ?Type $propertyName = $this->nodeNameResolver->getName($property); foreach ($class->getMethods() as $classMethod) { - if (! $this->classMethodAndPropertyAnalyzer->hasOnlyFirstParamPropertyAssign($classMethod, $propertyName)) { + if (! $this->classMethodAndPropertyAnalyzer->hasOnlyPropertyAssign($classMethod, $propertyName)) { continue; }