From 5d58a8f5b82b4773832fee7c15c5119189367a3f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 4 Nov 2024 15:44:41 +0100 Subject: [PATCH 1/5] More precise types in immediately invoked callables --- src/Analyser/MutatingScope.php | 6 +----- src/Analyser/NodeScopeResolver.php | 25 ++++++++++++++++++++--- tests/PHPStan/Analyser/nsrt/bug-11561.php | 25 ++++++++++++++++++++--- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 2724ba1fca..90b3fdd465 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -46,7 +46,6 @@ use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Node\PropertyAssignNode; use PHPStan\Parser\ArrayMapArgVisitor; -use PHPStan\Parser\ImmediatelyInvokedClosureVisitor; use PHPStan\Parser\NewAssignedToPropertyVisitor; use PHPStan\Parser\Parser; use PHPStan\Php\PhpVersion; @@ -4795,7 +4794,6 @@ private function processFinallyScopeVariableTypeHolders( * @param Expr\ClosureUse[] $byRefUses */ public function processClosureScope( - Expr\Closure $expr, self $closureScope, ?self $prevScope, array $byRefUses, @@ -4828,9 +4826,7 @@ public function processClosureScope( $prevVariableType = $prevScope->getVariableType($variableName); if (!$variableType->equals($prevVariableType)) { $variableType = TypeCombinator::union($variableType, $prevVariableType); - if ($expr->getAttribute(ImmediatelyInvokedClosureVisitor::ATTRIBUTE_NAME) !== true) { - $variableType = self::generalizeType($variableType, $prevVariableType, 0); - } + $variableType = self::generalizeType($variableType, $prevVariableType, 0); } } diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index fd6ac13fc3..83a00677cf 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -119,6 +119,7 @@ use PHPStan\Node\VarTagChangedExpressionTypeNode; use PHPStan\Parser\ArrowFunctionArgVisitor; use PHPStan\Parser\ClosureArgVisitor; +use PHPStan\Parser\ImmediatelyInvokedClosureVisitor; use PHPStan\Parser\Parser; use PHPStan\Php\PhpVersion; use PHPStan\PhpDoc\PhpDocInheritanceResolver; @@ -4232,7 +4233,7 @@ private function processClosureNode( } $closureScope = $scope->enterAnonymousFunction($expr, $callableParameters); - $closureScope = $closureScope->processClosureScope($expr, $scope, null, $byRefUses); + $closureScope = $closureScope->processClosureScope($scope, null, $byRefUses); $closureType = $closureScope->getAnonymousFunctionReflection(); if (!$closureType instanceof ClosureType) { throw new ShouldNotHappenException(); @@ -4291,6 +4292,24 @@ private function processClosureNode( return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions); } + if ($expr->getAttribute(ImmediatelyInvokedClosureVisitor::ATTRIBUTE_NAME) === true) { + $intermediaryClosureScopeResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, static function (): void { + }, StatementContext::createTopLevel()); + $intermediaryClosureScope = $intermediaryClosureScopeResult->getScope(); + + $statementResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback, StatementContext::createTopLevel()); + $nodeCallback(new ClosureReturnStatementsNode( + $expr, + $gatheredReturnStatements, + $gatheredYieldStatements, + $statementResult, + $executionEnds, + array_merge($statementResult->getImpurePoints(), $closureImpurePoints), + ), $closureScope); + + return new ProcessClosureResult($scope->processClosureScope($intermediaryClosureScope, null, $byRefUses), $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions); + } + $count = 0; do { $prevScope = $closureScope; @@ -4302,7 +4321,7 @@ private function processClosureNode( $intermediaryClosureScope = $intermediaryClosureScope->mergeWith($exitPoint->getScope()); } $closureScope = $scope->enterAnonymousFunction($expr, $callableParameters); - $closureScope = $closureScope->processClosureScope($expr, $intermediaryClosureScope, $prevScope, $byRefUses); + $closureScope = $closureScope->processClosureScope($intermediaryClosureScope, $prevScope, $byRefUses); if ($closureScope->equals($prevScope)) { break; } @@ -4322,7 +4341,7 @@ private function processClosureNode( array_merge($statementResult->getImpurePoints(), $closureImpurePoints), ), $closureScope); - return new ProcessClosureResult($scope->processClosureScope($expr, $closureScope, null, $byRefUses), $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions); + return new ProcessClosureResult($scope->processClosureScope($closureScope, null, $byRefUses), $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions); } /** diff --git a/tests/PHPStan/Analyser/nsrt/bug-11561.php b/tests/PHPStan/Analyser/nsrt/bug-11561.php index 1a01e5f97a..0675f09bd8 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11561.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11561.php @@ -12,13 +12,13 @@ function main(mixed $c): void{ assertType('array{date: DateTime, id: 1}', $c); $x = (function() use (&$c) { - assertType("array{date: DateTime, id: 1, name?: 'ruud'}", $c); + assertType("array{date: DateTime, id: 1}", $c); $c['name'] = 'ruud'; assertType("array{date: DateTime, id: 1, name: 'ruud'}", $c); return 'x'; })(); - assertType("array{date: DateTime, id: 1, name?: 'ruud'}", $c); + assertType("array{date: DateTime, id: 1, name: 'ruud'}", $c); } @@ -30,11 +30,30 @@ function main2(mixed $c): void{ assertType("array{date: DateTime, id: 1, name: 'staabm'}", $c); $x = (function() use (&$c) { - assertType("array{date: DateTime, id: 1, name: 'ruud'|'staabm'}", $c); + assertType("array{date: DateTime, id: 1, name: 'staabm'}", $c); $c['name'] = 'ruud'; assertType("array{date: DateTime, id: 1, name: 'ruud'}", $c); return 'x'; })(); + assertType("array{date: DateTime, id: 1, name: 'ruud'}", $c); +} + +/** @param array{date: DateTime} $c */ +function main3(mixed $c): void{ + assertType('array{date: DateTime}', $c); + $c['id']=1; + $c['name'] = 'staabm'; + assertType("array{date: DateTime, id: 1, name: 'staabm'}", $c); + + $x = (function() use (&$c) { + assertType("array{date: DateTime, id: 1, name: 'staabm'}", $c); + if (rand(0,1)) { + $c['name'] = 'ruud'; + } + assertType("array{date: DateTime, id: 1, name: 'ruud'|'staabm'}", $c); + return 'x'; + })(); + assertType("array{date: DateTime, id: 1, name: 'ruud'|'staabm'}", $c); } From c7672652d9783746a1417f57b5463ffb8bb0d56d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 4 Nov 2024 16:45:12 +0100 Subject: [PATCH 2/5] add another failling test --- tests/PHPStan/Analyser/nsrt/bug-11561.php | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/PHPStan/Analyser/nsrt/bug-11561.php b/tests/PHPStan/Analyser/nsrt/bug-11561.php index 0675f09bd8..f6894d4724 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11561.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11561.php @@ -57,3 +57,24 @@ function main3(mixed $c): void{ assertType("array{date: DateTime, id: 1, name: 'ruud'|'staabm'}", $c); } + +/** @param array{date: DateTime} $c */ +function main4(mixed $c): void{ + assertType('array{date: DateTime}', $c); + $c['id']=1; + $c['name'] = 'staabm'; + assertType("array{date: DateTime, id: 1, name: 'staabm'}", $c); + + $x = (function() use (&$c) { + assertType("array{date: DateTime, id: 1, name: 'staabm'}", $c); + if (rand(0,1)) { + $c['name'] = 'ruud'; + assertType("array{date: DateTime, id: 1, name: 'ruud'}", $c); + return 'y'; + } + assertType("array{date: DateTime, id: 1, name: 'staabm'}", $c); + return 'x'; + })(); + + assertType("array{date: DateTime, id: 1, name: 'ruud'|'staabm'}", $c); +} From f2bf92bb576bd8b9c61a3d118e1f02a21971dd66 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 4 Nov 2024 16:51:58 +0100 Subject: [PATCH 3/5] fix --- src/Analyser/NodeScopeResolver.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 83a00677cf..57c44c0405 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4278,6 +4278,7 @@ private function processClosureNode( $gatheredReturnStatements[] = new ReturnStatement($scope, $node); }; + if (count($byRefUses) === 0) { $statementResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback, StatementContext::createTopLevel()); $nodeCallback(new ClosureReturnStatementsNode( @@ -4296,6 +4297,9 @@ private function processClosureNode( $intermediaryClosureScopeResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, static function (): void { }, StatementContext::createTopLevel()); $intermediaryClosureScope = $intermediaryClosureScopeResult->getScope(); + foreach ($intermediaryClosureScopeResult->getExitPoints() as $exitPoint) { + $intermediaryClosureScope = $intermediaryClosureScope->mergeWith($exitPoint->getScope()); + } $statementResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback, StatementContext::createTopLevel()); $nodeCallback(new ClosureReturnStatementsNode( From 9f336654d9ae39f07701da11854ff70fa81e5589 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 4 Nov 2024 17:00:35 +0100 Subject: [PATCH 4/5] de-duplicate fix --- src/Analyser/NodeScopeResolver.php | 34 +++++++++++------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 57c44c0405..a8bf0a3ff7 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4293,27 +4293,6 @@ private function processClosureNode( return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions); } - if ($expr->getAttribute(ImmediatelyInvokedClosureVisitor::ATTRIBUTE_NAME) === true) { - $intermediaryClosureScopeResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, static function (): void { - }, StatementContext::createTopLevel()); - $intermediaryClosureScope = $intermediaryClosureScopeResult->getScope(); - foreach ($intermediaryClosureScopeResult->getExitPoints() as $exitPoint) { - $intermediaryClosureScope = $intermediaryClosureScope->mergeWith($exitPoint->getScope()); - } - - $statementResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback, StatementContext::createTopLevel()); - $nodeCallback(new ClosureReturnStatementsNode( - $expr, - $gatheredReturnStatements, - $gatheredYieldStatements, - $statementResult, - $executionEnds, - array_merge($statementResult->getImpurePoints(), $closureImpurePoints), - ), $closureScope); - - return new ProcessClosureResult($scope->processClosureScope($intermediaryClosureScope, null, $byRefUses), $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions); - } - $count = 0; do { $prevScope = $closureScope; @@ -4324,8 +4303,15 @@ private function processClosureNode( foreach ($intermediaryClosureScopeResult->getExitPoints() as $exitPoint) { $intermediaryClosureScope = $intermediaryClosureScope->mergeWith($exitPoint->getScope()); } + + if ($expr->getAttribute(ImmediatelyInvokedClosureVisitor::ATTRIBUTE_NAME) === true) { + $closureResultScope = $intermediaryClosureScope; + break; + } + $closureScope = $scope->enterAnonymousFunction($expr, $callableParameters); $closureScope = $closureScope->processClosureScope($intermediaryClosureScope, $prevScope, $byRefUses); + if ($closureScope->equals($prevScope)) { break; } @@ -4335,6 +4321,10 @@ private function processClosureNode( $count++; } while ($count < self::LOOP_SCOPE_ITERATIONS); + if (!isset($closureResultScope)) { + $closureResultScope = $closureScope; + } + $statementResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback, StatementContext::createTopLevel()); $nodeCallback(new ClosureReturnStatementsNode( $expr, @@ -4345,7 +4335,7 @@ private function processClosureNode( array_merge($statementResult->getImpurePoints(), $closureImpurePoints), ), $closureScope); - return new ProcessClosureResult($scope->processClosureScope($closureScope, null, $byRefUses), $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions); + return new ProcessClosureResult($scope->processClosureScope($closureResultScope, null, $byRefUses), $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions); } /** From 26681377defb774f48c02b40887959c3d2304ec0 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 4 Nov 2024 20:13:46 +0100 Subject: [PATCH 5/5] cleanup --- src/Analyser/NodeScopeResolver.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index a8bf0a3ff7..fc7b1e9df7 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4294,6 +4294,7 @@ private function processClosureNode( } $count = 0; + $closureResultScope = null; do { $prevScope = $closureScope; @@ -4321,7 +4322,7 @@ private function processClosureNode( $count++; } while ($count < self::LOOP_SCOPE_ITERATIONS); - if (!isset($closureResultScope)) { + if ($closureResultScope === null) { $closureResultScope = $closureScope; }