Fix phpstan/phpstan#13473: Take into account Property Hooks in __construct()

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<Assign|AssignRef>
@@ -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);

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,10 +205,23 @@
 		}
 		if (
 			$scope->isInClass()
+			&& !$scope->isInAnonymousFunction()
 			&& $scope->getClassReflection()->getName() === $methodReflection->getDeclaringClass()->getName()
 			&& ($scope->getFunctionName() !== null && strtolower($scope->getFunctionName()) === '__construct')
 			&& 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;
+				}
+			}
+			$nodeScopeResolver->registerCalledMethodUninitializedProperties($stackName, $uninitializedProperties);
+
 			$calledMethodScope = $nodeScopeResolver->processCalledMethod($methodReflection);
 			if ($calledMethodScope !== null) {
 				$scope = $scope->mergeInitializedProperties($calledMethodScope);

src/Analyser/NodeScopeResolver.php

@@ -199,6 +199,9 @@ class NodeScopeResolver
 	/** @var array<string, MutatingScope|null> */
 	private array $calledMethodResults = [];
 
+	/** @var array<string, array<string, true>> */
+	private array $calledMethodUninitializedProperties = [];
+
 	/**
 	 * @param string[][] $earlyTerminatingMethodCalls className(string) => methods(string[])
 	 * @param array<int, string> $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 = [];
@@ -3085,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();
@@ -4045,6 +4074,19 @@ private function processNodesForTraitUse($node, ClassReflection $traitReflection
 		}
 	}
 
+	/**
+	 * @param array<string, true> $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();

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,

tests/PHPStan/Rules/Variables/IssetRuleTest.php

@@ -526,6 +526,51 @@ 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'], [
+			[
+				'Property Bug13473\Bar::$bar in isset() is not nullable nor uninitialized.',
+				30,
+			],
+			[
+				'Property Bug13473\Baz::$foo (int) in isset() is not nullable.',
+				67,
+			],
+			[
+				'Property Bug13473\PropertyInitializedBeforeHookedAssignment::$foo in isset() is not nullable nor uninitialized.',
+				88,
+			],
+		]);
+	}
+
+	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,
+			],
+			[
+				'Property IssetMethodCalledFromConstructor\PropertyInitializedBeforeMethodCall::$foo in isset() is not nullable nor uninitialized.',
+				91,
+			],
+		]);
+	}
+
 	public function testBug14393(): void
 	{
 		$this->treatPhpDocTypesAsCertain = true;

tests/PHPStan/Rules/Variables/data/bug-13473.php

@@ -0,0 +1,100 @@
+<?php // lint >= 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);
+
+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;
+    }
+}
+
+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 {
+        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;
+    }
+}
+
+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;
+    }
+}