From deaafc79c93cf520e99857763e61ae2f60b9e81d Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Sat, 21 Mar 2026 10:31:03 +0000 Subject: [PATCH] Fix phpstan/phpstan#14001: Type loss in foreach when using StatementResult::getScope() - Changed StatementResult, StatementExitPoint, and ReturnStatement to store and return MutatingScope instead of Scope, since mergeWith() is only on MutatingScope and calling it on Scope produced ErrorType, poisoning the variable type across foreach loop iterations - Added toMutatingScope() calls in NodeScopeResolver where ReturnStatement is constructed from a Scope-typed callback parameter - Fixed a null check on getClassReflection() in NodeScopeResolver that was exposed by the more precise type tracking - Removed now-redundant toMutatingScope() calls on ReturnStatement::getScope() - New regression test in tests/PHPStan/Analyser/nsrt/bug-14001.php --- src/Analyser/NodeScopeResolver.php | 14 +++---- src/Analyser/StatementExitPoint.php | 4 +- src/Analyser/StatementResult.php | 4 +- src/Node/ReturnStatement.php | 6 +-- tests/PHPStan/Analyser/nsrt/bug-14001.php | 49 +++++++++++++++++++++++ 5 files changed, 63 insertions(+), 14 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14001.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 8935ade8e2..ef8852332e 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -646,7 +646,7 @@ public function processStmtNode( return; } - $gatheredReturnStatements[] = new ReturnStatement($scope, $node); + $gatheredReturnStatements[] = new ReturnStatement($scope->toMutatingScope(), $node); }, StatementContext::createTopLevel())->toPublic(); $this->callNodeCallback($nodeCallback, new FunctionReturnStatementsNode( @@ -804,7 +804,7 @@ public function processStmtNode( return; } - $gatheredReturnStatements[] = new ReturnStatement($scope, $node); + $gatheredReturnStatements[] = new ReturnStatement($scope->toMutatingScope(), $node); }, StatementContext::createTopLevel())->toPublic(); $methodReflection = $methodScope->getFunction(); @@ -842,11 +842,11 @@ public function processStmtNode( foreach ($gatheredReturnStatements as $statement) { if ($finalScope === null) { - $finalScope = $statement->getScope()->toMutatingScope(); + $finalScope = $statement->getScope(); continue; } - $finalScope = $finalScope->mergeWith($statement->getScope()->toMutatingScope()); + $finalScope = $finalScope->mergeWith($statement->getScope()); } if ($finalScope !== null) { @@ -855,7 +855,7 @@ public function processStmtNode( } } - if (!$scope->getClassReflection()->isAnonymous() && !$scope->isInAnonymousFunction()) { + if ($scope->getClassReflection() !== null && !$scope->getClassReflection()->isAnonymous() && !$scope->isInAnonymousFunction()) { $this->processPendingFibers($storage); } } elseif ($stmt instanceof Echo_) { @@ -2763,7 +2763,7 @@ public function processClosureNode( return; } - $gatheredReturnStatements[] = new ReturnStatement($scope, $node); + $gatheredReturnStatements[] = new ReturnStatement($scope->toMutatingScope(), $node); }; if (count($byRefUses) === 0) { @@ -3142,7 +3142,7 @@ private function processPropertyHooks( return; } - $gatheredReturnStatements[] = new ReturnStatement($scope, $node); + $gatheredReturnStatements[] = new ReturnStatement($scope->toMutatingScope(), $node); }, StatementContext::createTopLevel())->toPublic(); $this->callNodeCallback($nodeCallback, new PropertyHookReturnStatementsNode( diff --git a/src/Analyser/StatementExitPoint.php b/src/Analyser/StatementExitPoint.php index aa36fd2346..5c4916373e 100644 --- a/src/Analyser/StatementExitPoint.php +++ b/src/Analyser/StatementExitPoint.php @@ -10,7 +10,7 @@ final class StatementExitPoint { - public function __construct(private Stmt $statement, private Scope $scope) + public function __construct(private Stmt $statement, private MutatingScope $scope) { } @@ -19,7 +19,7 @@ public function getStatement(): Stmt return $this->statement; } - public function getScope(): Scope + public function getScope(): MutatingScope { return $this->scope; } diff --git a/src/Analyser/StatementResult.php b/src/Analyser/StatementResult.php index 33bc973a4a..dad528dc18 100644 --- a/src/Analyser/StatementResult.php +++ b/src/Analyser/StatementResult.php @@ -18,7 +18,7 @@ final class StatementResult * @param EndStatementResult[] $endStatements */ public function __construct( - private Scope $scope, + private MutatingScope $scope, private bool $hasYield, private bool $isAlwaysTerminating, private array $exitPoints, @@ -29,7 +29,7 @@ public function __construct( { } - public function getScope(): Scope + public function getScope(): MutatingScope { return $this->scope; } diff --git a/src/Node/ReturnStatement.php b/src/Node/ReturnStatement.php index 153faf1534..3c48212d01 100644 --- a/src/Node/ReturnStatement.php +++ b/src/Node/ReturnStatement.php @@ -4,7 +4,7 @@ use PhpParser\Node; use PhpParser\Node\Stmt\Return_; -use PHPStan\Analyser\Scope; +use PHPStan\Analyser\MutatingScope; /** * @api @@ -14,12 +14,12 @@ final class ReturnStatement private Node\Stmt\Return_ $returnNode; - public function __construct(private Scope $scope, Return_ $returnNode) + public function __construct(private MutatingScope $scope, Return_ $returnNode) { $this->returnNode = $returnNode; } - public function getScope(): Scope + public function getScope(): MutatingScope { return $this->scope; } diff --git a/tests/PHPStan/Analyser/nsrt/bug-14001.php b/tests/PHPStan/Analyser/nsrt/bug-14001.php new file mode 100644 index 0000000000..60ab55a644 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14001.php @@ -0,0 +1,49 @@ + $executionEnds + */ + public function testWithEarlyTermination(array $executionEnds): void + { + $finalScope = null; + foreach ($executionEnds as $executionEnd) { + $statementResult = $executionEnd->getStatementResult(); + if ($statementResult->isAlwaysTerminating()) { + continue; + } + if ($finalScope === null) { + $finalScope = $statementResult->getScope(); + continue; + } + + $finalScope = $finalScope->mergeWith($statementResult->getScope()); + } + assertType('PHPStan\Analyser\MutatingScope|null', $finalScope); + } + + /** + * @param list $executionEnds + */ + public function testWithoutEarlyTermination(array $executionEnds): void + { + $finalScope = null; + foreach ($executionEnds as $executionEnd) { + $endScope = $executionEnd->getStatementResult()->getScope(); + if ($finalScope === null) { + $finalScope = $endScope; + continue; + } + + $finalScope = $finalScope->mergeWith($endScope); + } + assertType('PHPStan\Analyser\MutatingScope|null', $finalScope); + } +}