From a02ef7ca25107ae07bf595585d651ce6c6a03388 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Fri, 31 Jul 2026 20:52:04 +0000 Subject: [PATCH] Widen by-ref `use` variables with the types they hold at calls and yields inside the closure body * `NodeScopeResolver::processClosureNode()` reached its by-ref `use` fixed point from the scope at the end of the closure body plus its exit points only, so a value assigned mid-body and overwritten before the body ended was invisible on the next entry. * The intermediary body walks now run with a `GatheringNodeCallback` that records each by-ref `use` variable's type (and native type) at every re-entry point, and those types are unioned into the intermediary scope before it is fed back through `processClosureScope()`. * Added `NodeScopeResolver::isClosureReentryPoint()`: non-first-class-callable `CallLike` nodes (function, method, nullsafe method, static method calls and `new`) plus `yield` / `yield from`, i.e. the places where control can leave the body and the closure can be entered again before the current invocation finishes. * Immediately invoked closures keep the old, more precise behaviour - they cannot be re-entered. * Same fix covers the sibling constructs, each verified to be broken before it: re-entry through a plain function call, a method call, a static method call, a constructor, a call made by a nested closure, a call inside a loop, the negated (`if (!$flag)`) direction, and generator suspension on `yield` / `yield from`. * Probed and found already correct: `global` variables (typed `mixed`), `static` variables (typed `mixed`), properties (no narrowing from the declared type), and arrow functions (cannot capture by reference). --- src/Analyser/NodeScopeResolver.php | 62 +++++++- tests/PHPStan/Analyser/nsrt/bug-15034.php | 22 +++ .../IfConstantConditionRuleTest.php | 12 ++ .../Rules/Comparison/data/bug-15034.php | 19 +++ .../data/closure-by-ref-use-reentry.php | 133 ++++++++++++++++++ 5 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-15034.php create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-15034.php create mode 100644 tests/PHPStan/Rules/Comparison/data/closure-by-ref-use-reentry.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index aefe7850be..556e10ee1d 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -3096,8 +3096,33 @@ public function processClosureNode( do { $prevScope = $closureScope; + /** @var array $reentryPointTypes */ + $reentryPointTypes = []; + $reentryPointCallback = new GatheringNodeCallback(static function (Node $node, Scope $nodeScope) use ($byRefUses, &$reentryPointTypes): void { + if (!self::isClosureReentryPoint($node)) { + return; + } + + foreach ($byRefUses as $byRefUse) { + $variableName = $byRefUse->var->name; + if (!is_string($variableName) || !$nodeScope->hasVariableType($variableName)->yes()) { + continue; + } + + $variableType = $nodeScope->getVariableType($variableName); + $variableNativeType = $nodeScope->getNativeType($byRefUse->var); + if (isset($reentryPointTypes[$variableName])) { + [$previousType, $previousNativeType] = $reentryPointTypes[$variableName]; + $variableType = TypeCombinator::union($previousType, $variableType); + $variableNativeType = TypeCombinator::union($previousNativeType, $variableNativeType); + } + + $reentryPointTypes[$variableName] = [$variableType, $variableNativeType]; + } + }, new NoopNodeCallback()); + $storage = $originalStorage->duplicate(); - $intermediaryClosureScopeResult = $this->processStmtNodesInternalWithoutFlushingPendingFibers($expr, $expr->stmts, $closureScope, $storage, new NoopNodeCallback(), StatementContext::createTopLevel()); + $intermediaryClosureScopeResult = $this->processStmtNodesInternalWithoutFlushingPendingFibers($expr, $expr->stmts, $closureScope, $storage, $reentryPointCallback, StatementContext::createTopLevel()); $intermediaryClosureScope = $intermediaryClosureScopeResult->getScope(); foreach ($intermediaryClosureScopeResult->getExitPoints() as $exitPoint) { $intermediaryClosureScope = $intermediaryClosureScope->mergeWith($exitPoint->getScope()); @@ -3108,6 +3133,28 @@ public function processClosureNode( break; } + // Control can leave the closure at every call in its body, so the closure + // can be entered again while an outer invocation sits at that call. The + // values a by-ref use holds there are therefore observable on entry, even + // when a later assignment overwrites them before the body ends. + foreach ($byRefUses as $byRefUse) { + $variableName = $byRefUse->var->name; + if (!is_string($variableName) || !isset($reentryPointTypes[$variableName])) { + continue; + } + if (!$intermediaryClosureScope->hasVariableType($variableName)->yes()) { + continue; + } + + [$reentryPointType, $reentryPointNativeType] = $reentryPointTypes[$variableName]; + $intermediaryClosureScope = $intermediaryClosureScope->assignVariable( + $variableName, + TypeCombinator::union($intermediaryClosureScope->getVariableType($variableName), $reentryPointType), + TypeCombinator::union($intermediaryClosureScope->getNativeType($byRefUse->var), $reentryPointNativeType), + TrinaryLogic::createYes(), + ); + } + $closureScope = $scope->enterAnonymousFunction($expr, $callableParameters, $nativeCallableParameters); $closureScope = $closureScope->processClosureScope($intermediaryClosureScope, $prevScope, $byRefUses); @@ -3139,6 +3186,19 @@ public function processClosureNode( return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions, $closureResultScope, $byRefUses); } + /** + * Points in a closure body where control can leave it and the closure can be + * entered again before the current invocation finishes. + */ + private static function isClosureReentryPoint(Node $node): bool + { + if ($node instanceof CallLike) { + return !$node->isFirstClassCallable(); + } + + return $node instanceof Expr\Yield_ || $node instanceof Expr\YieldFrom; + } + /** * @param InvalidateExprNode[] $invalidatedExpressions * @param string[] $uses diff --git a/tests/PHPStan/Analyser/nsrt/bug-15034.php b/tests/PHPStan/Analyser/nsrt/bug-15034.php new file mode 100644 index 0000000000..40f83ed13a --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15034.php @@ -0,0 +1,22 @@ +treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-15034.php'], []); + } + + public function testClosureByRefUseReentry(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/closure-by-ref-use-reentry.php'], []); + } + public function testMarkerFromAnotherFileDoesNotSuppress(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Comparison/data/bug-15034.php b/tests/PHPStan/Rules/Comparison/data/bug-15034.php new file mode 100644 index 0000000000..aa16a99bf8 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-15034.php @@ -0,0 +1,19 @@ +dispatch(); + $viaMethodCall = false; +}; + +// re-entry through a static method call +$viaStaticCall = false; +$c = function () use (&$viaStaticCall): void { + if ($viaStaticCall) { + return; + } + + $viaStaticCall = true; + Dispatcher::dispatchStatic(); + $viaStaticCall = false; +}; + +// re-entry through a constructor +$viaNew = false; +$d = static function () use (&$viaNew): void { + if ($viaNew) { + return; + } + + $viaNew = true; + new Dispatcher(); + $viaNew = false; +}; + +// negated condition +$negated = false; +$e = function () use (&$negated): void { + if (!$negated) { + $negated = true; + dispatch(); + $negated = false; + } +}; + +// assignment and call inside a loop +$inLoop = false; +$f = function (int $times) use (&$inLoop): void { + for ($i = 0; $i < $times; $i++) { + if ($inLoop) { + return; + } + + $inLoop = true; + dispatch(); + $inLoop = false; + } +}; + +// re-entry through a call made by a nested closure +$viaNestedClosure = false; +$g = function () use (&$viaNestedClosure): void { + if ($viaNestedClosure) { + return; + } + + $inner = function () use (&$viaNestedClosure): void { + $viaNestedClosure = true; + dispatch(); + $viaNestedClosure = false; + }; + $inner(); +}; + +// re-entry while the generator is suspended on yield +$viaYield = false; +$h = function () use (&$viaYield): \Generator { + if ($viaYield) { + return; + } + + $viaYield = true; + yield 1; + $viaYield = false; +}; + +// re-entry while the generator is suspended on yield from +$viaYieldFrom = false; +$i = function () use (&$viaYieldFrom): \Generator { + if ($viaYieldFrom) { + return; + } + + $viaYieldFrom = true; + yield from [1, 2]; + $viaYieldFrom = false; +};