diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 53ac5a06f71..3e986650e29 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -3037,6 +3037,22 @@ public function callNodeCallback( $nodeCallback($node, $scope); } + /** + * Whether a walk driven by this callback produces no rule or collector + * output - its only product is the resulting scope. Such a walk may be + * replaced by any other walk of the same body from the same entry scope. + * + * @param callable(Node $node, Scope $scope): void $nodeCallback + */ + private static function isScopeOnlyWalk(callable $nodeCallback): bool + { + while ($nodeCallback instanceof GatheringNodeCallback) { + $nodeCallback = $nodeCallback->getInner(); + } + + return $nodeCallback instanceof NoopNodeCallback; + } + /** * @param callable(Node $node, Scope $scope): void $nodeCallback */ @@ -3188,17 +3204,45 @@ public function processClosureNode( $originalStorage = $storage; + // A scope-only walk emits no rule output, so the convergence pass that + // settles the by-ref uses already IS this walk's result pass: let it + // gather and reuse it instead of walking the body once more below. + // Without this a by-ref closure walks its body twice, which multiplies + // with every level of closure nesting. + $reuseSettledPass = self::isScopeOnlyWalk($nodeCallback); + + // only the gathering walk that ends up being the result walk may + // contribute - every gathering walk starts the arrays over so a repeated + // one does not append to them twice + $restartGathering = static function () use (&$executionEnds, &$gatheredReturnStatements, &$gatheredReturnStatementsWithScope, &$gatheredYieldStatements, &$gatheredYieldStatementsWithScope, &$closureImpurePoints, &$invalidateExpressions): void { + $executionEnds = []; + $gatheredReturnStatements = []; + $gatheredReturnStatementsWithScope = []; + $gatheredYieldStatements = []; + $gatheredYieldStatementsWithScope = []; + $closureImpurePoints = []; + $invalidateExpressions = []; + }; + $count = 0; $closureResultScope = null; + $settledStatementResult = null; do { $prevScope = $closureScope; - $storage = $originalStorage->duplicate(); + if ($reuseSettledPass) { + $restartGathering(); + $storage = $originalStorage; + } else { + $storage = $originalStorage->duplicate(); + } // deep context, like the loop handlers' own convergence passes: inner // loops walk single-pass here and only the final walk below (top-level) // runs their full convergence - otherwise every closure-convergence - // pass would re-converge every inner loop from scratch - $intermediaryClosureScopeResult = $this->processStmtNodesInternalWithoutFlushingPendingFibers($expr, $expr->stmts, $closureScope, $storage, new NoopNodeCallback(), StatementContext::createDeep()); + // pass would re-converge every inner loop from scratch. A scope-only + // walk has no final walk to reach that convergence, matching how it + // approximates every other loop it enters deep. + $intermediaryClosureScopeResult = $this->processStmtNodesInternalWithoutFlushingPendingFibers($expr, $expr->stmts, $closureScope, $storage, $reuseSettledPass ? $closureStmtsCallback : new NoopNodeCallback(), StatementContext::createDeep()); $intermediaryClosureScope = $intermediaryClosureScopeResult->getScope(); foreach ($intermediaryClosureScopeResult->getExitPoints() as $exitPoint) { $intermediaryClosureScope = $intermediaryClosureScope->mergeWith($exitPoint->getScope()); @@ -3206,6 +3250,9 @@ public function processClosureNode( if ($expr->getAttribute(ImmediatelyInvokedClosureVisitor::ATTRIBUTE_NAME) === true) { $closureResultScope = $intermediaryClosureScope; + if ($reuseSettledPass) { + $settledStatementResult = $intermediaryClosureScopeResult; + } break; } @@ -3213,6 +3260,10 @@ public function processClosureNode( $closureScope = $closureScope->processClosureScope($intermediaryClosureScope, $prevScope, $byRefUses); if ($closureScope->equals($prevScope)) { + if ($reuseSettledPass) { + // the settled entry scope is the one this pass walked from + $settledStatementResult = $intermediaryClosureScopeResult; + } break; } if ($count >= self::GENERALIZE_AFTER_ITERATION) { @@ -3226,7 +3277,14 @@ public function processClosureNode( } $storage = $originalStorage; - $statementResult = $this->processStmtNodesInternalWithoutFlushingPendingFibers($expr, $expr->stmts, $closureScope, $storage, $closureStmtsCallback, StatementContext::createTopLevel()); + if ($settledStatementResult !== null) { + $statementResult = $settledStatementResult; + } else { + if ($reuseSettledPass) { + $restartGathering(); + } + $statementResult = $this->processStmtNodesInternalWithoutFlushingPendingFibers($expr, $expr->stmts, $closureScope, $storage, $closureStmtsCallback, StatementContext::createTopLevel()); + } $publicStatementResult = $statementResult->toPublic(); $closureReturnStatementsNodeScope = $this->refineClosureNodeScope($closureScope, $scope, $expr, $gatheredReturnStatementsWithScope, $gatheredYieldStatementsWithScope, $executionEnds, $statementResult->getThrowPoints(), array_merge($closureImpurePoints, $statementResult->getImpurePoints()), $invalidateExpressions); $this->callNodeCallback($nodeCallback, new ClosureReturnStatementsNode( diff --git a/tests/PHPStan/Analyser/nsrt/nested-by-ref-closures.php b/tests/PHPStan/Analyser/nsrt/nested-by-ref-closures.php new file mode 100644 index 00000000000..292a534e6a4 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/nested-by-ref-closures.php @@ -0,0 +1,71 @@ +run(function () use (&$counter, &$collected): void { + assertType('int<0, max>', $counter); + assertType("array<'first'|'second'|'third', true>", $collected); + $counter++; + $collected['first'] = true; + $this->run(function () use (&$counter, &$collected): void { + assertType('int<1, max>', $counter); + assertType("non-empty-array<'first'|'second'|'third', true>&hasOffsetValue('first', true)", $collected); + $counter++; + $collected['second'] = true; + $this->run(function () use (&$counter, &$collected): void { + assertType('int<2, max>', $counter); + assertType("non-empty-array<'first'|'second'|'third', true>&hasOffsetValue('first', true)&hasOffsetValue('second', true)", $collected); + $counter++; + $collected['third'] = true; + }); + assertType('int<2, max>', $counter); + assertType("non-empty-array<'first'|'second'|'third', true>&hasOffsetValue('first', true)&hasOffsetValue('second', true)", $collected); + }); + assertType('int<1, max>', $counter); + assertType("non-empty-array<'first'|'second'|'third', true>&hasOffsetValue('first', true)", $collected); + }); + assertType('int<0, max>', $counter); + assertType("array<'first'|'second'|'third', true>", $collected); + } + + /** @param list $items */ + public function nestedInLoop(array $items): void + { + $seen = []; + foreach ($items as $item) { + $this->run(function () use (&$seen, $item): void { + $seen[] = $item; + $this->run(function () use (&$seen): void { + $seen[] = 'inner'; + }); + }); + } + assertType('list', $seen); + } + + public function immediatelyInvoked(): void + { + $value = null; + (function () use (&$value): void { + $this->run(function () use (&$value): void { + $value = new Runner(); + }); + })(); + assertType('NestedByRefClosures\Runner|null', $value); + } + +}