diff --git a/src/Node/ClassPropertiesNode.php b/src/Node/ClassPropertiesNode.php index fe9eaf3eb4..3b364646c5 100644 --- a/src/Node/ClassPropertiesNode.php +++ b/src/Node/ClassPropertiesNode.php @@ -20,9 +20,9 @@ use PHPStan\Rules\Properties\ReadWritePropertiesExtension; use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider; use PHPStan\ShouldNotHappenException; +use PHPStan\TrinaryLogic; use function array_key_exists; use function array_keys; -use function count; use function in_array; /** @api */ @@ -100,6 +100,8 @@ public function getUninitializedProperties( $properties = []; $originalProperties = []; + $initialInitializedProperties = []; + $initializedProperties = []; foreach ($this->getProperties() as $property) { if ($property->isStatic()) { continue; @@ -111,6 +113,11 @@ public function getUninitializedProperties( continue; } $originalProperties[$property->getName()] = $property; + $is = TrinaryLogic::createFromBoolean($property->isPromoted()); + $initialInitializedProperties[$property->getName()] = $is; + foreach ($constructors as $constructor) { + $initializedProperties[$constructor][$property->getName()] = $is; + } if ($property->isPromoted()) { continue; } @@ -138,7 +145,8 @@ public function getUninitializedProperties( if ($constructors === []) { return [$properties, [], []]; } - $methodsCalledFromConstructor = $this->getMethodsCalledFromConstructor($classReflection, $this->methodCalls, $constructors); + + $methodsCalledFromConstructor = $this->getMethodsCalledFromConstructor($classReflection, $this->methodCalls, $initialInitializedProperties, $initializedProperties, $constructors); $prematureAccess = []; $additionalAssigns = []; @@ -158,10 +166,12 @@ public function getUninitializedProperties( if ($function->getDeclaringClass()->getName() !== $classReflection->getName()) { continue; } - if (!in_array($function->getName(), $methodsCalledFromConstructor, true)) { + if (!array_key_exists($function->getName(), $methodsCalledFromConstructor)) { continue; } + $initializedPropertiesMap = $methodsCalledFromConstructor[$function->getName()]; + if (!$fetch->name instanceof Identifier) { continue; } @@ -181,9 +191,9 @@ public function getUninitializedProperties( unset($properties[$propertyName]); } - if (array_key_exists($propertyName, $originalProperties)) { - $hasInitialization = $usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName)); - if (!$hasInitialization->no()) { + if (array_key_exists($propertyName, $initializedPropertiesMap)) { + $hasInitialization = $initializedPropertiesMap[$propertyName]->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName))); + if (!$hasInitialization->no() && !$usage->isPromotedPropertyWrite()) { $additionalAssigns[] = [ $propertyName, $fetch->getLine(), @@ -191,8 +201,8 @@ public function getUninitializedProperties( ]; } } - } elseif (array_key_exists($propertyName, $originalProperties)) { - $hasInitialization = $usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName)); + } elseif (array_key_exists($propertyName, $initializedPropertiesMap)) { + $hasInitialization = $initializedPropertiesMap[$propertyName]->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName))); if (!$hasInitialization->yes()) { $prematureAccess[] = [ $propertyName, @@ -213,15 +223,20 @@ public function getUninitializedProperties( /** * @param MethodCall[] $methodCalls * @param string[] $methods - * @return string[] + * @param array $initialInitializedProperties + * @param array> $initializedProperties + * @return array> */ private function getMethodsCalledFromConstructor( ClassReflection $classReflection, array $methodCalls, + array $initialInitializedProperties, + array $initializedProperties, array $methods, ): array { - $originalCount = count($methods); + $originalMap = $initializedProperties; + $originalMethods = $methods; foreach ($methodCalls as $methodCall) { $methodCallNode = $methodCall->getNode(); if ($methodCallNode instanceof Array_) { @@ -242,7 +257,10 @@ private function getMethodsCalledFromConstructor( } $methodName = $methodCallNode->name->toString(); - if (in_array($methodName, $methods, true)) { + if (array_key_exists($methodName, $initializedProperties)) { + foreach ($this->getInitializedProperties($callScope, $initialInitializedProperties) as $propertyName => $isInitialized) { + $initializedProperties[$methodName][$propertyName] = $initializedProperties[$methodName][$propertyName]->and($isInitialized); + } continue; } $methodReflection = $callScope->getMethodReflection($calledOnType, $methodName); @@ -259,14 +277,28 @@ private function getMethodsCalledFromConstructor( if (!in_array($inMethod->getName(), $methods, true)) { continue; } + $initializedProperties[$methodName] = $this->getInitializedProperties($callScope, $initialInitializedProperties); $methods[] = $methodName; } - if ($originalCount === count($methods)) { - return $methods; + if ($originalMap === $initializedProperties && $originalMethods === $methods) { + return $initializedProperties; + } + + return $this->getMethodsCalledFromConstructor($classReflection, $methodCalls, $initialInitializedProperties, $initializedProperties, $methods); + } + + /** + * @param array $initialInitializedProperties + * @return array + */ + private function getInitializedProperties(Scope $scope, array $initialInitializedProperties): array + { + foreach ($initialInitializedProperties as $propertyName => $isInitialized) { + $initialInitializedProperties[$propertyName] = $isInitialized->or($scope->hasExpressionType(new PropertyInitializationExpr($propertyName))); } - return $this->getMethodsCalledFromConstructor($classReflection, $methodCalls, $methods); + return $initialInitializedProperties; } } diff --git a/src/Node/ClassStatementsGatherer.php b/src/Node/ClassStatementsGatherer.php index 6b75bb9446..f16a23e6f4 100644 --- a/src/Node/ClassStatementsGatherer.php +++ b/src/Node/ClassStatementsGatherer.php @@ -130,6 +130,7 @@ private function gatherNodes(Node $node, Scope $scope): void $this->propertyUsages[] = new PropertyWrite( new PropertyFetch(new Expr\Variable('this'), new Identifier($node->getName())), $scope, + true, ); } return; @@ -167,7 +168,7 @@ private function gatherNodes(Node $node, Scope $scope): void return; } if ($node instanceof PropertyAssignNode) { - $this->propertyUsages[] = new PropertyWrite($node->getPropertyFetch(), $scope); + $this->propertyUsages[] = new PropertyWrite($node->getPropertyFetch(), $scope, false); return; } if (!$node instanceof Expr) { @@ -184,7 +185,7 @@ private function gatherNodes(Node $node, Scope $scope): void } $this->propertyUsages[] = new PropertyRead($node->expr, $scope); - $this->propertyUsages[] = new PropertyWrite($node->expr, $scope); + $this->propertyUsages[] = new PropertyWrite($node->expr, $scope, false); return; } if ($node instanceof Node\Scalar\EncapsedStringPart) { diff --git a/src/Node/Property/PropertyWrite.php b/src/Node/Property/PropertyWrite.php index 00970b832d..9577dc7fa8 100644 --- a/src/Node/Property/PropertyWrite.php +++ b/src/Node/Property/PropertyWrite.php @@ -10,7 +10,7 @@ class PropertyWrite { - public function __construct(private PropertyFetch|StaticPropertyFetch $fetch, private Scope $scope) + public function __construct(private PropertyFetch|StaticPropertyFetch $fetch, private Scope $scope, private bool $promotedPropertyWrite) { } @@ -27,4 +27,9 @@ public function getScope(): Scope return $this->scope; } + public function isPromotedPropertyWrite(): bool + { + return $this->promotedPropertyWrite; + } + } diff --git a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php index 86273863ad..0c53c2d216 100644 --- a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php @@ -112,6 +112,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, + ], ]); } @@ -174,4 +182,13 @@ public function testBug6402(): void ]); } + public function testBug7198(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/bug-7198.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-7198.php b/tests/PHPStan/Rules/Properties/data/bug-7198.php new file mode 100644 index 0000000000..b3a973c141 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-7198.php @@ -0,0 +1,51 @@ +callee->foo(); + } +} + +class TestCallee { + public function foo(): void + { + echo "FOO\n"; + } +} + +class TestCaller { + use TestTrait; + + public function __construct(private readonly TestCallee $callee) + { + $this->foo(); + } +} + +class TestCaller2 { + public function foo(): void + { + $this->callee->foo(); + } + + public function __construct(private readonly TestCallee $callee) + { + $this->foo(); + } +} + +class TestCaller3 { + + public function __construct(private readonly TestCallee $callee) + { + $this->foo(); + } + + public function foo(): void + { + $this->callee->foo(); + } +} diff --git a/tests/PHPStan/Rules/Properties/data/missing-readonly-property-assign.php b/tests/PHPStan/Rules/Properties/data/missing-readonly-property-assign.php index 742d46cd05..a42e3b8653 100644 --- a/tests/PHPStan/Rules/Properties/data/missing-readonly-property-assign.php +++ b/tests/PHPStan/Rules/Properties/data/missing-readonly-property-assign.php @@ -189,3 +189,58 @@ public function __construct(private readonly int $x) } } + +class MethodCalledFromConstructorAfterAssign +{ + + + private readonly int $foo; + + public function __construct() + { + $this->foo = 1; + $this->doFoo(); + } + + public function doFoo(): void + { + echo $this->foo; + } + +} + +class MethodCalledFromConstructorBeforeAssign +{ + + + private readonly int $foo; + + public function __construct() + { + $this->doFoo(); + $this->foo = 1; + } + + public function doFoo(): void + { + echo $this->foo; + } + +} + +class MethodCalledTwice +{ + private readonly int $foo; + + public function __construct() + { + $this->doFoo(); + $this->foo = 1; + $this->doFoo(); + } + + public function doFoo(): void + { + echo $this->foo; + } +}