Widen by-ref use variables with the types they hold at calls and yields inside the closure body

src/Analyser/NodeScopeResolver.php

@@ -3096,8 +3096,33 @@
 		do {
 			$prevScope = $closureScope;
 
+			/** @var array<string, array{Type, Type}> $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 @@
 				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 @@
 		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

tests/PHPStan/Analyser/nsrt/bug-15034.php

@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Bug15034;
+
+use function PHPStan\Testing\assertType;
+
+$isRunning = false;
+
+$listener = function () use (&$isRunning, &$listener): void {
+	assertType('bool', $isRunning);
+	if ($isRunning) {
+		return;
+	}
+
+	$isRunning = true;
+	$listener();
+	$isRunning = false;
+};
+
+$listener();

tests/PHPStan/Rules/Comparison/IfConstantConditionRuleTest.php

@@ -408,6 +408,18 @@ public function testDeferredErrorsAreNotCollapsedAcrossFiles(): void
 		]);
 	}
 
+	public function testBug15034(): void
+	{
+		$this->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;

tests/PHPStan/Rules/Comparison/data/bug-15034.php

@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Bug15034;
+
+$isRunning = false;
+
+$listener = function () use (&$isRunning, &$listener): void {
+	if ($isRunning) {
+		return;
+	}
+
+	$isRunning = true;
+	$listener();
+	$isRunning = false;
+};
+
+$listener();

tests/PHPStan/Rules/Comparison/data/closure-by-ref-use-reentry.php

@@ -0,0 +1,133 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ClosureByRefUseReentry;
+
+class Dispatcher
+{
+
+	public function dispatch(): void
+	{
+	}
+
+	public static function dispatchStatic(): void
+	{
+	}
+
+}
+
+function dispatch(): void
+{
+}
+
+// re-entry through a plain function call
+$viaFunctionCall = false;
+$a = function () use (&$viaFunctionCall): void {
+	if ($viaFunctionCall) {
+		return;
+	}
+
+	$viaFunctionCall = true;
+	dispatch();
+	$viaFunctionCall = false;
+};
+
+// re-entry through a method call
+$viaMethodCall = false;
+$b = function (Dispatcher $dispatcher) use (&$viaMethodCall): void {
+	if ($viaMethodCall) {
+		return;
+	}
+
+	$viaMethodCall = true;
+	$dispatcher->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;
+};