From c9833142a6f2ab9340c8f9fb799dcf96bdb9549a Mon Sep 17 00:00:00 2001 From: Martin Jonas Date: Sun, 18 Dec 2022 11:44:56 +0100 Subject: [PATCH] NodeScopeResolver analysed files not as global state and optional --- src/Analyser/Analyser.php | 2 - src/Analyser/FileAnalyser.php | 1 + src/Analyser/NodeScopeResolver.php | 79 ++++++++++++++++--------- src/Command/WorkerCommand.php | 5 -- src/PhpDoc/StubValidator.php | 5 -- src/Testing/RuleTestCase.php | 1 - src/Testing/TypeInferenceTestCase.php | 3 +- tests/PHPStan/Analyser/AnalyserTest.php | 1 - 8 files changed, 53 insertions(+), 44 deletions(-) diff --git a/src/Analyser/Analyser.php b/src/Analyser/Analyser.php index 8858407c2d..ab53524a44 100644 --- a/src/Analyser/Analyser.php +++ b/src/Analyser/Analyser.php @@ -19,7 +19,6 @@ public function __construct( private FileAnalyser $fileAnalyser, private RuleRegistry $ruleRegistry, private CollectorRegistry $collectorRegistry, - private NodeScopeResolver $nodeScopeResolver, private int $internalErrorsCountLimit, ) { @@ -43,7 +42,6 @@ public function analyse( $allAnalysedFiles = $files; } - $this->nodeScopeResolver->setAnalysedFiles($allAnalysedFiles); $allAnalysedFiles = array_fill_keys($allAnalysedFiles, true); /** @var list $errors */ diff --git a/src/Analyser/FileAnalyser.php b/src/Analyser/FileAnalyser.php index f27181ccd7..2c58de922d 100644 --- a/src/Analyser/FileAnalyser.php +++ b/src/Analyser/FileAnalyser.php @@ -179,6 +179,7 @@ public function analyseFile( $parserNodes, $scope, $nodeCallback, + array_keys($analysedFiles), ); $unmatchedLineIgnores = $linesToIgnore; foreach ($temporaryFileErrors as $tmpFileError) { diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 0102032c72..7bb25e097a 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -181,7 +181,7 @@ class NodeScopeResolver private const GENERALIZE_AFTER_ITERATION = 1; /** @var bool[] filePath(string) => bool(true) */ - private array $analysedFiles = []; + private ?array $analysedFiles = null; /** @var array */ private array $earlyTerminatingMethodNames = []; @@ -221,25 +221,27 @@ public function __construct( } /** - * @api - * @param string[] $files + * @param ?string[] $files */ - public function setAnalysedFiles(array $files): void + private function setAnalysedFiles(?array $files): void { - $this->analysedFiles = array_fill_keys($files, true); + $this->analysedFiles = $files !== null ? array_fill_keys($files, true) : null; } /** * @api * @param Node[] $nodes * @param callable(Node $node, Scope $scope): void $nodeCallback + * @param ?string[] $analysedFiles */ public function processNodes( array $nodes, MutatingScope $scope, callable $nodeCallback, + ?array $analysedFiles = null, ): void { + $this->setAnalysedFiles($analysedFiles); $nodesCount = count($nodes); foreach ($nodes as $i => $node) { if (!$node instanceof Node\Stmt) { @@ -268,6 +270,7 @@ public function processNodes( * @api * @param Node\Stmt[] $stmts * @param callable(Node $node, Scope $scope): void $nodeCallback + * @param ?string[] $analysedFiles */ public function processStmtNodes( Node $parentNode, @@ -275,6 +278,24 @@ public function processStmtNodes( MutatingScope $scope, callable $nodeCallback, ?StatementContext $context = null, + ?array $analysedFiles = null, + ): StatementResult + { + $this->setAnalysedFiles($analysedFiles); + return $this->iterateStmtNodes($parentNode, $stmts, $scope, $nodeCallback, $context); + } + + /** + * @api + * @param Node\Stmt[] $stmts + * @param callable(Node $node, Scope $scope): void $nodeCallback + */ + private function iterateStmtNodes( + Node $parentNode, + array $stmts, + MutatingScope $scope, + callable $nodeCallback, + ?StatementContext $context = null, ): StatementResult { if ($context === null) { @@ -449,7 +470,7 @@ private function processStmtNode( $gatheredReturnStatements = []; $executionEnds = []; - $statementResult = $this->processStmtNodes($stmt, $stmt->stmts, $functionScope, static function (Node $node, Scope $scope) use ($nodeCallback, $functionScope, &$gatheredReturnStatements, &$executionEnds): void { + $statementResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $functionScope, static function (Node $node, Scope $scope) use ($nodeCallback, $functionScope, &$gatheredReturnStatements, &$executionEnds): void { $nodeCallback($node, $scope); if ($scope->getFunction() !== $functionScope->getFunction()) { return; @@ -549,7 +570,7 @@ private function processStmtNode( if ($stmt->stmts !== null) { $gatheredReturnStatements = []; $executionEnds = []; - $statementResult = $this->processStmtNodes($stmt, $stmt->stmts, $methodScope, static function (Node $node, Scope $scope) use ($nodeCallback, $methodScope, &$gatheredReturnStatements, &$executionEnds): void { + $statementResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $methodScope, static function (Node $node, Scope $scope) use ($nodeCallback, $methodScope, &$gatheredReturnStatements, &$executionEnds): void { $nodeCallback($node, $scope); if ($scope->getFunction() !== $methodScope->getFunction()) { return; @@ -635,7 +656,7 @@ private function processStmtNode( $scope = $scope->enterNamespace($stmt->name->toString()); } - $scope = $this->processStmtNodes($stmt, $stmt->stmts, $scope, $nodeCallback, $context)->getScope(); + $scope = $this->iterateStmtNodes($stmt, $stmt->stmts, $scope, $nodeCallback, $context)->getScope(); $hasYield = false; $throwPoints = []; } elseif ($stmt instanceof Node\Stmt\Trait_) { @@ -665,7 +686,7 @@ private function processStmtNode( $classStatementsGatherer = new ClassStatementsGatherer($classReflection, $nodeCallback); $this->processAttributeGroups($stmt->attrGroups, $classScope, $classStatementsGatherer); - $this->processStmtNodes($stmt, $stmt->stmts, $classScope, $classStatementsGatherer, $context); + $this->iterateStmtNodes($stmt, $stmt->stmts, $classScope, $classStatementsGatherer, $context); $nodeCallback(new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensionProvider, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls()), $classScope); $nodeCallback(new ClassMethodsNode($stmt, $classStatementsGatherer->getMethods(), $classStatementsGatherer->getMethodCalls()), $classScope); $nodeCallback(new ClassConstantsNode($stmt, $classStatementsGatherer->getConstants(), $classStatementsGatherer->getConstantFetches()), $classScope); @@ -732,7 +753,7 @@ private function processStmtNode( $alwaysTerminating = true; $hasYield = $condResult->hasYield(); - $branchScopeStatementResult = $this->processStmtNodes($stmt, $stmt->stmts, $condResult->getTruthyScope(), $nodeCallback, $context); + $branchScopeStatementResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $condResult->getTruthyScope(), $nodeCallback, $context); if (!$conditionType instanceof ConstantBooleanType || $conditionType->getValue()) { $exitPoints = $branchScopeStatementResult->getExitPoints(); @@ -753,7 +774,7 @@ private function processStmtNode( $condResult = $this->processExprNode($elseif->cond, $condScope, $nodeCallback, ExpressionContext::createDeep()); $throwPoints = array_merge($throwPoints, $condResult->getThrowPoints()); $condScope = $condResult->getScope(); - $branchScopeStatementResult = $this->processStmtNodes($elseif, $elseif->stmts, $condResult->getTruthyScope(), $nodeCallback, $context); + $branchScopeStatementResult = $this->iterateStmtNodes($elseif, $elseif->stmts, $condResult->getTruthyScope(), $nodeCallback, $context); if ( !$ifAlwaysTrue @@ -790,7 +811,7 @@ private function processStmtNode( } } else { $nodeCallback($stmt->else, $scope); - $branchScopeStatementResult = $this->processStmtNodes($stmt->else, $stmt->else->stmts, $scope, $nodeCallback, $context); + $branchScopeStatementResult = $this->iterateStmtNodes($stmt->else, $stmt->else->stmts, $scope, $nodeCallback, $context); if (!$ifAlwaysTrue && !$lastElseIfConditionIsTrue) { $exitPoints = array_merge($exitPoints, $branchScopeStatementResult->getExitPoints()); @@ -833,7 +854,7 @@ private function processStmtNode( $prevScope = $bodyScope; $bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope); $bodyScope = $this->enterForeach($bodyScope, $stmt); - $bodyScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $bodyScope, static function (): void { + $bodyScopeResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $bodyScope, static function (): void { }, $context->enterDeep())->filterOutLoopExitPoints(); $bodyScope = $bodyScopeResult->getScope(); foreach ($bodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { @@ -852,7 +873,7 @@ private function processStmtNode( $bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope); $bodyScope = $this->enterForeach($bodyScope, $stmt); - $finalScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $bodyScope, $nodeCallback, $context)->filterOutLoopExitPoints(); + $finalScopeResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $bodyScope, $nodeCallback, $context)->filterOutLoopExitPoints(); $finalScope = $finalScopeResult->getScope(); foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { $finalScope = $continueExitPoint->getScope()->mergeWith($finalScope); @@ -901,7 +922,7 @@ private function processStmtNode( $bodyScope = $bodyScope->mergeWith($scope); $bodyScope = $this->processExprNode($stmt->cond, $bodyScope, static function (): void { }, ExpressionContext::createDeep())->getTruthyScope(); - $bodyScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $bodyScope, static function (): void { + $bodyScopeResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $bodyScope, static function (): void { }, $context->enterDeep())->filterOutLoopExitPoints(); $bodyScope = $bodyScopeResult->getScope(); foreach ($bodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { @@ -921,7 +942,7 @@ private function processStmtNode( $bodyScope = $bodyScope->mergeWith($scope); $bodyScopeMaybeRan = $bodyScope; $bodyScope = $this->processExprNode($stmt->cond, $bodyScope, $nodeCallback, ExpressionContext::createDeep())->getTruthyScope(); - $finalScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $bodyScope, $nodeCallback, $context)->filterOutLoopExitPoints(); + $finalScopeResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $bodyScope, $nodeCallback, $context)->filterOutLoopExitPoints(); $finalScope = $finalScopeResult->getScope()->filterByFalseyValue($stmt->cond); foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { $finalScope = $finalScope->mergeWith($continueExitPoint->getScope()); @@ -976,7 +997,7 @@ private function processStmtNode( do { $prevScope = $bodyScope; $bodyScope = $bodyScope->mergeWith($scope); - $bodyScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $bodyScope, static function (): void { + $bodyScopeResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $bodyScope, static function (): void { }, $context->enterDeep())->filterOutLoopExitPoints(); $alwaysTerminating = $bodyScopeResult->isAlwaysTerminating(); $bodyScope = $bodyScopeResult->getScope(); @@ -1002,7 +1023,7 @@ private function processStmtNode( $bodyScope = $bodyScope->mergeWith($scope); } - $bodyScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $bodyScope, $nodeCallback, $context)->filterOutLoopExitPoints(); + $bodyScopeResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $bodyScope, $nodeCallback, $context)->filterOutLoopExitPoints(); $bodyScope = $bodyScopeResult->getScope(); foreach ($bodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { $bodyScope = $bodyScope->mergeWith($continueExitPoint->getScope()); @@ -1078,7 +1099,7 @@ private function processStmtNode( $bodyScope = $this->processExprNode($condExpr, $bodyScope, static function (): void { }, ExpressionContext::createDeep())->getTruthyScope(); } - $bodyScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $bodyScope, static function (): void { + $bodyScopeResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $bodyScope, static function (): void { }, $context->enterDeep())->filterOutLoopExitPoints(); $bodyScope = $bodyScopeResult->getScope(); foreach ($bodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { @@ -1108,7 +1129,7 @@ private function processStmtNode( $bodyScope = $this->processExprNode($condExpr, $bodyScope, $nodeCallback, ExpressionContext::createDeep())->getTruthyScope(); } - $finalScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $bodyScope, $nodeCallback, $context)->filterOutLoopExitPoints(); + $finalScopeResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $bodyScope, $nodeCallback, $context)->filterOutLoopExitPoints(); $finalScope = $finalScopeResult->getScope(); foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { $finalScope = $continueExitPoint->getScope()->mergeWith($finalScope); @@ -1178,7 +1199,7 @@ private function processStmtNode( } $branchScope = $branchScope->mergeWith($prevScope); - $branchScopeResult = $this->processStmtNodes($caseNode, $caseNode->stmts, $branchScope, $nodeCallback, $context); + $branchScopeResult = $this->iterateStmtNodes($caseNode, $caseNode->stmts, $branchScope, $nodeCallback, $context); $branchScope = $branchScopeResult->getScope(); $branchFinalScopeResult = $branchScopeResult->filterOutLoopExitPoints(); $hasYield = $hasYield || $branchFinalScopeResult->hasYield(); @@ -1222,7 +1243,7 @@ private function processStmtNode( return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPointsForOuterLoop, $throwPoints); } elseif ($stmt instanceof TryCatch) { - $branchScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $scope, $nodeCallback, $context); + $branchScopeResult = $this->iterateStmtNodes($stmt, $stmt->stmts, $scope, $nodeCallback, $context); $branchScope = $branchScopeResult->getScope(); $finalScope = $branchScopeResult->isAlwaysTerminating() ? null : $branchScope; @@ -1325,7 +1346,7 @@ private function processStmtNode( $variableName = $catchNode->var->name; } - $catchScopeResult = $this->processStmtNodes($catchNode, $catchNode->stmts, $catchScope->enterCatchType($catchType, $variableName), $nodeCallback, $context); + $catchScopeResult = $this->iterateStmtNodes($catchNode, $catchNode->stmts, $catchScope->enterCatchType($catchType, $variableName), $nodeCallback, $context); $catchScopeForFinally = $catchScopeResult->getScope(); $finalScope = $catchScopeResult->isAlwaysTerminating() ? $finalScope : $catchScopeResult->getScope()->mergeWith($finalScope); @@ -1369,7 +1390,7 @@ private function processStmtNode( if ($finallyScope !== null && $stmt->finally !== null) { $originalFinallyScope = $finallyScope; - $finallyResult = $this->processStmtNodes($stmt->finally, $stmt->finally->stmts, $finallyScope, $nodeCallback, $context); + $finallyResult = $this->iterateStmtNodes($stmt->finally, $stmt->finally->stmts, $finallyScope, $nodeCallback, $context); $alwaysTerminating = $alwaysTerminating || $finallyResult->isAlwaysTerminating(); $hasYield = $hasYield || $finallyResult->hasYield(); $throwPointsForLater = array_merge($throwPointsForLater, $finallyResult->getThrowPoints()); @@ -3167,7 +3188,7 @@ private function processClosureNode( $gatheredReturnStatements[] = new ReturnStatement($scope, $node); }; if (count($byRefUses) === 0) { - $statementResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback, StatementContext::createTopLevel()); + $statementResult = $this->iterateStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback, StatementContext::createTopLevel()); $nodeCallback(new ClosureReturnStatementsNode( $expr, $gatheredReturnStatements, @@ -3182,7 +3203,7 @@ private function processClosureNode( do { $prevScope = $closureScope; - $intermediaryClosureScopeResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, static function (): void { + $intermediaryClosureScopeResult = $this->iterateStmtNodes($expr, $expr->stmts, $closureScope, static function (): void { }, StatementContext::createTopLevel()); $intermediaryClosureScope = $intermediaryClosureScopeResult->getScope(); foreach ($intermediaryClosureScopeResult->getExitPoints() as $exitPoint) { @@ -3199,7 +3220,7 @@ private function processClosureNode( $count++; } while ($count < self::LOOP_SCOPE_ITERATIONS); - $statementResult = $this->processStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback, StatementContext::createTopLevel()); + $statementResult = $this->iterateStmtNodes($expr, $expr->stmts, $closureScope, $closureStmtsCallback, StatementContext::createTopLevel()); $nodeCallback(new ClosureReturnStatementsNode( $expr, $gatheredReturnStatements, @@ -4075,7 +4096,7 @@ private function processTraitUse(Node\Stmt\TraitUse $node, MutatingScope $classS continue; // trait from eval or from PHP itself } $fileName = $this->fileHelper->normalizePath($traitFileName); - if (!isset($this->analysedFiles[$fileName])) { + if ($this->analysedFiles !== null && !isset($this->analysedFiles[$fileName])) { continue; } $parserNodes = $this->parser->parseFile($fileName); @@ -4119,7 +4140,7 @@ private function processNodesForTraitUse($node, ClassReflection $traitReflection $methodAst->flags = ($methodAst->flags & ~ Node\Stmt\Class_::VISIBILITY_MODIFIER_MASK) | $methodModifiers[$methodName]; $stmts[$i] = $methodAst; } - $this->processStmtNodes($node, $stmts, $scope->enterTrait($traitReflection), $nodeCallback, StatementContext::createTopLevel()); + $this->iterateStmtNodes($node, $stmts, $scope->enterTrait($traitReflection), $nodeCallback, StatementContext::createTopLevel()); return; } if ($node instanceof Node\Stmt\ClassLike) { diff --git a/src/Command/WorkerCommand.php b/src/Command/WorkerCommand.php index 232d299fad..4fe54753ea 100644 --- a/src/Command/WorkerCommand.php +++ b/src/Command/WorkerCommand.php @@ -5,7 +5,6 @@ use Clue\React\NDJson\Decoder; use Clue\React\NDJson\Encoder; use PHPStan\Analyser\FileAnalyser; -use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Collectors\Registry as CollectorRegistry; use PHPStan\DependencyInjection\Container; use PHPStan\File\PathNotFoundException; @@ -134,10 +133,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - /** @var NodeScopeResolver $nodeScopeResolver */ - $nodeScopeResolver = $container->getByType(NodeScopeResolver::class); - $nodeScopeResolver->setAnalysedFiles($analysedFiles); - $analysedFiles = array_fill_keys($analysedFiles, true); $tcpConector = new TcpConnector($loop); diff --git a/src/PhpDoc/StubValidator.php b/src/PhpDoc/StubValidator.php index 578b719bb0..8f115b8c4a 100644 --- a/src/PhpDoc/StubValidator.php +++ b/src/PhpDoc/StubValidator.php @@ -4,7 +4,6 @@ use PHPStan\Analyser\Error; use PHPStan\Analyser\FileAnalyser; -use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Broker\Broker; use PHPStan\Collectors\Registry as CollectorRegistry; use PHPStan\DependencyInjection\Container; @@ -94,10 +93,6 @@ public function validate(array $stubFiles, bool $debug): array /** @var FileAnalyser $fileAnalyser */ $fileAnalyser = $container->getByType(FileAnalyser::class); - /** @var NodeScopeResolver $nodeScopeResolver */ - $nodeScopeResolver = $container->getByType(NodeScopeResolver::class); - $nodeScopeResolver->setAnalysedFiles($stubFiles); - $pathRoutingParser = $container->getService('pathRoutingParser'); $pathRoutingParser->setAnalysedFiles($stubFiles); diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php index 6c260beca9..cb7023f3eb 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -110,7 +110,6 @@ private function getAnalyser(): Analyser $fileAnalyser, $ruleRegistry, $collectorRegistry, - $nodeScopeResolver, 50, ); } diff --git a/src/Testing/TypeInferenceTestCase.php b/src/Testing/TypeInferenceTestCase.php index 070fbd9292..08ceb6e0de 100644 --- a/src/Testing/TypeInferenceTestCase.php +++ b/src/Testing/TypeInferenceTestCase.php @@ -62,7 +62,7 @@ public function processFile( $this->getEarlyTerminatingFunctionCalls(), true, ); - $resolver->setAnalysedFiles(array_map(static fn (string $file): string => $fileHelper->normalizePath($file), array_merge([$file], $this->getAdditionalAnalysedFiles()))); + $analysedFiles = array_map(static fn (string $file): string => $fileHelper->normalizePath($file), array_merge([$file], $this->getAdditionalAnalysedFiles())); $scopeFactory = $this->createScopeFactory($reflectionProvider, $typeSpecifier, $dynamicConstantNames); $scope = $scopeFactory->create(ScopeContext::create($file)); @@ -71,6 +71,7 @@ public function processFile( $this->getParser()->parseFile($file), $scope, $callback, + $analysedFiles, ); } diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 57d216fe21..2a0c343d31 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -622,7 +622,6 @@ private function createAnalyser(bool $reportUnmatchedIgnoredErrors): Analyser $fileAnalyser, $ruleRegistry, $collectorRegistry, - $nodeScopeResolver, 50, ); }