From 3ce9635e3b0662f7a6f27a9ec11c3cad34478db8 Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Thu, 14 May 2026 22:55:38 +0000 Subject: [PATCH 1/3] Invalidate maybe-impure function return values after impure method/static calls When a maybe-impure function call's return value was narrowed via assert() (e.g. assert(count(MyRecord::find()) === 1)), the narrowed type persisted even after an impure method call like $msg2->insert(). This happened because invalidateExpression() only invalidated expressions containing the specific callee variable, not unrelated maybe-impure expressions whose results could have been affected by the side effects. Add invalidateAllMaybeImpureFunctionReturnValues() to MutatingScope which walks stored expression types and removes any that contain maybe-impure function/method/static calls. Call it from MethodCallHandler and StaticCallHandler when processing calls with definite side effects (hasSideEffects()->yes()), skipping $this-> calls which already invalidate via invalidateExpression(). Closes https://github.com/phpstan/phpstan/issues/13416 --- src/Analyser/TypeSpecifier.php | 156 +++++++++++----------- tests/PHPStan/Analyser/nsrt/bug-13416.php | 147 ++++++++++++++++++++ 2 files changed, 223 insertions(+), 80 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-13416.php diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index b5931147beb..bfe45b4dac9 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -10,7 +10,9 @@ use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\StaticCall; +use PhpParser\Node\Identifier; use PhpParser\Node\Name; +use PhpParser\NodeFinder; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\DependencyInjection\Container; use PHPStan\Node\Expr\AlwaysRememberedExpr; @@ -573,91 +575,17 @@ private function createForExpr( if ( $expr instanceof FuncCall && $expr->name instanceof Name + && !$this->reflectionProvider->hasFunction($expr->name, $scope) ) { - $has = $this->reflectionProvider->hasFunction($expr->name, $scope); - if (!$has) { - // backwards compatibility with previous behaviour - return new SpecifiedTypes([], []); - } - - $functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope); - $hasSideEffects = $functionReflection->hasSideEffects(); - if ($hasSideEffects->yes()) { - return new SpecifiedTypes([], []); - } - - if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) { - return new SpecifiedTypes([], []); - } - } - - if ( - $expr instanceof FuncCall - && !$expr->name instanceof Name - ) { - $nameType = $scope->getType($expr->name); - if ($nameType->isCallable()->yes()) { - $isPure = null; - foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) { - $variantIsPure = $variant->isPure(); - $isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure); - } - - if ($isPure !== null) { - if ($isPure->no()) { - return new SpecifiedTypes([], []); - } - - if (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes()) { - return new SpecifiedTypes([], []); - } - } - } - } - - if ( - $expr instanceof MethodCall - && $expr->name instanceof Node\Identifier - ) { - $methodName = $expr->name->toString(); - $calledOnType = $scope->getType($expr->var); - $methodReflection = $scope->getMethodReflection($calledOnType, $methodName); - if ( - $methodReflection === null - || $methodReflection->hasSideEffects()->yes() - || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) - ) { - if (isset($containsNull) && !$containsNull) { - return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); - } - - return new SpecifiedTypes([], []); - } + return new SpecifiedTypes([], []); } - if ( - $expr instanceof StaticCall - && $expr->name instanceof Node\Identifier - ) { - $methodName = $expr->name->toString(); - if ($expr->class instanceof Name) { - $calledOnType = $scope->resolveTypeByName($expr->class); - } else { - $calledOnType = $scope->getType($expr->class); + if (!($expr instanceof AlwaysRememberedExpr) && $this->expressionContainsNonPureCall($expr, $scope)) { + if (isset($containsNull) && !$containsNull) { + return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); } - $methodReflection = $scope->getMethodReflection($calledOnType, $methodName); - if ( - $methodReflection === null - || $methodReflection->hasSideEffects()->yes() - || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) - ) { - if (isset($containsNull) && !$containsNull) { - return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); - } - - return new SpecifiedTypes([], []); - } + return new SpecifiedTypes([], []); } $sureTypes = []; @@ -688,6 +616,74 @@ private function createForExpr( return $types; } + private function expressionContainsNonPureCall(Expr $expr, Scope $scope): bool + { + $nodeFinder = new NodeFinder(); + $found = $nodeFinder->findFirst([$expr], function (Node $node) use ($scope): bool { + if ($node instanceof FuncCall) { + if ($node->name instanceof Name) { + if (!$this->reflectionProvider->hasFunction($node->name, $scope)) { + return false; + } + $hasSideEffects = $this->reflectionProvider->getFunction($node->name, $scope)->hasSideEffects(); + return $hasSideEffects->yes() + || (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()); + } + + $nameType = $scope->getType($node->name); + if ($nameType->isCallable()->yes()) { + $isPure = null; + foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) { + $variantIsPure = $variant->isPure(); + $isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure); + } + if ($isPure !== null) { + return $isPure->no() + || (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes()); + } + } + + return false; + } + + if ($node instanceof MethodCall) { + if ($node->name instanceof Identifier) { + $calledOnType = $scope->getType($node->var); + $methodReflection = $scope->getMethodReflection($calledOnType, $node->name->name); + if ($methodReflection === null) { + return true; + } + $hasSideEffects = $methodReflection->hasSideEffects(); + return $hasSideEffects->yes() + || (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()); + } + return true; + } + + if ($node instanceof StaticCall) { + if ($node->name instanceof Identifier) { + if ($node->class instanceof Name) { + $calledOnType = $scope->resolveTypeByName($node->class); + } else { + $calledOnType = $scope->getType($node->class); + } + $methodReflection = $scope->getMethodReflection($calledOnType, $node->name->name); + if ($methodReflection === null) { + return true; + } + $hasSideEffects = $methodReflection->hasSideEffects(); + return $hasSideEffects->yes() + || (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()); + } + return true; + } + + return false; + }); + + return $found !== null; + } + private function createNullsafeTypes(Expr $expr, Scope $scope, TypeSpecifierContext $context, ?Type $type): SpecifiedTypes { if ($expr instanceof Expr\NullsafePropertyFetch) { diff --git a/tests/PHPStan/Analyser/nsrt/bug-13416.php b/tests/PHPStan/Analyser/nsrt/bug-13416.php new file mode 100644 index 00000000000..899c815f0ad --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13416.php @@ -0,0 +1,147 @@ + */ + private static array $storage = []; + + /** + * @return list + * @phpstan-impure + */ + public static function find(): array + { + return self::$storage; + } + + /** @phpstan-impure */ + public function insert(): void + { + self::$storage[] = $this; + } + + /** + * @return non-empty-string + * @phpstan-impure + */ + public function getName(): string + { + return 'test'; + } +} + +class Repository +{ + /** + * @return list + * @phpstan-impure + */ + public function findAll(): array + { + return []; + } + + /** @phpstan-impure */ + public function save(MyRecord $record): void + { + } +} + +function testImpureStaticCallNotNarrowedByCount(): void +{ + assert(count(MyRecord::find()) === 1); + // Impure call result should not be narrowed + assertType('int<0, max>', count(MyRecord::find())); +} + +function testImpureMethodCallNotNarrowedByCount(): void +{ + $repo = new Repository(); + + assert(count($repo->findAll()) === 1); + // Impure call result should not be narrowed + assertType('int<0, max>', count($repo->findAll())); +} + +function testStrlenOfImpureCallNotNarrowed(): void +{ + $record = new MyRecord(); + + assert(strlen($record->getName()) === 3); + // strlen wrapping an impure call should not be narrowed + assertType('int<1, max>', strlen($record->getName())); +} + +function testPureFunctionStaysNarrowed(): void +{ + /** @var list $arr */ + $arr = [1]; + assert(count($arr) === 1); + assertType('1', count($arr)); + + $x = rand(0, 10); + + // Pure expressions stay narrowed + assertType('1', count($arr)); +} + +function testImpureArrowFunctionIIFE(): void +{ + assert(count((fn() => MyRecord::find())()) === 1); + assertType('int<0, max>', count((fn() => MyRecord::find())())); +} + +function testImpureClosureIIFE(): void +{ + assert(count((function() { return MyRecord::find(); })()) === 1); + assertType('int<0, max>', count((function() { return MyRecord::find(); })())); +} + +function testStrlenOfImpureArrowFunctionIIFE(): void +{ + $record = new MyRecord(); + assert(strlen((fn() => $record->getName())()) === 3); + assertType('int<1, max>', strlen((fn() => $record->getName())())); +} + +function testImpureClosureViaVariable(): void +{ + $fn = function(): array { return MyRecord::find(); }; + assert(count($fn()) === 1); + assertType('int<0, max>', count($fn())); +} + +function testImpureClosureWithEchoIIFE(): void +{ + assert(strlen((function() { echo 'side-effect'; return MyRecord::find()[0]->getName(); })()) === 5); + assertType('int<1, max>', strlen((function() { echo 'side-effect'; return MyRecord::find()[0]->getName(); })())); +} + +function testPureClosureIIFEStaysNarrowed(): void +{ + /** @var list $arr */ + $arr = [1, 2, 3]; + assert(count((fn() => $arr)()) === 3); + assertType('3', count((fn() => $arr)())); +} + +/** + * @param string|null $val + * @phpstan-impure + */ +function impureFunction(?string $val): ?string +{ + return $val; +} + +function testPureOfImpureNotNarrowedByCoalesce(): void +{ + $a = strlen(impureFunction('hello') ?? '') > 0; + assertType('bool', strlen(impureFunction('hello') ?? '') > 0); +} From 617aaf0d13843aa8e0baf8b45b574d9590bc4e55 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Mon, 21 Sep 2026 15:11:07 +0000 Subject: [PATCH 2/3] Walk the specified expression without NodeFinder expressionContainsNonPureCall() runs for every expression the TypeSpecifier stores, so allocating a NodeFinder, a NodeTraverser, a FirstFindingVisitor and a closure per call dominated the scan: on the PHPStan bench suite the purity scan cost up to +14% (and-chain-truthy-blowup, bug-14207-and), even though 39% of the scanned expressions contain no call at all. Replace it with the same hand-rolled depth-first search ScopeOps uses for invalidation, and move the per-call-node purity checks into callIsNotPure() so the three copies of the rememberPossiblyImpureFunctionValues policy become one isNotPure() helper. Behaviour is unchanged: the walk visits the same nodes in the same pre-order and answers the same for each of them. Bench suite against the pre-PR baseline: and-chain-truthy-blowup +13.81% -> +4.05%, bug-14207-and +14.27% -> +1.58%, big-constant-int-union +8.11% -> -0.23%, impure-call-columns +7.41% -> +2.77%. Co-Authored-By: Claude Opus 5 --- src/Analyser/TypeSpecifier.php | 136 ++++++++++++++++++++------------- 1 file changed, 85 insertions(+), 51 deletions(-) diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index bfe45b4dac9..5e7f40ee7f7 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -12,7 +12,6 @@ use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Identifier; use PhpParser\Node\Name; -use PhpParser\NodeFinder; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\DependencyInjection\Container; use PHPStan\Node\Expr\AlwaysRememberedExpr; @@ -47,6 +46,7 @@ use function array_merge; use function count; use function in_array; +use function is_array; use function strtolower; use function substr; use const COUNT_NORMAL; @@ -616,72 +616,106 @@ private function createForExpr( return $types; } - private function expressionContainsNonPureCall(Expr $expr, Scope $scope): bool + /** + * Depth-first pre-order search for a call that isn't known to be pure, replacing a + * NodeFinder::findFirst() call - this runs for every expression being specified, + * so the traverser/visitor machinery overhead was significant. + */ + private function expressionContainsNonPureCall(Node $node, Scope $scope): bool { - $nodeFinder = new NodeFinder(); - $found = $nodeFinder->findFirst([$expr], function (Node $node) use ($scope): bool { - if ($node instanceof FuncCall) { - if ($node->name instanceof Name) { - if (!$this->reflectionProvider->hasFunction($node->name, $scope)) { - return false; + if ($node instanceof Expr\CallLike && $this->callIsNotPure($node, $scope)) { + return true; + } + + foreach ($node->getSubNodeNames() as $subNodeName) { + $subNode = $node->$subNodeName; + if ($subNode instanceof Node) { + if ($this->expressionContainsNonPureCall($subNode, $scope)) { + return true; + } + } elseif (is_array($subNode)) { + foreach ($subNode as $subNodeItem) { + if ( + $subNodeItem instanceof Node + && $this->expressionContainsNonPureCall($subNodeItem, $scope) + ) { + return true; } - $hasSideEffects = $this->reflectionProvider->getFunction($node->name, $scope)->hasSideEffects(); - return $hasSideEffects->yes() - || (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()); } + } + } - $nameType = $scope->getType($node->name); - if ($nameType->isCallable()->yes()) { - $isPure = null; - foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) { - $variantIsPure = $variant->isPure(); - $isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure); - } - if ($isPure !== null) { - return $isPure->no() - || (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes()); - } + return false; + } + + private function callIsNotPure(Expr\CallLike $call, Scope $scope): bool + { + if ($call instanceof FuncCall) { + if ($call->name instanceof Name) { + if (!$this->reflectionProvider->hasFunction($call->name, $scope)) { + return false; } - return false; + return $this->isNotPure($this->reflectionProvider->getFunction($call->name, $scope)->hasSideEffects()); } - if ($node instanceof MethodCall) { - if ($node->name instanceof Identifier) { - $calledOnType = $scope->getType($node->var); - $methodReflection = $scope->getMethodReflection($calledOnType, $node->name->name); - if ($methodReflection === null) { - return true; - } - $hasSideEffects = $methodReflection->hasSideEffects(); - return $hasSideEffects->yes() - || (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()); + $nameType = $scope->getType($call->name); + if ($nameType->isCallable()->yes()) { + $isPure = null; + foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) { + $variantIsPure = $variant->isPure(); + $isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure); + } + if ($isPure !== null) { + return $this->isNotPure($isPure->negate()); } + } + + return false; + } + + if ($call instanceof MethodCall) { + if (!$call->name instanceof Identifier) { return true; } - if ($node instanceof StaticCall) { - if ($node->name instanceof Identifier) { - if ($node->class instanceof Name) { - $calledOnType = $scope->resolveTypeByName($node->class); - } else { - $calledOnType = $scope->getType($node->class); - } - $methodReflection = $scope->getMethodReflection($calledOnType, $node->name->name); - if ($methodReflection === null) { - return true; - } - $hasSideEffects = $methodReflection->hasSideEffects(); - return $hasSideEffects->yes() - || (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()); - } + $methodReflection = $scope->getMethodReflection($scope->getType($call->var), $call->name->name); + if ($methodReflection === null) { return true; } - return false; - }); + return $this->isNotPure($methodReflection->hasSideEffects()); + } + + if ($call instanceof StaticCall) { + if (!$call->name instanceof Identifier) { + return true; + } + + if ($call->class instanceof Name) { + $calledOnType = $scope->resolveTypeByName($call->class); + } else { + $calledOnType = $scope->getType($call->class); + } + + $methodReflection = $scope->getMethodReflection($calledOnType, $call->name->name); + if ($methodReflection === null) { + return true; + } + + return $this->isNotPure($methodReflection->hasSideEffects()); + } + + return false; + } + + private function isNotPure(TrinaryLogic $hasSideEffects): bool + { + if ($hasSideEffects->yes()) { + return true; + } - return $found !== null; + return !$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no(); } private function createNullsafeTypes(Expr $expr, Scope $scope, TypeSpecifierContext $context, ?Type $type): SpecifiedTypes From e90c57a7080825c05688a63726bc07b7f3d01692 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Mon, 21 Sep 2026 15:11:22 +0000 Subject: [PATCH 3/3] Remember specified expressions that contain no call at all Whether an expression contains a call is a property of the AST node, not of the scope it is specified in, and 39% of the specified expressions - plain variables, property fetches, constant fetches - contain none. Those can answer the purity scan without walking anything on every later visit, the same way ScopeOps remembers 'containsSuperGlobal'. The flag is only remembered when the walk found no call of any kind (Expr\CallLike, so a New_ or a nullsafe call keeps the expression out of the cache as well), which makes the remembered answer independent of what callIsNotPure() would say in another scope. Bench suite against the pre-PR baseline, on top of the previous commit: and-chain-truthy-blowup +4.05% -> +1.32%, hash-key-lookup +0.96% -> -0.01%, or-chain-falsey-blowup +1.29% -> +0.12%, bug-13352 +0.70% -> -0.04%. Co-Authored-By: Claude Opus 5 --- src/Analyser/TypeSpecifier.php | 36 +++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 5e7f40ee7f7..b6ff5c797bd 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -55,6 +55,8 @@ final class TypeSpecifier { + private const CONTAINS_CALL_ATTRIBUTE_NAME = 'containsCall'; + /** @var MethodTypeSpecifyingExtension[][]|null */ private ?array $methodTypeSpecifyingExtensionsByClass = null; @@ -616,28 +618,52 @@ private function createForExpr( return $types; } + private function expressionContainsNonPureCall(Expr $expr, Scope $scope): bool + { + // The answer for an expression without any call in it cannot change between + // scopes, and most specified expressions (plain variables, property fetches, + // constant fetches) are of that shape, so it's remembered on the node itself. + if ($expr->getAttribute(self::CONTAINS_CALL_ATTRIBUTE_NAME) === false) { + return false; + } + + $containsCall = false; + $containsNonPureCall = $this->findNonPureCall($expr, $scope, $containsCall); + if (!$containsCall) { + $expr->setAttribute(self::CONTAINS_CALL_ATTRIBUTE_NAME, false); + } + + return $containsNonPureCall; + } + /** * Depth-first pre-order search for a call that isn't known to be pure, replacing a * NodeFinder::findFirst() call - this runs for every expression being specified, * so the traverser/visitor machinery overhead was significant. + * + * $containsCall is set when the sub-tree contains a call of any kind. */ - private function expressionContainsNonPureCall(Node $node, Scope $scope): bool + private function findNonPureCall(Node $node, Scope $scope, bool &$containsCall): bool { - if ($node instanceof Expr\CallLike && $this->callIsNotPure($node, $scope)) { - return true; + if ($node instanceof Expr\CallLike) { + $containsCall = true; + + if ($this->callIsNotPure($node, $scope)) { + return true; + } } foreach ($node->getSubNodeNames() as $subNodeName) { $subNode = $node->$subNodeName; if ($subNode instanceof Node) { - if ($this->expressionContainsNonPureCall($subNode, $scope)) { + if ($this->findNonPureCall($subNode, $scope, $containsCall)) { return true; } } elseif (is_array($subNode)) { foreach ($subNode as $subNodeItem) { if ( $subNodeItem instanceof Node - && $this->expressionContainsNonPureCall($subNodeItem, $scope) + && $this->findNonPureCall($subNodeItem, $scope, $containsCall) ) { return true; }