Fix false positives about uninitialized properties

Closes phpstan/phpstan#7198

Author: ondrejmirtes

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,18 +191,18 @@ 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(),
 $originalProperties[$propertyName],
 ];
 }
 }
-} 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<string, TrinaryLogic> $initialInitializedProperties
+ * @param array<string, array<string, TrinaryLogic>> $initializedProperties
+ * @return array<string, array<string, TrinaryLogic>>
  */
 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<string, TrinaryLogic> $initialInitializedProperties
+ * @return array<string, TrinaryLogic>
+ */
+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;
 }
 
 }

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) {

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;
+}
+
 }

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'], []);
+}
+
 }

tests/PHPStan/Rules/Properties/data/bug-7198.php

@@ -0,0 +1,51 @@
+<?php
+
+namespace Bug7198;
+
+trait TestTrait {
+public function foo(): void
+{
+$this->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();
+}
+}

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;
+}
+}