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/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-13810.php b/tests/PHPStan/Analyser/nsrt/bug-13810.php new file mode 100644 index 0000000000..353aeff6f0 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13810.php @@ -0,0 +1,19 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug13810; + +use function PHPStan\Testing\assertType; + +function test(): void +{ + static $isSupported; + $isSupported ??= function (mixed $arg) use (&$isSupported): bool { + assertType('Closure(mixed): bool', $isSupported); + return $isSupported($arg); + }; + + assertType('mixed~null', $isSupported); + $isSupported('foo'); +} 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 @@ +