Properties set in the native constructor are initialized in additional constructors

ondrejmirtes · ondrejmirtes · commit 1b0c6a0166e6 · 2023-08-08T10:35:36.000+02:00
diff --git a/src/Node/ClassPropertiesNode.php b/src/Node/ClassPropertiesNode.php
@@ -22,6 +22,7 @@
 use PHPStan\TrinaryLogic;
 use PHPStan\Type\NeverType;
 use PHPStan\Type\TypeUtils;
+use function array_diff_key;
 use function array_key_exists;
 use function array_keys;
 use function in_array;
@@ -157,6 +158,11 @@ public function getUninitializedProperties(
 		$prematureAccess = [];
 		$additionalAssigns = [];
 
+		$initializedInConstructor = [];
+		if ($classReflection->hasConstructor()) {
+			$initializedInConstructor = array_diff_key($uninitializedProperties, $this->collectUninitializedProperties([$classReflection->getConstructor()->getName()], $uninitializedProperties));
+		}
+
 		foreach ($this->getPropertyUsages() as $usage) {
 			$fetch = $usage->getFetch();
 			if (!$fetch instanceof PropertyFetch) {
@@ -213,6 +219,13 @@ public function getUninitializedProperties(
 				}
 			} elseif (array_key_exists($propertyName, $initializedPropertiesMap)) {
 				$hasInitialization = $initializedPropertiesMap[$propertyName]->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName)));
+				if (
+					strtolower($function->getName()) !== '__construct'
+					&& array_key_exists($propertyName, $initializedInConstructor)
+					&& in_array($function->getName(), $constructors, true)
+				) {
+					continue;
+				}
 				if (!$hasInitialization->yes()) {
 					$prematureAccess[] = [
 						$propertyName,
@@ -224,7 +237,21 @@ public function getUninitializedProperties(
 			}
 		}
 
-		foreach (array_keys($methodsCalledFromConstructor) as $constructor) {
+		return [
+			$this->collectUninitializedProperties(array_keys($methodsCalledFromConstructor), $uninitializedProperties),
+			$prematureAccess,
+			$additionalAssigns,
+		];
+	}
+
+	/**
+	 * @param list<string> $constructors
+	 * @param array<string, ClassPropertyNode> $uninitializedProperties
+	 * @return array<string, ClassPropertyNode>
+	 */
+	private function collectUninitializedProperties(array $constructors, array $uninitializedProperties): array
+	{
+		foreach ($constructors as $constructor) {
 			$lowerConstructorName = strtolower($constructor);
 			if (!array_key_exists($lowerConstructorName, $this->returnStatementNodes)) {
 				continue;
@@ -275,11 +302,7 @@ public function getUninitializedProperties(
 			}
 		}
 
-		return [
-			$uninitializedProperties,
-			$prematureAccess,
-			$additionalAssigns,
-		];
+		return $uninitializedProperties;
 	}
 
 	/**
diff --git a/tests/PHPStan/Rules/Properties/UninitializedPropertyRuleTest.php b/tests/PHPStan/Rules/Properties/UninitializedPropertyRuleTest.php
@@ -22,6 +22,9 @@ protected function getRule(): Rule
 				[
 					'UninitializedProperty\\TestCase::setUp',
 					'Bug9619\\AdminPresenter::startup',
+					'Bug9619\\AdminPresenter2::startup',
+					'Bug9619\\AdminPresenter3::startup',
+					'Bug9619\\AdminPresenter3::startup2',
 				],
 			),
 		);
@@ -181,7 +184,12 @@ public function testEfabricaLatteBug(): void
 
 	public function testBug9619(): void
 	{
-		$this->analyse([__DIR__ . '/data/bug-9619.php'], []);
+		$this->analyse([__DIR__ . '/data/bug-9619.php'], [
+			[
+				'Access to an uninitialized property Bug9619\AdminPresenter3::$user.',
+				55,
+			],
+		]);
 	}
 
 }
diff --git a/tests/PHPStan/Rules/Properties/data/bug-9619.php b/tests/PHPStan/Rules/Properties/data/bug-9619.php
@@ -21,3 +21,39 @@ public function startup()
 		}
 	}
 }
+
+class AdminPresenter2
+{
+	private User $user;
+
+	public function __construct(User $user)
+	{
+		$this->user = $user;
+	}
+
+	public function startup()
+	{
+		// do not report uninitialized property - it's initialized for sure
+		if (!$this->user->isLoggedIn()) {
+			// do something
+		}
+	}
+}
+
+class AdminPresenter3
+{
+	private \stdClass $user;
+
+	public function startup()
+	{
+		$this->user = new \stdClass();
+	}
+
+	public function startup2()
+	{
+		// we cannot be sure which additional constructor gets called first
+		if (!$this->user->loggedIn) {
+			// do something
+		}
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,9 @@ protected function getRule(): Rule`
`22`	`22`	`[`
`23`	`23`	`'UninitializedProperty\\TestCase::setUp',`
`24`	`24`	`'Bug9619\\AdminPresenter::startup',`
	`25`	`+ 'Bug9619\\AdminPresenter2::startup',`
	`26`	`+ 'Bug9619\\AdminPresenter3::startup',`
	`27`	`+ 'Bug9619\\AdminPresenter3::startup2',`
`25`	`28`	`],`
`26`	`29`	`),`
`27`	`30`	`);`
`@@ -181,7 +184,12 @@ public function testEfabricaLatteBug(): void`
`181`	`184`
`182`	`185`	`public function testBug9619(): void`
`183`	`186`	`{`
`184`		`- $this->analyse([__DIR__ . '/data/bug-9619.php'], []);`
	`187`	`+ $this->analyse([__DIR__ . '/data/bug-9619.php'], [`
	`188`	`+ [`
	`189`	`+ 'Access to an uninitialized property Bug9619\AdminPresenter3::$user.',`
	`190`	`+ 55,`
	`191`	`+ ],`
	`192`	`+ ]);`
`185`	`193`	`}`
`186`	`194`
`187`	`195`	`}`