From 3f50db18683b11aa0ab8bb81eecd526b671e7777 Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Thu, 18 Jun 2026 09:06:34 +0000 Subject: [PATCH 1/3] Enter right-side-assign context for `??=` so closures capturing the assigned variable by reference see its assigned type - `AssignOpHandler` now calls `ExpressionContext::enterRightSideAssign()` when evaluating the right-hand side of a `??=` whose target is a simple variable, mirroring what `AssignHandler` already does for plain `=`/`=&`. - This lets the by-reference closure-use handling in `NodeScopeResolver` (which inspects `inAssignRightSideVariableName`/`inAssignRightSideExpr`) recognise patterns like `$cb ??= function () use (&$cb) { ... }` and resolve `$cb` to the assigned closure type inside the closure body instead of `null`. - Without this, `??=` (unlike `=`) left the right-side-assign context empty, so a recursive `use (&$var)` closure saw `$var` as `null` and produced false `callable.nonCallable` errors. - Probed the sibling axis (other assignment operators): plain `=`/`=&` already set this context; the arithmetic/string compound ops (`+=`, `.=`, ...) cannot meaningfully capture-and-reassign the same variable by reference, so only `??=` needed the change. --- src/Analyser/ExprHandler/AssignOpHandler.php | 8 ++++++++ tests/PHPStan/Analyser/nsrt/bug-13810.php | 17 +++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-13810.php diff --git a/src/Analyser/ExprHandler/AssignOpHandler.php b/src/Analyser/ExprHandler/AssignOpHandler.php index 31af0b9c77..6208008ee9 100644 --- a/src/Analyser/ExprHandler/AssignOpHandler.php +++ b/src/Analyser/ExprHandler/AssignOpHandler.php @@ -29,6 +29,7 @@ use PHPStan\Type\Type; use function array_merge; use function get_class; +use function is_string; use function sprintf; /** @@ -68,6 +69,13 @@ static function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $contex $scope = $scope->filterByFalseyValue( new BinaryOp\NotIdentical($expr->var, new ConstFetch(new Name('null'))), ); + + if ($expr->var instanceof Expr\Variable && is_string($expr->var->name)) { + $context = $context->enterRightSideAssign( + $expr->var->name, + $expr->expr, + ); + } } $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeep()); diff --git a/tests/PHPStan/Analyser/nsrt/bug-13810.php b/tests/PHPStan/Analyser/nsrt/bug-13810.php new file mode 100644 index 0000000000..c5e801c479 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13810.php @@ -0,0 +1,17 @@ + Date: Thu, 18 Jun 2026 09:40:23 +0000 Subject: [PATCH 2/3] Widen by-reference closure use variables with their later enclosing assignments A variable captured by reference shares storage with the enclosing variable, and the closure may be invoked at any later point. Forward flow analysis only sees the variable's type at the point the closure is created, so reassignments performed in the enclosing scope afterwards were invisible inside the closure body, producing false "always true/false" impossible-type errors. ClosureUseByRefAssignmentsVisitor collects, per closure, the right-hand side expressions of assignments to each by-reference use variable that occur after the closure definition in the enclosing function-like scope. processClosureExpr() then unions those types into the variable's type seeding the closure body. Assignments preceding the closure are already reflected in the variable's narrowed type and are left untouched, preserving existing precision. Co-Authored-By: Claude Opus 4.8 --- src/Analyser/NodeScopeResolver.php | 58 ++++++++ .../ClosureUseByRefAssignmentsVisitor.php | 126 ++++++++++++++++++ tests/PHPStan/Analyser/nsrt/bug-7751.php | 39 ++++++ 3 files changed, 223 insertions(+) create mode 100644 src/Parser/ClosureUseByRefAssignmentsVisitor.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-7751.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 3f63434d03..eea172a9e2 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -111,6 +111,7 @@ use PHPStan\Node\VarTagChangedExpressionTypeNode; use PHPStan\Parser\ArrowFunctionArgVisitor; use PHPStan\Parser\ClosureArgVisitor; +use PHPStan\Parser\ClosureUseByRefAssignmentsVisitor; use PHPStan\Parser\GotoLabelVisitor; use PHPStan\Parser\ImmediatelyInvokedClosureVisitor; use PHPStan\Parser\LineAttributesVisitor; @@ -2933,6 +2934,11 @@ public function processClosureNode( $callableParameters = $this->createCallableParameters($scope, $expr, $closureCallArgs, $passedToType); $nativeCallableParameters = $this->createNativeCallableParameters($scope, $expr, $closureCallArgs, $nativePassedToType); + $enclosingByRefAssignedExprs = $expr->getAttribute(ClosureUseByRefAssignmentsVisitor::ATTRIBUTE_NAME); + if (!is_array($enclosingByRefAssignedExprs)) { + $enclosingByRefAssignedExprs = []; + } + $useScope = $scope; foreach ($expr->uses as $use) { if ($use->byRef) { @@ -2969,6 +2975,18 @@ public function processClosureNode( } $scope = $scope->assignVariable($inAssignRightSideVariableName, $variableType, $variableNativeType, TrinaryLogic::createYes()); } + + if ( + is_string($use->var->name) + && isset($enclosingByRefAssignedExprs[$use->var->name]) + ) { + $scope = $this->widenByRefUseFromEnclosingAssignments( + $scope, + $expr, + $use->var->name, + $enclosingByRefAssignedExprs[$use->var->name], + ); + } } $this->processExprNode($stmt, $use->var, $useScope, $storage, $nodeCallback, $context); if (!$use->byRef) { @@ -3095,6 +3113,46 @@ public function processClosureNode( return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions, $closureResultScope, $byRefUses); } + /** + * A variable captured by reference shares storage with the enclosing variable and the + * closure may run at any time, so inside the closure body the variable can hold any value + * the enclosing variable is ever assigned. Widen the seeded type with those assignments. + * + * @param Expr[] $assignedExprs + */ + private function widenByRefUseFromEnclosingAssignments( + MutatingScope $scope, + Expr\Closure $closure, + string $variableName, + array $assignedExprs, + ): MutatingScope + { + $alreadyDefined = $scope->hasVariableType($variableName)->yes(); + + $type = $alreadyDefined ? $scope->getVariableType($variableName) : new NullType(); + $nativeType = $alreadyDefined ? $scope->getNativeType(new Expr\Variable($variableName)) : new NullType(); + $changed = false; + + foreach ($assignedExprs as $assignedExpr) { + // The closure's own value when it is assigned to the captured variable is already + // handled precisely via the right-side-assign context; skip it to avoid recursing + // into the closure type we are currently resolving. + if ($assignedExpr === $closure) { + continue; + } + + $type = TypeCombinator::union($type, $scope->getType($assignedExpr)); + $nativeType = TypeCombinator::union($nativeType, $scope->getNativeType($assignedExpr)); + $changed = true; + } + + if (!$changed) { + return $scope; + } + + return $scope->assignVariable($variableName, $type, $nativeType, TrinaryLogic::createYes()); + } + /** * @param InvalidateExprNode[] $invalidatedExpressions * @param string[] $uses diff --git a/src/Parser/ClosureUseByRefAssignmentsVisitor.php b/src/Parser/ClosureUseByRefAssignmentsVisitor.php new file mode 100644 index 0000000000..07d6715655 --- /dev/null +++ b/src/Parser/ClosureUseByRefAssignmentsVisitor.php @@ -0,0 +1,126 @@ +> keyed by by-reference use variable name. + */ + public const ATTRIBUTE_NAME = 'enclosingByRefUseAssignedExprs'; + + /** + * @var list, closures: list}>}> + */ + private array $frames = []; + + #[Override] + public function beforeTraverse(array $nodes): ?array + { + $this->frames = [['assignments' => [], 'closures' => []]]; + return null; + } + + #[Override] + public function enterNode(Node $node): ?Node + { + if ( + ($node instanceof Node\Expr\Assign || $node instanceof Node\Expr\AssignRef || $node instanceof Node\Expr\AssignOp) + && $node->var instanceof Node\Expr\Variable + && is_string($node->var->name) + ) { + $this->frames[count($this->frames) - 1]['assignments'][] = [$node->getStartFilePos(), $node->var->name, $node->expr]; + } + + if ($node instanceof Node\Expr\Closure) { + $byRefNames = []; + foreach ($node->uses as $use) { + if (!$use->byRef || !is_string($use->var->name)) { + continue; + } + $byRefNames[] = $use->var->name; + } + if (count($byRefNames) > 0) { + $this->frames[count($this->frames) - 1]['closures'][] = [$node, $byRefNames]; + } + } + + if ($node instanceof Node\FunctionLike) { + $this->frames[] = ['assignments' => [], 'closures' => []]; + } + + return null; + } + + #[Override] + public function leaveNode(Node $node): ?Node + { + if ($node instanceof Node\FunctionLike) { + $frame = array_pop($this->frames); + if ($frame !== null) { + $this->attachFrame($frame); + } + } + + return null; + } + + #[Override] + public function afterTraverse(array $nodes): ?array + { + $frame = array_pop($this->frames); + if ($frame !== null) { + $this->attachFrame($frame); + } + + return null; + } + + /** + * @param array{assignments: list, closures: list}>} $frame + */ + private function attachFrame(array $frame): void + { + foreach ($frame['closures'] as [$closure, $byRefNames]) { + $closureStartPos = $closure->getStartFilePos(); + $map = []; + foreach ($frame['assignments'] as [$assignStartPos, $assignName, $assignExpr]) { + if ($assignStartPos <= $closureStartPos) { + continue; + } + foreach ($byRefNames as $name) { + if ($name !== $assignName) { + continue; + } + $map[$name][] = $assignExpr; + } + } + if (count($map) === 0) { + continue; + } + $closure->setAttribute(self::ATTRIBUTE_NAME, $map); + } + } + +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-7751.php b/tests/PHPStan/Analyser/nsrt/bug-7751.php new file mode 100644 index 0000000000..2b7be1d079 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-7751.php @@ -0,0 +1,39 @@ + Date: Thu, 18 Jun 2026 09:40:27 +0000 Subject: [PATCH 3/3] Mark bug-13810 fixture as requiring PHP 8.0 The fixture uses the mixed type, which requires a `// lint >= 8.0` comment so it is skipped on older PHP versions in CI. Co-Authored-By: Claude Opus 4.8 --- tests/PHPStan/Analyser/nsrt/bug-13810.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-13810.php b/tests/PHPStan/Analyser/nsrt/bug-13810.php index c5e801c479..353aeff6f0 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13810.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13810.php @@ -1,4 +1,6 @@ -= 8.0 + +declare(strict_types = 1); namespace Bug13810;