Fix #13736: don't forget private property expressions of different classes

CLAUDE.md

@@ -236,6 +236,10 @@ When considering a bug fix that involves checking "is this type a Foo?", first c
 
 When two scopes are merged (e.g. after if/else branches), `MutatingScope::generalizeWith()` must invalidate dependent expressions. If variable `$i` changes, then `$locations[$i]` must be invalidated too. Bugs arise when stale `ExpressionTypeHolder` entries survive scope merges. Fix pattern: in `MutatingScope`, when a root expression changes, skip/invalidate all deep expressions that depend on it.
 
+### MutatingScope: expression invalidation after method calls and private property visibility
+
+When a method with side effects is called, `invalidateExpression()` invalidates tracked expression types that depend on the call target. When `$this` is invalidated, `shouldInvalidateExpression()` also matches `self`, `static`, `parent`, and class name references — so `self::$prop`, `$this->prop`, etc. all get invalidated. However, private properties of the current class cannot be modified by methods declared in a different class (parent/other). The `invalidatingClass` parameter on `invalidateExpression()` and `shouldInvalidateExpression()` enables skipping invalidation for private properties whose declaring class differs from the method's declaring class. This is checked via `isPrivatePropertyOfDifferentClass()`. The pattern mirrors the existing readonly property protection (`isReadonlyPropertyFetch`). Both `NodeScopeResolver` call sites (instance method calls at ~line 3188, static method calls at ~line 3398) pass `$methodReflection->getDeclaringClass()` as the invalidating class.
+
 ### Closure::bind() scope leaking into argument evaluation
 
 `NodeScopeResolver::processArgs()` has special handling for `Closure::bind()` and `Closure::bindTo()` calls. When the first argument is a closure/arrow function literal, a `$closureBindScope` is created with `$this` rebound to the second argument's type, and this scope is used to process the closure body. However, this `$closureBindScope` must ONLY be applied when the first argument is actually an `Expr\Closure` or `Expr\ArrowFunction`. If the first argument is a general expression that returns a closure (e.g. `$this->hydrate()`), the expression itself must be evaluated in the original scope — otherwise `$this` in the expression gets incorrectly resolved as the bound object type instead of the current class. The condition at the `$scopeToPass` assignment must check the argument node type.

src/Analyser/MutatingScope.php

@@ -3460,7 +3460,7 @@ public function assignInitializedProperty(Type $fetchedOnType, string $propertyN
 		return $this->assignExpression(new PropertyInitializationExpr($propertyName), new MixedType(), new MixedType());
 	}
 
