Generalize by-reference closure use variables that are reassigned in the enclosing scope after the closure is declared

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)) {

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();

src/Parser/ClosureUseByRefReassignmentVisitor.php

@@ -0,0 +1,174 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Parser;
+
+use Override;
+use PhpParser\Node;
+use PhpParser\NodeVisitorAbstract;
+use PHPStan\DependencyInjection\AutowiredService;
+use function array_pop;
+use function count;
+use function is_string;
+
+/**
+ * Marks by-reference closure uses (`use (&$var)`) whose captured variable is
+ * reassigned in the enclosing scope after the closure was declared.
+ *
+ * Such variables may hold a different value by the time the closure is invoked,
+ * so the closure body must not assume the value present at declaration time.
+ */
+#[AutowiredService]
+final class ClosureUseByRefReassignmentVisitor extends NodeVisitorAbstract
+{
+
+	public const ATTRIBUTE_NAME = 'closureUseByRefReassigned';
+
+	/** @var list<array{closures: list<array{order: int, uses: list<Node\ClosureUse>}>, assignments: array<string, list<int>>}> */
+	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<array{order: int, uses: list<Node\ClosureUse>}>, assignments: array<string, list<int>>}
+	 */
+	private static function createFrame(): array
+	{
+		return ['closures' => [], 'assignments' => []];
+	}
+
+	/**
+	 * @param array{closures: list<array{order: int, uses: list<Node\ClosureUse>}>, assignments: array<string, list<int>>} $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;
+	}
+
+}

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

@@ -0,0 +1,106 @@
+<?php
+
+namespace Bug14848;
+
+use function PHPStan\Testing\assertType;
+
+function reassignedAfter(): void
+{
+	$tmpdir = "";
+	$myfunc = function () use (&$tmpdir) {
+		assertType('string', $tmpdir);
+	};
+
+	$tmpdir = "/tmp/my/useful/tempdir";
+	$myfunc();
+}
+
+function notReassigned(): void
+{
+	$tmpdir = "";
+	$myfunc = function () use (&$tmpdir) {
+		assertType("''", $tmpdir);
+	};
+
+	$myfunc();
+}
+
+function reassignedBeforeOnly(): void
+{
+	$tmpdir = "";
+	$tmpdir = "foo";
+	$myfunc = function () use (&$tmpdir) {
+		assertType("'foo'", $tmpdir);
+	};
+
+	$myfunc();
+}
+
+function reassignedAfterWithDifferentType(): void
+{
+	$x = 1;
+	$myfunc = function () use (&$x) {
+		assertType('int', $x);
+	};
+
+	$x = 5;
+	$myfunc();
+}
+
+function assignOpAfter(): void
+{
+	$s = "a";
+	$myfunc = function () use (&$s) {
+		assertType('string', $s);
+	};
+
+	$s .= "b";
+	$myfunc();
+}
+
+function incrementAfter(): void
+{
+	$i = 0;
+	$myfunc = function () use (&$i) {
+		assertType('int', $i);
+	};
+
+	$i++;
+	$myfunc();
+}
+
+function reassignedInNestedBlockAfter(): void
+{
+	$v = 1;
+	$myfunc = function () use (&$v) {
+		assertType('int', $v);
+	};
+
+	if (rand(0, 1)) {
+		$v = 2;
+	}
+	$myfunc();
+}
+
+function arrayElementReassignedAfter(): void
+{
+	$arr = ['a'];
+	$myfunc = function () use (&$arr) {
+		assertType('non-empty-list<string>', $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();
+}