From 5cbf11be45e6d7be1ab20ce729551e30b02015e1 Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Mon, 6 Apr 2026 18:59:17 +0000 Subject: [PATCH 01/10] Fix phpstan/phpstan#13473: Take into account Property Hooks in __construct() - Property set hooks should not assume the property is initialized, since the hook runs at assignment time when the property may not yet have been set - Invalidate PropertyInitializationExpr for the hooked property when entering a set hook scope - New regression test in tests/PHPStan/Rules/Variables/IssetRuleTest.php --- src/Analyser/MutatingScope.php | 8 ++++++- .../PHPStan/Rules/Variables/IssetRuleTest.php | 8 +++++++ .../Rules/Variables/data/bug-13473.php | 24 +++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Variables/data/bug-13473.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index a43ba35dda5..0997fb2217e 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1574,7 +1574,7 @@ public function enterPropertyHook( $realParameterTypes = $this->getRealParameterTypes($hook); - return $this->enterFunctionLike( + $scope = $this->enterFunctionLike( new PhpMethodFromParserNodeReflection( $this->getClassReflection(), $hook, @@ -1606,6 +1606,12 @@ public function enterPropertyHook( ), true, ); + + if ($hookName === 'set') { + $scope = $scope->invalidateExpression(new PropertyInitializationExpr($propertyName)); + } + + return $scope; } private function transformStaticType(Type $type): Type diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index fd841e49b16..ac4a49fe051 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -526,6 +526,14 @@ public function testBug9503(): void $this->analyse([__DIR__ . '/data/bug-9503.php'], []); } + #[RequiresPhp('>= 8.4')] + public function testBug13473(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/bug-13473.php'], []); + } + public function testBug14393(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Variables/data/bug-13473.php b/tests/PHPStan/Rules/Variables/data/bug-13473.php new file mode 100644 index 00000000000..e4ab49046a6 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-13473.php @@ -0,0 +1,24 @@ += 8.4 + +declare(strict_types = 1); + +namespace Bug13473; + +class Foo { + private(set) int $bar { + get => $this->bar; + set(int $bar) { + if (isset($this->bar)) { + throw new \Exception('bar is set'); + } + $this->bar = $bar; + } + } + + public function __construct(int $bar) + { + $this->bar = $bar; + } +} + +$foo = new Foo(10); From 7a6c50aab90c9972f90f724ddc899ab1836b85db Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Mon, 6 Apr 2026 19:28:17 +0000 Subject: [PATCH 02/10] Add test for property with default value in set hook isset check When a property has a default value (e.g. $bar = 1), it is always initialized, so isset() in a set hook should still report an error. Co-Authored-By: Claude Opus 4.6 --- tests/PHPStan/Rules/Variables/IssetRuleTest.php | 7 ++++++- .../PHPStan/Rules/Variables/data/bug-13473.php | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index ac4a49fe051..ec04c01b7e9 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -531,7 +531,12 @@ public function testBug13473(): void { $this->treatPhpDocTypesAsCertain = true; - $this->analyse([__DIR__ . '/data/bug-13473.php'], []); + $this->analyse([__DIR__ . '/data/bug-13473.php'], [ + [ + 'Property Bug13473\Bar::$bar (int) in isset() is not nullable.', + 30, + ], + ]); } public function testBug14393(): void diff --git a/tests/PHPStan/Rules/Variables/data/bug-13473.php b/tests/PHPStan/Rules/Variables/data/bug-13473.php index e4ab49046a6..558228d1d2b 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-13473.php +++ b/tests/PHPStan/Rules/Variables/data/bug-13473.php @@ -22,3 +22,20 @@ public function __construct(int $bar) } $foo = new Foo(10); + +class Bar { + private(set) int $bar = 1 { + get => $this->bar; + set(int $bar) { + if (isset($this->bar)) { + throw new \Exception('bar is set'); + } + $this->bar = $bar; + } + } + + public function __construct(int $bar) + { + $this->bar = $bar; + } +} From be75095c4bd5aba364e59b77d72d97ff950ee4e1 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Mon, 6 Apr 2026 19:41:08 +0000 Subject: [PATCH 03/10] Keep property as initialized in set hook when it has a default value When a property has a default value (e.g. `int $bar = 1`), it is always initialized even during __construct(). Only invalidate the property initialization in set hooks for properties without default values. Co-Authored-By: Claude Opus 4.6 --- src/Analyser/MutatingScope.php | 2 +- tests/PHPStan/Rules/Variables/IssetRuleTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 0997fb2217e..b82d5c1119b 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1607,7 +1607,7 @@ public function enterPropertyHook( true, ); - if ($hookName === 'set') { + if ($hookName === 'set' && !$this->getClassReflection()->getNativeProperty($propertyName)->getNativeReflection()->hasDefaultValue()) { $scope = $scope->invalidateExpression(new PropertyInitializationExpr($propertyName)); } diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index ec04c01b7e9..c2a29dd0116 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -533,7 +533,7 @@ public function testBug13473(): void $this->analyse([__DIR__ . '/data/bug-13473.php'], [ [ - 'Property Bug13473\Bar::$bar (int) in isset() is not nullable.', + 'Property Bug13473\Bar::$bar in isset() is not nullable nor uninitialized.', 30, ], ]); From 185d02d10acaff172553fb7ea5d55149aa856be1 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 11 Apr 2026 11:46:17 +0000 Subject: [PATCH 04/10] Explain why preserveConstructorScope must be true for property hooks Add comment and cross-property test demonstrating that preserveConstructorScope: false would break $this access and lose initialization state for sibling properties (e.g. isset() on a different property with a default value inside a set hook). Co-Authored-By: Claude Opus 4.6 --- src/Analyser/MutatingScope.php | 5 +++++ .../PHPStan/Rules/Variables/IssetRuleTest.php | 4 ++++ .../Rules/Variables/data/bug-13473.php | 21 +++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index b82d5c1119b..ae0954c4ed6 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1574,6 +1574,11 @@ public function enterPropertyHook( $realParameterTypes = $this->getRealParameterTypes($hook); + // preserveConstructorScope must be true: it provides $this and + // PropertyInitializationExpr entries for other properties (needed to + // correctly report isset() on always-initialized sibling properties). + // We then surgically invalidate only this property's initialization + // for set hooks without a default value. $scope = $this->enterFunctionLike( new PhpMethodFromParserNodeReflection( $this->getClassReflection(), diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index c2a29dd0116..831b78027de 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -536,6 +536,10 @@ public function testBug13473(): void 'Property Bug13473\Bar::$bar in isset() is not nullable nor uninitialized.', 30, ], + [ + 'Property Bug13473\Baz::$foo (int) in isset() is not nullable.', + 48, + ], ]); } diff --git a/tests/PHPStan/Rules/Variables/data/bug-13473.php b/tests/PHPStan/Rules/Variables/data/bug-13473.php index 558228d1d2b..900ee5255d6 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-13473.php +++ b/tests/PHPStan/Rules/Variables/data/bug-13473.php @@ -39,3 +39,24 @@ public function __construct(int $bar) $this->bar = $bar; } } + +class Baz { + private(set) int $foo = 5; + private(set) int $bar { + get => $this->bar; + set(int $bar) { + if (isset($this->foo)) { // $foo has default value, always initialized - should error + echo 'foo is set'; + } + if (isset($this->bar)) { // $bar has no default, could be uninitialized - no error + throw new \Exception('bar is set'); + } + $this->bar = $bar; + } + } + + public function __construct(int $bar) + { + $this->bar = $bar; + } +} From 5b1dabc91a8714085957d97e69f660eca71b5549 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 11 Apr 2026 12:05:34 +0000 Subject: [PATCH 05/10] Invalidate all properties without defaults in set hooks, not just the hooked one A set hook can run during __construct() when any property without a default value might still be uninitialized, not just the property being hooked. For example, $bar's set hook checking isset($this->foo) where $foo also has no default value should not report a false positive. Co-Authored-By: Claude Opus 4.6 --- src/Analyser/MutatingScope.php | 18 ++++++++++++------ .../PHPStan/Rules/Variables/IssetRuleTest.php | 2 +- .../Rules/Variables/data/bug-13473.php | 19 +++++++++++++++++++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index ae0954c4ed6..cd782fc003a 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1575,10 +1575,11 @@ public function enterPropertyHook( $realParameterTypes = $this->getRealParameterTypes($hook); // preserveConstructorScope must be true: it provides $this and - // PropertyInitializationExpr entries for other properties (needed to - // correctly report isset() on always-initialized sibling properties). - // We then surgically invalidate only this property's initialization - // for set hooks without a default value. + // PropertyInitializationExpr entries for properties with default values + // (needed to correctly report isset() on always-initialized properties). + // For set hooks, we then invalidate initialization state for all + // properties without a default value, since the hook can run during + // __construct() when any such property might still be uninitialized. $scope = $this->enterFunctionLike( new PhpMethodFromParserNodeReflection( $this->getClassReflection(), @@ -1612,8 +1613,13 @@ public function enterPropertyHook( true, ); - if ($hookName === 'set' && !$this->getClassReflection()->getNativeProperty($propertyName)->getNativeReflection()->hasDefaultValue()) { - $scope = $scope->invalidateExpression(new PropertyInitializationExpr($propertyName)); + if ($hookName === 'set') { + foreach ($this->getClassReflection()->getNativeReflection()->getProperties() as $nativeProperty) { + if ($nativeProperty->hasDefaultValue()) { + continue; + } + $scope = $scope->invalidateExpression(new PropertyInitializationExpr($nativeProperty->getName())); + } } return $scope; diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index 831b78027de..f04e6fafc4e 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -538,7 +538,7 @@ public function testBug13473(): void ], [ 'Property Bug13473\Baz::$foo (int) in isset() is not nullable.', - 48, + 67, ], ]); } diff --git a/tests/PHPStan/Rules/Variables/data/bug-13473.php b/tests/PHPStan/Rules/Variables/data/bug-13473.php index 900ee5255d6..1defd8ac54a 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-13473.php +++ b/tests/PHPStan/Rules/Variables/data/bug-13473.php @@ -40,6 +40,25 @@ public function __construct(int $bar) } } +class Qux { + private(set) int $foo; + private(set) int $bar { + get => $this->bar; + set(int $bar) { + if (isset($this->foo)) { // $foo has no default, could be uninitialized - no error + throw new \Exception('foo is set'); + } + $this->bar = $bar; + } + } + + public function __construct(int $bar) + { + $this->bar = $bar; + $this->foo = 42; + } +} + class Baz { private(set) int $foo = 5; private(set) int $bar { From 0fa10a65488ba853c720e66a730b1180fab58fb2 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 11 Apr 2026 12:42:05 +0000 Subject: [PATCH 06/10] Remove explanatory comment about preserveConstructorScope in enterPropertyHook Co-Authored-By: Claude Opus 4.6 --- src/Analyser/MutatingScope.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index cd782fc003a..59ad3f638f3 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1574,12 +1574,6 @@ public function enterPropertyHook( $realParameterTypes = $this->getRealParameterTypes($hook); - // preserveConstructorScope must be true: it provides $this and - // PropertyInitializationExpr entries for properties with default values - // (needed to correctly report isset() on always-initialized properties). - // For set hooks, we then invalidate initialization state for all - // properties without a default value, since the hook can run during - // __construct() when any such property might still be uninitialized. $scope = $this->enterFunctionLike( new PhpMethodFromParserNodeReflection( $this->getClassReflection(), From cd5f31fbaccde360a3d582b946af74eff7217b6c Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 11 Apr 2026 14:47:50 +0000 Subject: [PATCH 07/10] Track uninitialized properties in methods called from constructor When a method is called from a constructor, track which properties were uninitialized at the call site and invalidate their PropertyInitializationExpr in the called method's scope. This allows isset() checks on uninitialized properties to be correctly recognized as valid rather than producing false positives. Also adds !isInAnonymousFunction() check to prevent closures inside constructors from being treated as constructor-called methods. Co-Authored-By: Claude Opus 4.6 --- .../ExprHandler/MethodCallHandler.php | 16 ++++++++++++- src/Analyser/NodeScopeResolver.php | 24 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/Analyser/ExprHandler/MethodCallHandler.php b/src/Analyser/ExprHandler/MethodCallHandler.php index 28e0181c331..417078c6e12 100644 --- a/src/Analyser/ExprHandler/MethodCallHandler.php +++ b/src/Analyser/ExprHandler/MethodCallHandler.php @@ -22,6 +22,7 @@ use PHPStan\Analyser\NodeScopeResolver; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; +use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Expr\PossiblyImpureCallExpr; use PHPStan\Node\InvalidateExprNode; use PHPStan\Reflection\Callables\SimpleImpurePoint; @@ -204,11 +205,24 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex } if ( $scope->isInClass() + && !$scope->isInAnonymousFunction() && $scope->getClassReflection()->getName() === $methodReflection->getDeclaringClass()->getName() && ($scope->getFunctionName() !== null && strtolower($scope->getFunctionName()) === '__construct') && TypeUtils::findThisType($calledOnType) !== null ) { - $calledMethodScope = $nodeScopeResolver->processCalledMethod($methodReflection); + $stackName = sprintf('%s::%s', $methodReflection->getDeclaringClass()->getName(), $methodReflection->getName()); + $uninitializedProperties = []; + foreach ($scope->getClassReflection()->getNativeReflection()->getProperties() as $nativeProperty) { + if ($nativeProperty->hasDefaultValue() || $nativeProperty->isStatic()) { + continue; + } + if (!$scope->hasExpressionType(new PropertyInitializationExpr($nativeProperty->getName()))->yes()) { + $uninitializedProperties[$nativeProperty->getName()] = true; + } + } + $nodeScopeResolver->registerCalledMethodUninitializedProperties($stackName, $uninitializedProperties); + + $calledMethodScope = $nodeScopeResolver->processCalledMethod($methodReflection); if ($calledMethodScope !== null) { $scope = $scope->mergeInitializedProperties($calledMethodScope); return new ExpressionResult( diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 001c8b5b368..6a6fe148726 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -199,6 +199,9 @@ class NodeScopeResolver /** @var array */ private array $calledMethodResults = []; + /** @var array> */ + private array $calledMethodUninitializedProperties = []; + /** * @param string[][] $earlyTerminatingMethodCalls className(string) => methods(string[]) * @param array $earlyTerminatingFunctionCalls @@ -751,6 +754,13 @@ public function processStmtNode( ); $methodScope = $methodScope->assignExpression(new PropertyInitializationExpr($param->var->name), new MixedType(), new MixedType()); } + } elseif (!$stmt->isStatic()) { + $stackName = sprintf('%s::%s', $classReflection->getName(), $stmt->name->toString()); + if (array_key_exists($stackName, $this->calledMethodUninitializedProperties)) { + foreach ($this->calledMethodUninitializedProperties[$stackName] as $propertyName => $_) { + $methodScope = $methodScope->invalidateExpression(new PropertyInitializationExpr($propertyName)); + } + } } if ($stmt->getAttribute('virtual', false) === false) { @@ -1017,6 +1027,7 @@ public function processStmtNode( $this->callNodeCallback($nodeCallback, new ClassConstantsNode($stmt, $classStatementsGatherer->getConstants(), $classStatementsGatherer->getConstantFetches(), $classReflection), $classScope, $storage); $classReflection->evictPrivateSymbols(); $this->calledMethodResults = []; + $this->calledMethodUninitializedProperties = []; } elseif ($stmt instanceof Node\Stmt\Property) { $hasYield = false; $throwPoints = []; @@ -4045,6 +4056,19 @@ private function processNodesForTraitUse($node, ClassReflection $traitReflection } } + /** + * @param array $uninitializedProperties + */ + public function registerCalledMethodUninitializedProperties(string $stackName, array $uninitializedProperties): void + { + if (!array_key_exists($stackName, $this->calledMethodUninitializedProperties)) { + $this->calledMethodUninitializedProperties[$stackName] = $uninitializedProperties; + return; + } + + $this->calledMethodUninitializedProperties[$stackName] = $this->calledMethodUninitializedProperties[$stackName] + $uninitializedProperties; + } + public function processCalledMethod(MethodReflection $methodReflection): ?MutatingScope { $declaringClass = $methodReflection->getDeclaringClass(); From 9660125dba4f35bc602dececd4e7f1bd7b23ae46 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 11 Apr 2026 14:47:58 +0000 Subject: [PATCH 08/10] Add test for isset in methods called from constructor Tests that isset() on properties without defaults in methods called from the constructor does not produce false positives, while isset() on properties with defaults or in non-constructor-called methods still correctly reports errors. Co-Authored-By: Claude Opus 4.6 --- .../PHPStan/Rules/Variables/IssetRuleTest.php | 20 +++++ .../isset-method-called-from-constructor.php | 77 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 tests/PHPStan/Rules/Variables/data/isset-method-called-from-constructor.php diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index f04e6fafc4e..ce2cc2f5af5 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -543,6 +543,26 @@ public function testBug13473(): void ]); } + public function testIssetMethodCalledFromConstructor(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/isset-method-called-from-constructor.php'], [ + [ + 'Property IssetMethodCalledFromConstructor\MethodCalledFromConstructorWithDefault::$bar in isset() is not nullable nor uninitialized.', + 34, + ], + [ + 'Property IssetMethodCalledFromConstructor\MethodNotCalledFromConstructor::$bar in isset() is not nullable nor uninitialized.', + 51, + ], + [ + 'Property IssetMethodCalledFromConstructor\MultipleProperties::$bar in isset() is not nullable nor uninitialized.', + 72, + ], + ]); + } + public function testBug14393(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Variables/data/isset-method-called-from-constructor.php b/tests/PHPStan/Rules/Variables/data/isset-method-called-from-constructor.php new file mode 100644 index 00000000000..b9597f4d5db --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/isset-method-called-from-constructor.php @@ -0,0 +1,77 @@ +setBar($bar); + } + + private function setBar(int $bar): void + { + if (isset($this->bar)) { // $bar has no default, could be uninitialized when called from constructor - no error + throw new \Exception('bar is set'); + } + $this->bar = $bar; + } +} + +final class MethodCalledFromConstructorWithDefault { + private int $bar = 1; + + public function __construct(int $bar) + { + $this->setBar($bar); + } + + private function setBar(int $bar): void + { + if (isset($this->bar)) { // $bar has default value, always initialized - should error + throw new \Exception('bar is set'); + } + $this->bar = $bar; + } +} + +final class MethodNotCalledFromConstructor { + private int $bar; + + public function __construct(int $bar) + { + $this->bar = $bar; + } + + private function checkBar(): void + { + if (isset($this->bar)) { // Not called from constructor, property is initialized after construction - should error + echo 'bar is set'; + } + } +} + +final class MultipleProperties { + private int $foo; + private int $bar = 5; + + public function __construct(int $bar) + { + $this->init($bar); + $this->foo = 42; + } + + private function init(int $bar): void + { + if (isset($this->foo)) { // $foo has no default, could be uninitialized - no error + throw new \Exception('foo is set'); + } + if (isset($this->bar)) { // $bar has default value, always initialized - should error + echo 'bar is set'; + } + $this->bar = $bar; + } +} From b8566d355b2710662784b1d23deb006a8894d2fd Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 11 Apr 2026 14:48:09 +0000 Subject: [PATCH 09/10] Uncomment expected premature access errors in readonly property test Now that uninitialized properties are correctly tracked in methods called from constructors, premature access to readonly properties in those methods is properly detected. Co-Authored-By: Claude Opus 4.6 --- .../Properties/MissingReadOnlyPropertyAssignRuleTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php index 2fc14f808da..e8269e5c3eb 100644 --- a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php @@ -137,14 +137,14 @@ public function testRule(): void 'Readonly property MissingReadOnlyPropertyAssign\AdditionalAssignOfReadonlyPromotedProperty::$x is already assigned.', 188, ], - /*[ + [ 'Access to an uninitialized readonly property MissingReadOnlyPropertyAssign\MethodCalledFromConstructorBeforeAssign::$foo.', 226, ], [ 'Access to an uninitialized readonly property MissingReadOnlyPropertyAssign\MethodCalledTwice::$foo.', 244, - ],*/ + ], [ 'Class MissingReadOnlyPropertyAssign\PropertyAssignedOnDifferentObjectUninitialized has an uninitialized readonly property $foo. Assign it in the constructor.', 264, From 703d40ea1e0e991c59fc22835e616c0678c1fc99 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 11 Apr 2026 15:35:21 +0000 Subject: [PATCH 10/10] Unify set hook and constructor-called method uninitialized property tracking Move PropertyInitializationExpr invalidation from MutatingScope::enterPropertyHook() to NodeScopeResolver::processPropertyHooks(), using the same calledMethodUninitializedProperties mechanism that constructor-called methods use. For set hooks assigned directly in the constructor, AssignHandler now records the actual scope state (which properties are initialized at the assignment point), giving more precise results. For hooks not directly triggered from the constructor, the previous fallback (invalidate all properties without defaults) is preserved. This also fixes indentation in MethodCallHandler and adds tests demonstrating the improved precision: isset() on a property initialized before the call site is now correctly reported as always-true. Co-Authored-By: Claude Opus 4.6 --- src/Analyser/ExprHandler/AssignHandler.php | 23 +++++++++++++++++++ .../ExprHandler/MethodCallHandler.php | 20 ++++++++-------- src/Analyser/MutatingScope.php | 13 +---------- src/Analyser/NodeScopeResolver.php | 18 +++++++++++++++ .../PHPStan/Rules/Variables/IssetRuleTest.php | 8 +++++++ .../Rules/Variables/data/bug-13473.php | 19 +++++++++++++++ .../isset-method-called-from-constructor.php | 22 ++++++++++++++++++ 7 files changed, 101 insertions(+), 22 deletions(-) diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 51995ac6657..fa2986274d2 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -39,6 +39,7 @@ use PHPStan\Node\Expr\GetOffsetValueTypeExpr; use PHPStan\Node\Expr\IntertwinedVariableByReferenceWithExpr; use PHPStan\Node\Expr\OriginalPropertyTypeExpr; +use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr; use PHPStan\Node\Expr\SetOffsetValueTypeExpr; use PHPStan\Node\Expr\TypeExpr; @@ -73,6 +74,8 @@ use function in_array; use function is_int; use function is_string; +use function sprintf; +use function strtolower; /** * @implements ExprHandler @@ -611,6 +614,26 @@ public function processAssignVar( } if ($this->phpVersion->supportsPropertyHooks()) { $throwPoints = array_merge($throwPoints, $nodeScopeResolver->getThrowPointsFromPropertyHook($scope, $var, $nativeProperty, 'set')); + if ( + $nativeProperty->hasHook('set') + && $scope->isInClass() + && !$scope->isInAnonymousFunction() + && $scope->getFunctionName() !== null + && strtolower($scope->getFunctionName()) === '__construct' + && TypeUtils::findThisType($propertyHolderType) !== null + ) { + $hookStackName = sprintf('%s::$%s::set', $declaringClass->getName(), $propertyName); + $uninitializedProperties = []; + foreach ($scope->getClassReflection()->getNativeReflection()->getProperties() as $refProperty) { + if ($refProperty->hasDefaultValue() || $refProperty->isStatic()) { + continue; + } + if (!$scope->hasExpressionType(new PropertyInitializationExpr($refProperty->getName()))->yes()) { + $uninitializedProperties[$refProperty->getName()] = true; + } + } + $nodeScopeResolver->registerCalledMethodUninitializedProperties($hookStackName, $uninitializedProperties); + } } if ($enterExpressionAssign) { $scope = $scope->assignInitializedProperty($propertyHolderType, $propertyName); diff --git a/src/Analyser/ExprHandler/MethodCallHandler.php b/src/Analyser/ExprHandler/MethodCallHandler.php index 417078c6e12..ffef401791d 100644 --- a/src/Analyser/ExprHandler/MethodCallHandler.php +++ b/src/Analyser/ExprHandler/MethodCallHandler.php @@ -211,18 +211,18 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex && TypeUtils::findThisType($calledOnType) !== null ) { $stackName = sprintf('%s::%s', $methodReflection->getDeclaringClass()->getName(), $methodReflection->getName()); - $uninitializedProperties = []; - foreach ($scope->getClassReflection()->getNativeReflection()->getProperties() as $nativeProperty) { - if ($nativeProperty->hasDefaultValue() || $nativeProperty->isStatic()) { - continue; - } - if (!$scope->hasExpressionType(new PropertyInitializationExpr($nativeProperty->getName()))->yes()) { - $uninitializedProperties[$nativeProperty->getName()] = true; - } + $uninitializedProperties = []; + foreach ($scope->getClassReflection()->getNativeReflection()->getProperties() as $nativeProperty) { + if ($nativeProperty->hasDefaultValue() || $nativeProperty->isStatic()) { + continue; } - $nodeScopeResolver->registerCalledMethodUninitializedProperties($stackName, $uninitializedProperties); + if (!$scope->hasExpressionType(new PropertyInitializationExpr($nativeProperty->getName()))->yes()) { + $uninitializedProperties[$nativeProperty->getName()] = true; + } + } + $nodeScopeResolver->registerCalledMethodUninitializedProperties($stackName, $uninitializedProperties); - $calledMethodScope = $nodeScopeResolver->processCalledMethod($methodReflection); + $calledMethodScope = $nodeScopeResolver->processCalledMethod($methodReflection); if ($calledMethodScope !== null) { $scope = $scope->mergeInitializedProperties($calledMethodScope); return new ExpressionResult( diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 59ad3f638f3..a43ba35dda5 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1574,7 +1574,7 @@ public function enterPropertyHook( $realParameterTypes = $this->getRealParameterTypes($hook); - $scope = $this->enterFunctionLike( + return $this->enterFunctionLike( new PhpMethodFromParserNodeReflection( $this->getClassReflection(), $hook, @@ -1606,17 +1606,6 @@ public function enterPropertyHook( ), true, ); - - if ($hookName === 'set') { - foreach ($this->getClassReflection()->getNativeReflection()->getProperties() as $nativeProperty) { - if ($nativeProperty->hasDefaultValue()) { - continue; - } - $scope = $scope->invalidateExpression(new PropertyInitializationExpr($nativeProperty->getName())); - } - } - - return $scope; } private function transformStaticType(Type $type): Type diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 6a6fe148726..bad7bff4908 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -3096,6 +3096,24 @@ private function processPropertyHooks( $phpDocComment, $resolvedPhpDoc, ); + + $hookName = $hook->name->toLowerString(); + if ($hookName === 'set') { + $hookStackName = sprintf('%s::$%s::set', $classReflection->getName(), $propertyName); + if (array_key_exists($hookStackName, $this->calledMethodUninitializedProperties)) { + foreach ($this->calledMethodUninitializedProperties[$hookStackName] as $propName => $_) { + $hookScope = $hookScope->invalidateExpression(new PropertyInitializationExpr($propName)); + } + } else { + foreach ($classReflection->getNativeReflection()->getProperties() as $nativeProperty) { + if ($nativeProperty->hasDefaultValue() || $nativeProperty->isStatic()) { + continue; + } + $hookScope = $hookScope->invalidateExpression(new PropertyInitializationExpr($nativeProperty->getName())); + } + } + } + $hookReflection = $hookScope->getFunction(); if (!$hookReflection instanceof PhpMethodFromParserNodeReflection) { throw new ShouldNotHappenException(); diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index ce2cc2f5af5..e690cdec768 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -540,6 +540,10 @@ public function testBug13473(): void 'Property Bug13473\Baz::$foo (int) in isset() is not nullable.', 67, ], + [ + 'Property Bug13473\PropertyInitializedBeforeHookedAssignment::$foo in isset() is not nullable nor uninitialized.', + 88, + ], ]); } @@ -560,6 +564,10 @@ public function testIssetMethodCalledFromConstructor(): void 'Property IssetMethodCalledFromConstructor\MultipleProperties::$bar in isset() is not nullable nor uninitialized.', 72, ], + [ + 'Property IssetMethodCalledFromConstructor\PropertyInitializedBeforeMethodCall::$foo in isset() is not nullable nor uninitialized.', + 91, + ], ]); } diff --git a/tests/PHPStan/Rules/Variables/data/bug-13473.php b/tests/PHPStan/Rules/Variables/data/bug-13473.php index 1defd8ac54a..2a028894c38 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-13473.php +++ b/tests/PHPStan/Rules/Variables/data/bug-13473.php @@ -79,3 +79,22 @@ public function __construct(int $bar) $this->bar = $bar; } } + +class PropertyInitializedBeforeHookedAssignment { + private(set) int $foo; + private(set) int $bar { + get => $this->bar; + set(int $bar) { + if (isset($this->foo)) { // $foo was initialized before $this->bar = ... in constructor - should error + echo 'foo is set'; + } + $this->bar = $bar; + } + } + + public function __construct(int $bar) + { + $this->foo = 42; + $this->bar = $bar; + } +} diff --git a/tests/PHPStan/Rules/Variables/data/isset-method-called-from-constructor.php b/tests/PHPStan/Rules/Variables/data/isset-method-called-from-constructor.php index b9597f4d5db..5b76becf6ac 100644 --- a/tests/PHPStan/Rules/Variables/data/isset-method-called-from-constructor.php +++ b/tests/PHPStan/Rules/Variables/data/isset-method-called-from-constructor.php @@ -75,3 +75,25 @@ private function init(int $bar): void $this->bar = $bar; } } + +final class PropertyInitializedBeforeMethodCall { + private int $foo; + private int $bar; + + public function __construct() + { + $this->foo = 1; + $this->setBar(); + } + + private function setBar(): void + { + if (isset($this->foo)) { // $foo was initialized before the call - should error + echo 'foo is set'; + } + if (isset($this->bar)) { // $bar has no default, not yet initialized - no error + echo 'bar is set'; + } + $this->bar = 2; + } +}