-	public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false): self
+	public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): self
 	{
 		$expressionTypes = $this->expressionTypes;
 		$nativeExpressionTypes = $this->nativeExpressionTypes;
@@ -3469,7 +3469,7 @@ public function invalidateExpression(Expr $expressionToInvalidate, bool $require
 
 		foreach ($expressionTypes as $exprString => $exprTypeHolder) {
 			$exprExpr = $exprTypeHolder->getExpr();
-			if (!$this->shouldInvalidateExpression($exprStringToInvalidate, $expressionToInvalidate, $exprExpr, $exprString, $requireMoreCharacters)) {
+			if (!$this->shouldInvalidateExpression($exprStringToInvalidate, $expressionToInvalidate, $exprExpr, $exprString, $requireMoreCharacters, $invalidatingClass)) {
 				continue;
 			}
 
@@ -3484,14 +3484,14 @@ public function invalidateExpression(Expr $expressionToInvalidate, bool $require
 				continue;
 			}
 			$firstExpr = $holders[array_key_first($holders)]->getTypeHolder()->getExpr();
-			if ($this->shouldInvalidateExpression($exprStringToInvalidate, $expressionToInvalidate, $firstExpr, $this->getNodeKey($firstExpr))) {
+			if ($this->shouldInvalidateExpression($exprStringToInvalidate, $expressionToInvalidate, $firstExpr, $this->getNodeKey($firstExpr), false, $invalidatingClass)) {
 				$invalidated = true;
 				continue;
 			}
 			foreach ($holders as $holder) {
 				$conditionalTypeHolders = $holder->getConditionExpressionTypeHolders();
 				foreach ($conditionalTypeHolders as $conditionalTypeHolderExprString => $conditionalTypeHolder) {
-					if ($this->shouldInvalidateExpression($exprStringToInvalidate, $expressionToInvalidate, $conditionalTypeHolder->getExpr(), $conditionalTypeHolderExprString)) {
+					if ($this->shouldInvalidateExpression($exprStringToInvalidate, $expressionToInvalidate, $conditionalTypeHolder->getExpr(), $conditionalTypeHolderExprString, false, $invalidatingClass)) {
 						$invalidated = true;
 						continue 3;
 					}
@@ -3524,7 +3524,7 @@ public function invalidateExpression(Expr $expressionToInvalidate, bool $require
 		);
 	}
 
-	private function shouldInvalidateExpression(string $exprStringToInvalidate, Expr $exprToInvalidate, Expr $expr, string $exprString, bool $requireMoreCharacters = false): bool
+	private function shouldInvalidateExpression(string $exprStringToInvalidate, Expr $exprToInvalidate, Expr $expr, string $exprString, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): bool
 	{
 		if ($requireMoreCharacters && $exprStringToInvalidate === $exprString) {
 			return false;
@@ -3570,9 +3570,33 @@ private function shouldInvalidateExpression(string $exprStringToInvalidate, Expr
 			return false;
 		}
 
+		if (
+			$invalidatingClass !== null
+			&& $requireMoreCharacters
+			&& $this->isPrivatePropertyOfDifferentClass($expr, $invalidatingClass)
+		) {
+			return false;
+		}
+
 		return true;
 	}
 
+	private function isPrivatePropertyOfDifferentClass(Expr $expr, ClassReflection $invalidatingClass): bool
+	{
+		if ($expr instanceof Expr\StaticPropertyFetch || $expr instanceof PropertyFetch) {
+			$propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this);
+			if ($propertyReflection === null) {
+				return false;
+			}
+			if (!$propertyReflection->isPrivate()) {
+				return false;
+			}
+			return $propertyReflection->getDeclaringClass()->getName() !== $invalidatingClass->getName();
+		}
+
+		return false;
+	}
+
 	private function invalidateMethodsOnExpression(Expr $expressionToInvalidate): self
 	{
 		$exprStringToInvalidate = null;

src/Analyser/NodeScopeResolver.php

@@ -3185,7 +3185,7 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
 			if ($methodReflection !== null) {
 				if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) {
 					$this->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage);
-					$scope = $scope->invalidateExpression($normalizedExpr->var, true);
+					$scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass());
 				}
 				if ($parametersAcceptor !== null && !$methodReflection->isStatic()) {
 					$selfOutType = $methodReflection->getSelfOutType();
@@ -3395,7 +3395,7 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
 				&& $scope->isInClass()
 				&& $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName())
 			) {
-				$scope = $scope->invalidateExpression(new Variable('this'), true);
+				$scope = $scope->invalidateExpression(new Variable('this'), true, $methodReflection->getDeclaringClass());
 			}
 
 			if (
@@ -7615,9 +7615,11 @@ private function isScopeConditionallyImpossible(MutatingScope $scope): bool
 		$boolVars = [];
 		foreach ($scope->getDefinedVariables() as $varName) {
 			$varType = $scope->getVariableType($varName);
-			if ($varType->isBoolean()->yes() && !$varType->isConstantScalarValue()->yes()) {
-				$boolVars[] = $varName;
+			if (!$varType->isBoolean()->yes() || $varType->isConstantScalarValue()->yes()) {
+				continue;
 			}
+
+			$boolVars[] = $varName;
 		}
 
 		if ($boolVars === []) {
@@ -7627,13 +7629,13 @@ private function isScopeConditionallyImpossible(MutatingScope $scope): bool
 		// Check if any boolean variable's both truth values lead to contradictions
 		foreach ($boolVars as $varName) {
 			$varExpr = new Variable($varName);
-			
+
 			$truthyScope = $scope->filterByTruthyValue($varExpr);
 			$truthyContradiction = $this->scopeHasNeverVariable($truthyScope, $boolVars);
 			if (!$truthyContradiction) {
 				continue;
 			}
-			
+
 			$falseyScope = $scope->filterByFalseyValue($varExpr);
 			$falseyContradiction = $this->scopeHasNeverVariable($falseyScope, $boolVars);
 			if ($falseyContradiction) {

src/Analyser/SpecifiedTypes.php

@@ -5,6 +5,8 @@
 use PhpParser\Node\Expr;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeCombinator;
+use function array_key_exists;
+use function array_merge;
 
 final class SpecifiedTypes
 {

tests/PHPStan/Analyser/nsrt/bug-13736.php

@@ -0,0 +1,42 @@
+<?php declare(strict_types = 1);
+
+namespace Bug13736;
+
+use function PHPStan\Testing\assertType;
+
+class BaseClass {
+	final public static function assertTrue(mixed $condition, string $message = ''): void
+	{
+
+	}
+
+	public function doSomething(): void
+	{
+
+	}
+}
+
+class Bug13736Test extends BaseClass
+{
+	private static ?Foo $foo = null;
+	private ?Foo $instanceFoo = null;
+
+	public function testStaticProperty(): void
+	{
+		self::$foo = new Foo();
+		assertType('Bug13736\Foo', self::$foo);
+		self::assertTrue(true);
+		assertType('Bug13736\Foo', self::$foo);
+	}
+
+	public function testInstanceProperty(): void
+	{
+		$this->instanceFoo = new Foo();
+		assertType('Bug13736\Foo', $this->instanceFoo);
+		parent::doSomething();
+		assertType('Bug13736\Foo', $this->instanceFoo);
+	}
+}
+
+class Foo {
+}

tests/PHPStan/Analyser/nsrt/bug-6623.php

@@ -0,0 +1,24 @@
+<?php declare(strict_types = 1);
+
+namespace Bug6623;
+
+use function PHPStan\Testing\assertType;
+
+class World {
+	protected function bar(): void {}
+}
+
+class HelloWorld extends World
+{
+	private ?int $foo = null;
+
+	public function useFoo(): void
+	{
+		if (is_null($this->foo)) {
+			throw new \LogicException();
+		}
+		assertType('int', $this->foo);
+		$this->bar();
+		assertType('int', $this->foo);
+	}
+}