Keep property fetches on the receiver when a static method or static closure invalidates it

build/PHPStan/Build/TurboAttributeCollector.php

@@ -14,13 +14,15 @@
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\New_;
+use PhpParser\Node\Expr\NullsafePropertyFetch;
 use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Expr\UnaryMinus;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Expr\Yield_;
 use PhpParser\Node\Expr\YieldFrom;
 use PhpParser\Node\FunctionLike;
+use PhpParser\Node\Identifier;
 use PhpParser\Node\Name;
 use PhpParser\Node\Scalar;
 use PhpParser\Node\Stmt;
@@ -88,6 +90,8 @@ final class TurboAttributeCollector
 		'name' => Name::class,
 		'expr' => Expr::class,
 		'propertyFetch' => PropertyFetch::class,
+		'nullsafePropertyFetch' => NullsafePropertyFetch::class,
+		'identifier' => Identifier::class,
 		'arrayDimFetch' => ArrayDimFetch::class,
 		'methodCall' => MethodCall::class,
 		'functionLike' => FunctionLike::class,

src/Analyser/ExprHandler/Helper/FuncCallScopeEffectsHelper.php

@@ -17,6 +17,7 @@
 use PHPStan\DependencyInjection\AutowiredService;
 use PHPStan\Node\Expr\NativeTypeExpr;
 use PHPStan\Node\Expr\PossiblyImpureCallExpr;
+use PHPStan\Node\InvalidateExprNode;
 use PHPStan\Reflection\Callables\CallableParametersAcceptor;
 use PHPStan\Reflection\FunctionReflection;
 use PHPStan\Reflection\ParametersAcceptor;
@@ -33,6 +34,7 @@
 use PHPStan\Type\IntersectionType;
 use PHPStan\Type\MixedType;
 use PHPStan\Type\NullType;
+use PHPStan\Type\ResourceType;
 use PHPStan\Type\StringType;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeCombinator;
