From 01ba8ae5324b0db8559710889389ac7756fefe28 Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Sun, 1 Mar 2026 08:41:33 +0000 Subject: [PATCH] Fix type narrowing persisting across impure function calls - TypeSpecifier::createForExpr() checked if the top-level function call was pure before remembering its type, but did not check whether arguments contained impure sub-expressions - Added containsImpureCall() helper to recursively detect impure function/method/static calls within expression trees - Applied the check to FuncCall, MethodCall, and StaticCall branches in createForExpr() - Added regression test in tests/PHPStan/Analyser/nsrt/bug-13416.php Closes https://github.com/phpstan/phpstan/issues/13416 --- src/Analyser/TypeSpecifier.php | 103 ++++++++++++++++++++++ tests/PHPStan/Analyser/nsrt/bug-13416.php | 46 ++++++++++ 2 files changed, 149 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-13416.php diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 0949e9eaf9..1530e17b74 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -87,6 +87,7 @@ use function array_shift; use function count; use function in_array; +use function is_array; use function is_string; use function strtolower; use function substr; @@ -1972,6 +1973,12 @@ private function createForExpr( if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) { return new SpecifiedTypes([], []); } + + foreach ($expr->getArgs() as $arg) { + if ($this->containsImpureCall($arg->value, $scope)) { + return new SpecifiedTypes([], []); + } + } } if ( @@ -1992,6 +1999,24 @@ private function createForExpr( return new SpecifiedTypes([], []); } + + if ($this->containsImpureCall($expr->var, $scope)) { + if (isset($containsNull) && !$containsNull) { + return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); + } + + return new SpecifiedTypes([], []); + } + + foreach ($expr->getArgs() as $arg) { + if ($this->containsImpureCall($arg->value, $scope)) { + if (isset($containsNull) && !$containsNull) { + return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); + } + + return new SpecifiedTypes([], []); + } + } } if ( @@ -2017,6 +2042,16 @@ private function createForExpr( return new SpecifiedTypes([], []); } + + foreach ($expr->getArgs() as $arg) { + if ($this->containsImpureCall($arg->value, $scope)) { + if (isset($containsNull) && !$containsNull) { + return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); + } + + return new SpecifiedTypes([], []); + } + } } $sureTypes = []; @@ -2047,6 +2082,74 @@ private function createForExpr( return $types; } + private function containsImpureCall(Expr $expr, Scope $scope): bool + { + if ($expr instanceof FuncCall && $expr->name instanceof Name) { + if (!$this->reflectionProvider->hasFunction($expr->name, $scope)) { + return true; + } + $functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope); + $hasSideEffects = $functionReflection->hasSideEffects(); + if ($hasSideEffects->yes()) { + return true; + } + if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) { + return true; + } + } + + if ($expr instanceof MethodCall && $expr->name instanceof Node\Identifier) { + $calledOnType = $scope->getType($expr->var); + $methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->toString()); + if ( + $methodReflection === null + || $methodReflection->hasSideEffects()->yes() + || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) + ) { + return true; + } + } + + if ($expr instanceof StaticCall && $expr->name instanceof Node\Identifier) { + if ($expr->class instanceof Name) { + $calledOnType = $scope->resolveTypeByName($expr->class); + } else { + $calledOnType = $scope->getType($expr->class); + } + $methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->toString()); + if ( + $methodReflection === null + || $methodReflection->hasSideEffects()->yes() + || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) + ) { + return true; + } + } + + foreach ($expr->getSubNodeNames() as $name) { + $subNode = $expr->{$name}; + if ($subNode instanceof Expr) { + if ($this->containsImpureCall($subNode, $scope)) { + return true; + } + } + if (!is_array($subNode)) { + continue; + } + + foreach ($subNode as $item) { + if ($item instanceof Node\Arg && $this->containsImpureCall($item->value, $scope)) { + return true; + } + if ($item instanceof Expr && $this->containsImpureCall($item, $scope)) { + return true; + } + } + } + + return false; + } + 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 0000000000..5b6f294905 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13416.php @@ -0,0 +1,46 @@ + */ + private static array $storage = []; + + /** @phpstan-impure */ + public function insert(): void { + self::$storage[] = $this; + } + + /** + * @return array + * @phpstan-impure + */ + public static function find(): array { + return self::$storage; + } +} + +class TestCase { + public function testMinimalBug(): void { + $msg1 = new MyRecord(); + $msg1->insert(); + + assert( + count(MyRecord::find()) === 1, + 'should have 1 record initially' + ); + + $msg2 = new MyRecord(); + $msg2->insert(); + + assertType('array', MyRecord::find()); + assertType('int<0, max>', count(MyRecord::find())); + + assert( + count(MyRecord::find()) === 2, + 'should have 2 messages after adding one' + ); + } +}