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; +};