@@ -96,7 +98,15 @@ public function applyCallScopeEffects(NodeScopeResolver $nodeScopeResolver, Stmt
 			$parametersAcceptor instanceof ClosureType && count($parametersAcceptor->getImpurePoints()) > 0
 			&& $scope->isInClass()
 		) {
-			$scope = $scope->invalidateExpression(new Variable('this'), true);
+			// a static closure is never bound to $this, so property fetches on it survive
+			$keepPropertyFetches = $parametersAcceptor->isStaticClosure()->yes();
+			if ($keepPropertyFetches) {
+				// ... but the object can still be handed to it as an argument. That's the
+				// same channel processArgs() invalidates for a callee with side effects,
+				// which is what keeps '$this' invalidated for 'self::mutate($this)'.
+				$scope = $this->invalidateObjectArgs($nodeScopeResolver, $normalizedExpr, $scope, $storage, $nodeCallback);
+			}
+			$scope = $scope->invalidateExpression(new Variable('this'), true, null, $keepPropertyFetches);
 		}
 
 		if (
@@ -354,6 +364,31 @@ public function applyCallScopeEffects(NodeScopeResolver $nodeScopeResolver, Stmt
 		return $scope;
 	}
 
+	/**
+	 * Invalidates the arguments a callee could write through, mirroring what
+	 * NodeScopeResolver::processArgs() does for a callee with side effects. A
+	 * closure has no FunctionReflection, so processArgs() skips it.
+	 *
+	 * @param callable(Node $node, Scope $scope): void $nodeCallback
+	 */
+	private function invalidateObjectArgs(NodeScopeResolver $nodeScopeResolver, FuncCall $normalizedExpr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback): MutatingScope
+	{
+		foreach ($normalizedExpr->getArgs() as $arg) {
+			$argType = $scope->getType($arg->value);
+			if (
+				$argType->isObject()->no()
+				&& (new ResourceType())->isSuperTypeOf($argType)->no()
+			) {
+				continue;
+			}
+
+			$nodeScopeResolver->callNodeCallback($nodeCallback, new InvalidateExprNode($arg->value), $scope, $storage);
+			$scope = $scope->invalidateExpression($arg->value, true);
+		}
+
+		return $scope;
+	}
+
 	private function getArrayFunctionAppendingType(FunctionReflection $functionReflection, Scope $scope, FuncCall $expr): Type
 	{
 		$arrayArg = $expr->getArgs()[0]->value;

src/Analyser/ExprHandler/MethodCallHandler.php

@@ -175,7 +175,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 
 			if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) {
 				$nodeScopeResolver->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage);
-				$scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass());
+				$scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass(), $methodReflection->isStatic());
 			} elseif ($this->rememberPossiblyImpureFunctionValues && $methodReflection->hasSideEffects()->maybe() && !$methodReflection->getDeclaringClass()->isBuiltin()) {
 				// the remembered call value and the @phpstan-self-out type are
 				// generic-sensitive: resolve them from the type-driven acceptor

src/Analyser/ExprHandler/StaticCallHandler.php

@@ -256,7 +256,8 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 			&& $scope->isInClass()
 			&& $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName())
 		) {
-			$scope = $scope->invalidateExpression(new Variable('this'), true, $methodReflection->getDeclaringClass());
+			// a static method never receives $this, so property fetches on it survive
+			$scope = $scope->invalidateExpression(new Variable('this'), true, $methodReflection->getDeclaringClass(), $methodReflection->isStatic());
 		} elseif (
 			$expr->class instanceof Name
 			&& $methodReflection !== null

src/Analyser/MutatingScope.php

@@ -1189,7 +1189,7 @@
 			}
 
 			if ($result === null) {
 				if ($hasVariable->yes()) {
 					if ($expr->name === '_SESSION') {
 						return null;
 					}
@@ -1254,8 +1254,8 @@
 				return null;
 			}
 
 			if ($propertyReflection->hasNativeType() && !$propertyReflection->isVirtual()->yes()) {
 				if (!$this->hasExpressionType($expr)->yes()) {
 					$nativeReflection = $propertyReflection->getNativeReflection();
 					if (
 						($nativeReflection === null || !$nativeReflection->getNativeReflection()->hasDefaultValue())
@@ -1320,7 +1320,7 @@
 
 		if ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) {
 			$type = $this->getType($expr->var);
 			if (!$type->isOffsetAccessible()->yes()) {
 				return $this->issetCheckUndefined($expr->var);
 			}
 
@@ -3261,7 +3261,12 @@
 		return $scope;
 	}
 
-	public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): self
+	/**
+	 * @param bool $keepPropertyFetches Keeps property fetches on the invalidated expression
+	 *                                  (like '$this->foo') - for callees that never receive
+	 *                                  the object, like static methods and static closures.
+	 */
+	public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null, bool $keepPropertyFetches = false): self
 	{
 		$exprStringToInvalidate = $this->getNodeKey($expressionToInvalidate);
 
@@ -3275,6 +3280,7 @@
 			$this->expressionTypes,
 			$this->nativeExpressionTypes,
 			$this->conditionalExpressions,
+			$keepPropertyFetches,
 		);
 		if ($result === null) {
 			return $this;

src/Analyser/NodeCallbackScope.php

@@ -250,12 +250,12 @@ public function assignExpression(Expr $expr, Type $type, Type $nativeType): self
 		return $scope;
 	}
 
-	public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): self
+	public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null, bool $keepPropertyFetches = false): self
 	{
 		/** @var self $scope */
-		$scope = parent::invalidateExpression($expressionToInvalidate, $requireMoreCharacters, $invalidatingClass);
+		$scope = parent::invalidateExpression($expressionToInvalidate, $requireMoreCharacters, $invalidatingClass, $keepPropertyFetches);
 		$scope->scopeOps = $this->scopeOps;
-		$scope->scopeOps[] = static fn (MutatingScope $scope): MutatingScope => $scope->invalidateExpression($expressionToInvalidate, $requireMoreCharacters, $invalidatingClass);
+		$scope->scopeOps[] = static fn (MutatingScope $scope): MutatingScope => $scope->invalidateExpression($expressionToInvalidate, $requireMoreCharacters, $invalidatingClass, $keepPropertyFetches);
 
 		return $scope;
 	}

