From 4d094b3d23458676c9825c657cd3b028bea870b5 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Sun, 21 Jun 2026 10:57:30 +0000 Subject: [PATCH] Generalize by-reference closure `use` variables that are reassigned in the enclosing scope after the closure is declared - Add `ClosureUseByRefReassignmentVisitor` (a RichParser node visitor) that marks `use (&$var)` closure uses whose captured variable is reassigned in the enclosing scope at a program point after the closure was declared. It tracks per-scope frames (one per function-like) and compares document order of closure declarations against assignments (`=`, `.=` and friends, `++`/`--`, and array-element writes) to the same variable. - `MutatingScope::processClosureScope()` gains a `$generalizeByRefType` flag; when set (initial closure-body scope setup), it generalizes the captured variable's type via `GeneralizePrecision::lessSpecific()` but only for uses flagged by the visitor. Variables not reassigned externally keep their precise declaration-time type, so existing behavior (e.g. body-local fixed-point tracking) is preserved. - This prevents the closure body from assuming a by-reference captured variable still holds its declaration-time value, since the closure may be invoked after the variable was reassigned. - Covers analogous reassignment forms: plain `=`, compound assignment, in/decrement, array-element writes, conditional reassignment, and reassignment with a different type. --- src/Analyser/MutatingScope.php | 9 + src/Analyser/NodeScopeResolver.php | 2 +- .../ClosureUseByRefReassignmentVisitor.php | 174 ++++++++++++++++++ tests/PHPStan/Analyser/nsrt/bug-14848.php | 106 +++++++++++ 4 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 src/Parser/ClosureUseByRefReassignmentVisitor.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14848.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index e02c18b0da3..4eca98d4c17 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -40,6 +40,7 @@ use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Node\VirtualNode; use PHPStan\Parser\ArrayMapArgVisitor; +use PHPStan\Parser\ClosureUseByRefReassignmentVisitor; use PHPStan\Parser\Parser; use PHPStan\Php\PhpVersion; use PHPStan\Php\PhpVersionFactory; @@ -4062,6 +4063,7 @@ public function processClosureScope( self $closureScope, ?self $prevScope, array $byRefUses, + bool $generalizeByRefType = false, ): self { $nativeExpressionTypes = $this->nativeExpressionTypes; @@ -4087,6 +4089,13 @@ public function processClosureScope( $variableType = $closureScope->getVariableType($variableName); + if ( + $generalizeByRefType + && $use->getAttribute(ClosureUseByRefReassignmentVisitor::ATTRIBUTE_NAME) === true + ) { + $variableType = $variableType->generalize(GeneralizePrecision::lessSpecific()); + } + if ($prevScope !== null) { $prevVariableType = $prevScope->getVariableType($variableName); if (!$variableType->equals($prevVariableType)) { diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 3f63434d03f..2eb154e3e71 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2983,7 +2983,7 @@ public function processClosureNode( } $closureScope = $scope->enterAnonymousFunction($expr, $callableParameters, $nativeCallableParameters); - $closureScope = $closureScope->processClosureScope($scope, null, $byRefUses); + $closureScope = $closureScope->processClosureScope($scope, null, $byRefUses, true); $closureType = $closureScope->getAnonymousFunctionReflection(); if (!$closureType instanceof ClosureType) { throw new ShouldNotHappenException(); diff --git a/src/Parser/ClosureUseByRefReassignmentVisitor.php b/src/Parser/ClosureUseByRefReassignmentVisitor.php new file mode 100644 index 00000000000..243a0a83da5 --- /dev/null +++ b/src/Parser/ClosureUseByRefReassignmentVisitor.php @@ -0,0 +1,174 @@ +}>, assignments: array>}> */ + private array $frames = []; + + private int $order = 0; + + #[Override] + public function beforeTraverse(array $nodes): ?array + { + $this->frames = [self::createFrame()]; + $this->order = 0; + + return null; + } + + #[Override] + public function enterNode(Node $node): ?Node + { + $this->order++; + + if ($node instanceof Node\Expr\Closure) { + $byRefUses = []; + foreach ($node->uses as $use) { + if (!$use->byRef) { + continue; + } + + $byRefUses[] = $use; + } + + if (count($byRefUses) > 0) { + $this->frames[count($this->frames) - 1]['closures'][] = [ + 'order' => $this->order, + 'uses' => $byRefUses, + ]; + } + + $this->frames[] = self::createFrame(); + + return null; + } + + if ( + $node instanceof Node\Expr\ArrowFunction + || $node instanceof Node\Stmt\Function_ + || $node instanceof Node\Stmt\ClassMethod + ) { + $this->frames[] = self::createFrame(); + + return null; + } + + $variableName = self::getReassignedVariableName($node); + if ($variableName !== null) { + $this->frames[count($this->frames) - 1]['assignments'][$variableName][] = $this->order; + } + + return null; + } + + #[Override] + public function leaveNode(Node $node): ?Node + { + if ( + $node instanceof Node\Expr\Closure + || $node instanceof Node\Expr\ArrowFunction + || $node instanceof Node\Stmt\Function_ + || $node instanceof Node\Stmt\ClassMethod + ) { + $frame = array_pop($this->frames); + if ($frame !== null) { + self::resolveFrame($frame); + } + } + + return null; + } + + #[Override] + public function afterTraverse(array $nodes): ?array + { + $frame = array_pop($this->frames); + if ($frame !== null) { + self::resolveFrame($frame); + } + + return null; + } + + /** + * @return array{closures: list}>, assignments: array>} + */ + private static function createFrame(): array + { + return ['closures' => [], 'assignments' => []]; + } + + /** + * @param array{closures: list}>, assignments: array>} $frame + */ + private static function resolveFrame(array $frame): void + { + foreach ($frame['closures'] as $closure) { + foreach ($closure['uses'] as $use) { + if (!is_string($use->var->name)) { + continue; + } + + $variableName = $use->var->name; + if (!isset($frame['assignments'][$variableName])) { + continue; + } + + foreach ($frame['assignments'][$variableName] as $assignmentOrder) { + if ($assignmentOrder > $closure['order']) { + $use->setAttribute(self::ATTRIBUTE_NAME, true); + break; + } + } + } + } + } + + private static function getReassignedVariableName(Node $node): ?string + { + if ( + !$node instanceof Node\Expr\Assign + && !$node instanceof Node\Expr\AssignRef + && !$node instanceof Node\Expr\AssignOp + && !$node instanceof Node\Expr\PreInc + && !$node instanceof Node\Expr\PostInc + && !$node instanceof Node\Expr\PreDec + && !$node instanceof Node\Expr\PostDec + ) { + return null; + } + + $var = $node->var; + while ($var instanceof Node\Expr\ArrayDimFetch) { + $var = $var->var; + } + + if ($var instanceof Node\Expr\Variable && is_string($var->name)) { + return $var->name; + } + + return null; + } + +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-14848.php b/tests/PHPStan/Analyser/nsrt/bug-14848.php new file mode 100644 index 00000000000..1e92d839de6 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14848.php @@ -0,0 +1,106 @@ +', $arr); + }; + + $arr[] = 'b'; + $myfunc(); +} + +// not reassigned -> value used inside is kept precise, body modifications still tracked +function bodyModificationsStillTracked(): void +{ + $counter = 0; + $myfunc = function () use (&$counter) { + assertType('int<0, max>', $counter); + $counter++; + }; + + $myfunc(); +}