From 70b43f294e9496109fdeb0e6499a047b50ef794e Mon Sep 17 00:00:00 2001 From: ondrejmirtes <104888+ondrejmirtes@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:30:33 +0000 Subject: [PATCH 1/2] Don't invalidate private property expressions of different classes - When a method with side effects is called, private properties that the method's declaring class cannot access should not be invalidated - Added invalidatingClass parameter to invalidateExpression() and shouldInvalidateExpression() in MutatingScope - Added isPrivatePropertyOfDifferentClass() to check if a property expression refers to a private property declared in a different class than the one whose method triggered the invalidation - New regression test in tests/PHPStan/Analyser/nsrt/bug-13736.php Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 4 +++ src/Analyser/MutatingScope.php | 34 +++++++++++++++--- src/Analyser/NodeScopeResolver.php | 14 ++++---- src/Analyser/SpecifiedTypes.php | 2 ++ tests/PHPStan/Analyser/nsrt/bug-13736.php | 42 +++++++++++++++++++++++ 5 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-13736.php diff --git a/CLAUDE.md b/CLAUDE.md index c2df251460..8f7314026e 100644 --- a/CLAUDE.md +++ b/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. diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 67e6009761..b81f89088c 100644 --- a/src/Analyser/MutatingScope.php +++ b/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; diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 79ea0e8344..828e2685cf 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/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) { diff --git a/src/Analyser/SpecifiedTypes.php b/src/Analyser/SpecifiedTypes.php index 7fb3381ae5..ccb2c5e8a3 100644 --- a/src/Analyser/SpecifiedTypes.php +++ b/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 { diff --git a/tests/PHPStan/Analyser/nsrt/bug-13736.php b/tests/PHPStan/Analyser/nsrt/bug-13736.php new file mode 100644 index 0000000000..fe237d8df9 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13736.php @@ -0,0 +1,42 @@ +instanceFoo = new Foo(); + assertType('Bug13736\Foo', $this->instanceFoo); + parent::doSomething(); + assertType('Bug13736\Foo', $this->instanceFoo); + } +} + +class Foo { +} From fad2dc1a9bcdca207262581b64b35e2c60647168 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Thu, 12 Feb 2026 14:38:17 +0000 Subject: [PATCH 2/2] Add regression test for #6623 Closes https://github.com/phpstan/phpstan/issues/6623 --- tests/PHPStan/Analyser/nsrt/bug-6623.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-6623.php diff --git a/tests/PHPStan/Analyser/nsrt/bug-6623.php b/tests/PHPStan/Analyser/nsrt/bug-6623.php new file mode 100644 index 0000000000..e542f08949 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-6623.php @@ -0,0 +1,24 @@ +foo)) { + throw new \LogicException(); + } + assertType('int', $this->foo); + $this->bar(); + assertType('int', $this->foo); + } +}