src/Analyser/ScopeOps.php

@@ -6,6 +6,7 @@
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\NullsafePropertyFetch;
 use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Name;
@@ -605,6 +606,7 @@ public static function invalidateExpressionEntries(
 		array $expressionTypes,
 		array $nativeExpressionTypes,
 		array $conditionalExpressions,
+		bool $keepPropertyFetches = false,
 	): ?array
 	{
 		$invalidated = false;
@@ -627,7 +629,7 @@ public static function invalidateExpressionEntries(
 				continue;
 			}
 			$exprExpr = $exprTypeHolder->getExpr();
-			if (!self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $exprExpr, $exprString, $requireMoreCharacters, $invalidatingClass)) {
+			if (!self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $exprExpr, $exprString, $requireMoreCharacters, $invalidatingClass, $keepPropertyFetches)) {
 				continue;
 			}
 
@@ -651,7 +653,7 @@ public static function invalidateExpressionEntries(
 				|| self::keyMayHideSubExpressions($conditionalExprString)
 			) {
 				$firstExpr = $holders[array_key_first($holders)]->getTypeHolder()->getExpr();
-				if (self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $firstExpr, $conditionalExprString, $requireMoreCharacters, $invalidatingClass)) {
+				if (self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $firstExpr, $conditionalExprString, $requireMoreCharacters, $invalidatingClass, $keepPropertyFetches)) {
 					$invalidated = true;
 					continue;
 				}
@@ -681,7 +683,7 @@ public static function invalidateExpressionEntries(
 				$shouldKeep = true;
 				$conditionalTypeHolders = $holder->getConditionExpressionTypeHolders();
 				foreach ($conditionalTypeHolders as $conditionalTypeHolderExprString => $conditionalTypeHolder) {
-					if (self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $conditionalTypeHolder->getExpr(), (string) $conditionalTypeHolderExprString, invalidatingClass: $invalidatingClass)) {
+					if (self::shouldInvalidateExpression($scope, $exprPrinter, $exprStringToInvalidate, $expressionToInvalidate, $conditionalTypeHolder->getExpr(), (string) $conditionalTypeHolderExprString, invalidatingClass: $invalidatingClass, keepPropertyFetches: $keepPropertyFetches)) {
 						$invalidated = true;
 						$shouldKeep = false;
 						break;
@@ -802,7 +804,7 @@ private static function keyMayHideSubExpressions(string $exprString): bool
 	/**
 	 * Mirrors the former MutatingScope::shouldInvalidateExpression().
 	 */
-	public static function shouldInvalidateExpression(MutatingScope $scope, ExprPrinter $exprPrinter, string $exprStringToInvalidate, Expr $exprToInvalidate, Expr $expr, string $exprString, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): bool
+	public static function shouldInvalidateExpression(MutatingScope $scope, ExprPrinter $exprPrinter, string $exprStringToInvalidate, Expr $exprToInvalidate, Expr $expr, string $exprString, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null, bool $keepPropertyFetches = false): bool
 	{
 		if (
 			$expr instanceof IntertwinedVariableByReferenceWithExpr
@@ -834,6 +836,10 @@ public static function shouldInvalidateExpression(MutatingScope $scope, ExprPrin
 			return $exprStringToInvalidate === $exprString;
 		}
 
+		if ($keepPropertyFetches && self::isPropertyFetchChainOn($expr, $exprStringToInvalidate, $exprPrinter)) {
+			return false;
+		}
+
 		// getNodeKey() is the pretty-printed expression, and the standard printer is
 		// compositional: the key of any sub-expression appears verbatim as a substring of
 		// the key of the expression containing it. So if the invalidated expression's key
@@ -878,6 +884,42 @@ public static function shouldInvalidateExpression(MutatingScope $scope, ExprPrin
 		return true;
 	}
 
+	/**
+	 * Whether $expr is a chain of property fetches rooted at the invalidated
+	 * expression, like '$this->foo', '$this->foo?->bar' or '$this->$name' for '$this'.
+	 *
+	 * Such an expression only reads state reachable through the object itself, so a
+	 * callee that never receives the object - a static method, a static closure -
+	 * cannot change it and it survives the invalidation. Anything else rooted at the
+	 * object (a method call, an offset access) can observe static state and keeps
+	 * being invalidated. A property name computed from anything but a plain variable
+	 * could do the same, so it is not accepted either.
+	 *
+	 * Callers must still invalidate the object when they hand it to the callee as an
+	 * argument. Reaching it through static state ('self::$instance = $this;' and then
+	 * a static method writing through 'self::$instance') is not tracked - the same
+	 * limitation every receiver other than '$this' has always had.
+	 */
+	private static function isPropertyFetchChainOn(Expr $expr, string $exprStringToInvalidate, ExprPrinter $exprPrinter): bool
+	{
+		if (!$expr instanceof PropertyFetch && !$expr instanceof NullsafePropertyFetch) {
+			return false;
+		}
+
+		while ($expr instanceof PropertyFetch || $expr instanceof NullsafePropertyFetch) {
+			if (
+				!$expr->name instanceof Node\Identifier
+				&& !($expr->name instanceof Variable && is_string($expr->name->name))
+			) {
+				return false;
+			}
+
+			$expr = $expr->var;
+		}
+
+		return self::nodeKey($expr, $exprPrinter) === $exprStringToInvalidate;
+	}
+
 	public static function getIntertwinedRefRootVariableName(Expr $expr): ?string
 	{
 		if ($expr instanceof Variable && is_string($expr->name)) {

src/Turbo/TurboExtensionEnabler.php

@@ -22,7 +22,7 @@ final class TurboExtensionEnabler
 	 * version is the short SHA of the last commit touching turbo-ext/src/,
 	 * enforced by the phar.yml turbo-version job.
 	 */
-	public const EXPECTED_EXTENSION_VERSION = 'e8e7544';
+	public const EXPECTED_EXTENSION_VERSION = 'e5a7514';
 
 	private static bool $typeCombinatorCacheEnabled = false;
 

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

@@ -0,0 +1,84 @@
+<?php // lint >= 8.0
+
+declare(strict_types = 1);
+
+namespace Bug13735;
+
+use function PHPStan\Testing\assertType;
+
+class Bug13735Test
+{
+	private ?Foo $foo = null;
+
+	public function testFoo(): void
+	{
+		$this->foo = new Foo();
+		assertType('Bug13735\Foo', $this->foo);
+		self::assertTrue(true);
+		assertType('Bug13735\Foo', $this->foo);
+	}
+
+	public static function assertTrue(mixed $condition, string $message = ''): void
+	{
+
+	}
+}
+
+class Foo {
+	public ?Foo $inner = null;
+
+	public function doSomething(): bool {
+		return true;
+	}
+}
+
+class Test
+{
+	private string $data;
+
+	public function __construct() {
+		$this->data = 'abc';
+		assertType("'abc'", $this->data);
+		self::noop('foo');
+		assertType("'abc'", $this->data);
+	}
+
+	static final public function noop(string $message): void {
+		file_put_contents('log file', $message);
+	}
+
+}
+
+final class FinalTest
+{
+	private string $data;
+
+	public function __construct() {
+		$this->data = 'abc';
+		assertType("'abc'", $this->data);
+		self::noop('foo');
+		assertType("'abc'", $this->data);
+	}
+
+	static public function noop(string $message): void {
+		file_put_contents('log file', $message);
+	}
+
+}
+
+final class PrivateTest
+{
+	private string $data;
+
+	public function __construct() {
+		$this->data = 'abc';
+		assertType("'abc'", $this->data);
+		self::noop('foo');
+		assertType("'abc'", $this->data);
+	}
+
+	static private function noop(string $message): void {
+		file_put_contents('log file', $message);
+	}
+
+}