AnalyserResultFinalizer - DRY of running CollectedDataNode rules

phpstan · Apr 20, 2024 · 38e2c96 · 38e2c96
1 parent 4c0e939
commit 38e2c96
Show file tree

Hide file tree

Showing 7 changed files with 106 additions and 132 deletions.
diff --git a/conf/config.neon b/conf/config.neon
@@ -476,6 +476,9 @@ services:
 		arguments:
 			internalErrorsCountLimit: %internalErrorsCountLimit%
 
+	-
+		class: PHPStan\Analyser\AnalyserResultFinalizer
+
 	-
 		class: PHPStan\Analyser\FileAnalyser
 		arguments:

diff --git a/src/Analyser/AnalyserResultFinalizer.php b/src/Analyser/AnalyserResultFinalizer.php
@@ -0,0 +1,75 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Analyser;
+
+use PHPStan\AnalysedCodeException;
+use PHPStan\BetterReflection\NodeCompiler\Exception\UnableToCompileNode;
+use PHPStan\BetterReflection\Reflection\Exception\CircularReference;
+use PHPStan\BetterReflection\Reflector\Exception\IdentifierNotFound;
+use PHPStan\Node\CollectedDataNode;
+use PHPStan\Rules\Registry as RuleRegistry;
+use function count;
+use function sprintf;
+
+class AnalyserResultFinalizer
+{
+
+	public function __construct(
+		private RuleRegistry $ruleRegistry,
+		private RuleErrorTransformer $ruleErrorTransformer,
+		private ScopeFactory $scopeFactory,
+	)
+	{
+	}
+
+	public function finalize(AnalyserResult $analyserResult, bool $onlyFiles): AnalyserResult
+	{
+		if (count($analyserResult->getCollectedData()) === 0) {
+			return $analyserResult;
+		}
+
+		$hasInternalErrors = count($analyserResult->getInternalErrors()) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit();
+		if ($hasInternalErrors) {
+			return $analyserResult;
+		}
+
+		$nodeType = CollectedDataNode::class;
+		$node = new CollectedDataNode($analyserResult->getCollectedData(), $onlyFiles);
+
+		$file = 'N/A';
+		$scope = $this->scopeFactory->create(ScopeContext::create($file));
+		$errors = $analyserResult->getUnorderedErrors();
+		foreach ($this->ruleRegistry->getRules($nodeType) as $rule) {
+			try {
+				$ruleErrors = $rule->processNode($node, $scope);
+			} catch (AnalysedCodeException $e) {
+				$errors[] = (new Error($e->getMessage(), $file, $node->getStartLine(), $e, null, null, $e->getTip()))->withIdentifier('phpstan.internal');
+				continue;
+			} catch (IdentifierNotFound $e) {
+				$errors[] = (new Error(sprintf('Reflection error: %s not found.', $e->getIdentifier()->getName()), $file, $node->getStartLine(), $e, null, null, 'Learn more at https://phpstan.org/user-guide/discovering-symbols'))->withIdentifier('phpstan.reflection');
+				continue;
+			} catch (UnableToCompileNode | CircularReference $e) {
+				$errors[] = (new Error(sprintf('Reflection error: %s', $e->getMessage()), $file, $node->getStartLine(), $e))->withIdentifier('phpstan.reflection');
+				continue;
+			}
+
+			foreach ($ruleErrors as $ruleError) {
+				$errors[] = $this->ruleErrorTransformer->transform($ruleError, $scope, $nodeType, $node->getStartLine());
+			}
+		}
+
+		return new AnalyserResult(
+			$errors,
+			$analyserResult->getLocallyIgnoredErrors(),
+			$analyserResult->getLinesToIgnore(),
+			$analyserResult->getUnmatchedLineIgnores(),
+			$analyserResult->getInternalErrors(),
+			$analyserResult->getCollectedData(),
+			$analyserResult->getDependencies(),
+			$analyserResult->getExportedNodes(),
+			$analyserResult->hasReachedInternalErrorsCountLimit(),
+			$analyserResult->getPeakMemoryUsageBytes(),
+		);
+	}
+
+}
diff --git a/src/Command/AnalyseApplication.php b/src/Command/AnalyseApplication.php
@@ -2,23 +2,13 @@
 
 namespace PHPStan\Command;
 
-use PHPStan\AnalysedCodeException;
 use PHPStan\Analyser\AnalyserResult;
-use PHPStan\Analyser\Error;
+use PHPStan\Analyser\AnalyserResultFinalizer;
 use PHPStan\Analyser\Ignore\IgnoredErrorHelper;
 use PHPStan\Analyser\ResultCache\ResultCacheManagerFactory;
-use PHPStan\Analyser\RuleErrorTransformer;
-use PHPStan\Analyser\ScopeContext;
-use PHPStan\Analyser\ScopeFactory;
-use PHPStan\BetterReflection\NodeCompiler\Exception\UnableToCompileNode;
-use PHPStan\BetterReflection\Reflection\Exception\CircularReference;
-use PHPStan\BetterReflection\Reflector\Exception\IdentifierNotFound;
-use PHPStan\Collectors\CollectedData;
 use PHPStan\Internal\BytesHelper;
-use PHPStan\Node\CollectedDataNode;
 use PHPStan\PhpDoc\StubFilesProvider;
 use PHPStan\PhpDoc\StubValidator;
-use PHPStan\Rules\Registry as RuleRegistry;
 use PHPStan\ShouldNotHappenException;
 use Symfony\Component\Console\Input\InputInterface;
 use function array_merge;
@@ -34,14 +24,12 @@ class AnalyseApplication
 
 	public function __construct(
 		private AnalyserRunner $analyserRunner,
+		private AnalyserResultFinalizer $analyserResultFinalizer,
 		private StubValidator $stubValidator,
 		private ResultCacheManagerFactory $resultCacheManagerFactory,
 		private IgnoredErrorHelper $ignoredErrorHelper,
 		private int $internalErrorsCountLimit,
 		private StubFilesProvider $stubFilesProvider,
-		private RuleRegistry $ruleRegistry,
-		private ScopeFactory $scopeFactory,
-		private RuleErrorTransformer $ruleErrorTransformer,
 	)
 	{
 	}
@@ -114,7 +102,7 @@ public function analyse(
 			}
 
 			$resultCacheResult = $resultCacheManager->process($intermediateAnalyserResult, $resultCache, $errorOutput, $onlyFiles, true);
-			$analyserResult = $resultCacheResult->getAnalyserResult();
+			$analyserResult = $this->analyserResultFinalizer->finalize($resultCacheResult->getAnalyserResult(), $onlyFiles);
 			$internalErrors = $analyserResult->getInternalErrors();
 			$errors = $analyserResult->getErrors();
 			$hasInternalErrors = count($internalErrors) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit();
@@ -147,11 +135,6 @@ public function analyse(
 				}
 			}
 
-			if (!$hasInternalErrors) {
-				foreach ($this->getCollectedDataErrors($analyserResult->getCollectedData(), $onlyFiles) as $error) {
-					$errors[] = $error;
-				}
-			}
 			$ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($errors, $onlyFiles, $files, $hasInternalErrors);
 			$fileSpecificErrors = $ignoredErrorHelperProcessedResult->getNotIgnoredErrors();
 			$notFileSpecificErrors = $ignoredErrorHelperProcessedResult->getOtherIgnoreMessages();
@@ -178,39 +161,6 @@ public function analyse(
 		);
 	}
 
-	/**
-	 * @param CollectedData[] $collectedData
-	 * @return Error[]
-	 */
-	private function getCollectedDataErrors(array $collectedData, bool $onlyFiles): array
-	{
-		$nodeType = CollectedDataNode::class;
-		$node = new CollectedDataNode($collectedData, $onlyFiles);
-		$file = 'N/A';
-		$scope = $this->scopeFactory->create(ScopeContext::create($file));
-		$errors = [];
-		foreach ($this->ruleRegistry->getRules($nodeType) as $rule) {
-			try {
-				$ruleErrors = $rule->processNode($node, $scope);
-			} catch (AnalysedCodeException $e) {
-				$errors[] = (new Error($e->getMessage(), $file, $node->getStartLine(), $e, null, null, $e->getTip()))->withIdentifier('phpstan.internal');
-				continue;
-			} catch (IdentifierNotFound $e) {
-				$errors[] = (new Error(sprintf('Reflection error: %s not found.', $e->getIdentifier()->getName()), $file, $node->getStartLine(), $e, null, null, 'Learn more at https://phpstan.org/user-guide/discovering-symbols'))->withIdentifier('phpstan.reflection');
-				continue;
-			} catch (UnableToCompileNode | CircularReference $e) {
-				$errors[] = (new Error(sprintf('Reflection error: %s', $e->getMessage()), $file, $node->getStartLine(), $e))->withIdentifier('phpstan.reflection');
-				continue;
-			}
-
-			foreach ($ruleErrors as $ruleError) {
-				$errors[] = $this->ruleErrorTransformer->transform($ruleError, $scope, $nodeType, $node->getStartLine());
-			}
-		}
-
-		return $errors;
-	}
-
 	/**
 	 * @param string[] $files
 	 * @param string[] $allAnalysedFiles

diff --git a/src/Command/FixerWorkerCommand.php b/src/Command/FixerWorkerCommand.php
@@ -3,27 +3,18 @@
 namespace PHPStan\Command;
 
 use Clue\React\NDJson\Encoder;
-use PHPStan\AnalysedCodeException;
 use PHPStan\Analyser\AnalyserResult;
+use PHPStan\Analyser\AnalyserResultFinalizer;
 use PHPStan\Analyser\Error;
 use PHPStan\Analyser\Ignore\IgnoredErrorHelper;
 use PHPStan\Analyser\Ignore\IgnoredErrorHelperResult;
 use PHPStan\Analyser\ResultCache\ResultCacheManager;
 use PHPStan\Analyser\ResultCache\ResultCacheManagerFactory;
-use PHPStan\Analyser\RuleErrorTransformer;
-use PHPStan\Analyser\ScopeContext;
-use PHPStan\Analyser\ScopeFactory;
-use PHPStan\BetterReflection\NodeCompiler\Exception\UnableToCompileNode;
-use PHPStan\BetterReflection\Reflection\Exception\CircularReference;
-use PHPStan\BetterReflection\Reflector\Exception\IdentifierNotFound;
-use PHPStan\Collectors\CollectedData;
 use PHPStan\DependencyInjection\Container;
 use PHPStan\File\PathNotFoundException;
-use PHPStan\Node\CollectedDataNode;
 use PHPStan\Parallel\ParallelAnalyser;
 use PHPStan\Parallel\Scheduler;
 use PHPStan\Process\CpuCoreCounter;
-use PHPStan\Rules\Registry as RuleRegistry;
 use PHPStan\ShouldNotHappenException;
 use React\EventLoop\LoopInterface;
 use React\EventLoop\StreamSelectLoop;
@@ -143,6 +134,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			$resultCacheManager = $container->getByType(ResultCacheManagerFactory::class)->create();
 			$projectConfigArray = $inceptionResult->getProjectConfigArray();
 
+			/** @var AnalyserResultFinalizer $analyserResultFinalizer */
+			$analyserResultFinalizer = $container->getByType(AnalyserResultFinalizer::class);
+
 			try {
 				[$inceptionFiles, $isOnlyFiles] = $inceptionResult->getFiles();
 			} catch (PathNotFoundException | InceptionNotSuccessfulException) {
@@ -227,25 +221,21 @@ function (array $errors, array $locallyIgnoredErrors, array $analysedFiles) use
 						],
 					]);
 				},
-			)->then(function (AnalyserResult $intermediateAnalyserResult) use ($resultCacheManager, $resultCache, $inceptionResult, $container, $isOnlyFiles, $ignoredErrorHelperResult, $inceptionFiles, $out): void {
+			)->then(function (AnalyserResult $intermediateAnalyserResult) use ($analyserResultFinalizer, $resultCacheManager, $resultCache, $inceptionResult, $isOnlyFiles, $ignoredErrorHelperResult, $inceptionFiles, $out): void {
 				$result = $resultCacheManager->process(
 					$intermediateAnalyserResult,
 					$resultCache,
 					$inceptionResult->getErrorOutput(),
 					false,
 					true,
 				)->getAnalyserResult();
+				$result = $analyserResultFinalizer->finalize($result, $isOnlyFiles);
 
 				$hasInternalErrors = count($result->getInternalErrors()) > 0 || $result->hasReachedInternalErrorsCountLimit();
 
 				$collectorErrors = [];
 				$intermediateErrors = $result->getErrors();
-				if (!$hasInternalErrors) {
-					foreach ($this->getCollectedDataErrors($container, $result->getCollectedData(), $isOnlyFiles) as $error) {
-						$collectorErrors[] = $error;
-						$intermediateErrors[] = $error;
-					}
-				} else {
+				if ($hasInternalErrors) {
 					$out->write(['action' => 'analysisCrash', 'data' => [
 						'errors' => count($result->getInternalErrors()) > 0 ? $result->getInternalErrors() : [
 							'Internal error occurred',
@@ -323,42 +313,6 @@ private function filterErrors(array $errors, IgnoredErrorHelperResult $ignoredEr
 		];
 	}
 
-	/**
-	 * @param CollectedData[] $collectedData
-	 * @return Error[]
-	 */
-	private function getCollectedDataErrors(Container $container, array $collectedData, bool $onlyFiles): array
-	{
-		$nodeType = CollectedDataNode::class;
-		$node = new CollectedDataNode($collectedData, $onlyFiles);
-		$file = 'N/A';
-		$scope = $container->getByType(ScopeFactory::class)->create(ScopeContext::create($file));
-		$ruleRegistry = $container->getByType(RuleRegistry::class);
-		$ruleErrorTransformer = $container->getByType(RuleErrorTransformer::class);
-
-		$errors = [];
-		foreach ($ruleRegistry->getRules($nodeType) as $rule) {
-			try {
-				$ruleErrors = $rule->processNode($node, $scope);
-			} catch (AnalysedCodeException $e) {
-				$errors[] = (new Error($e->getMessage(), $file, $node->getStartLine(), $e, null, null, $e->getTip()))->withIdentifier('phpstan.internal');
-				continue;
-			} catch (IdentifierNotFound $e) {
-				$errors[] = (new Error(sprintf('Reflection error: %s not found.', $e->getIdentifier()->getName()), $file, $node->getStartLine(), $e, null, null, 'Learn more at https://phpstan.org/user-guide/discovering-symbols'))->withIdentifier('phpstan.reflection');
-				continue;
-			} catch (UnableToCompileNode | CircularReference $e) {
-				$errors[] = (new Error(sprintf('Reflection error: %s', $e->getMessage()), $file, $node->getStartLine(), $e))->withIdentifier('phpstan.reflection');
-				continue;
-			}
-
-			foreach ($ruleErrors as $ruleError) {
-				$errors[] = $ruleErrorTransformer->transform($ruleError, $scope, $nodeType, $node->getStartLine());
-			}
-		}
-
-		return $errors;
-	}
-
 	/**
 	 * @param string[] $files
 	 * @param callable(list<Error>, list<Error>, string[]): void $onFileAnalysisHandler

diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php
@@ -4,18 +4,17 @@
 
 use PhpParser\Node;
 use PHPStan\Analyser\Analyser;
+use PHPStan\Analyser\AnalyserResultFinalizer;
 use PHPStan\Analyser\Error;
 use PHPStan\Analyser\FileAnalyser;
 use PHPStan\Analyser\NodeScopeResolver;
 use PHPStan\Analyser\RuleErrorTransformer;
-use PHPStan\Analyser\ScopeContext;
 use PHPStan\Analyser\TypeSpecifier;
 use PHPStan\Collectors\Collector;
 use PHPStan\Collectors\Registry as CollectorRegistry;
 use PHPStan\Dependency\DependencyResolver;
 use PHPStan\DependencyInjection\Type\DynamicThrowTypeExtensionProvider;
 use PHPStan\File\FileHelper;
-use PHPStan\Node\CollectedDataNode;
 use PHPStan\Php\PhpVersion;
 use PHPStan\PhpDoc\PhpDocInheritanceResolver;
 use PHPStan\PhpDoc\StubPhpDocProvider;
@@ -177,26 +176,15 @@ public function gatherAnalyserErrors(array $files): array
 			$this->fail(implode("\n", $analyserResult->getInternalErrors()));
 		}
 
-		$actualErrors = $analyserResult->getUnorderedErrors();
-		$ruleErrorTransformer = new RuleErrorTransformer();
-		if (count($analyserResult->getCollectedData()) > 0) {
-			$ruleRegistry = new DirectRuleRegistry([
+		$finalizer = new AnalyserResultFinalizer(
+			new DirectRuleRegistry([
 				$this->getRule(),
-			]);
-
-			$nodeType = CollectedDataNode::class;
-			$node = new CollectedDataNode($analyserResult->getCollectedData(), false);
-			$scopeFactory = $this->createScopeFactory($this->createReflectionProvider(), $this->getTypeSpecifier());
-			$scope = $scopeFactory->create(ScopeContext::create('irrelevant'));
-			foreach ($ruleRegistry->getRules($nodeType) as $rule) {
-				$ruleErrors = $rule->processNode($node, $scope);
-				foreach ($ruleErrors as $ruleError) {
-					$actualErrors[] = $ruleErrorTransformer->transform($ruleError, $scope, $nodeType, $node->getStartLine());
-				}
-			}
-		}
+			]),
+			new RuleErrorTransformer(),
+			$this->createScopeFactory($this->createReflectionProvider(), $this->getTypeSpecifier()),
+		);
 
-		return $actualErrors;
+		return $finalizer->finalize($analyserResult, false)->getUnorderedErrors();
 	}
 
 	protected function shouldPolluteScopeWithLoopInitialAssignments(): bool

diff --git a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php
@@ -1339,7 +1339,11 @@ private function runAnalyse(string $file, ?array $allAnalysedFiles = null): arra
 		$file = $this->getFileHelper()->normalizePath($file);
 
 		$analyser = self::getContainer()->getByType(Analyser::class);
-		$errors = $analyser->analyse([$file], null, null, true, $allAnalysedFiles)->getErrors();
+		$finalizer = self::getContainer()->getByType(AnalyserResultFinalizer::class);
+		$errors = $finalizer->finalize(
+			$analyser->analyse([$file], null, null, true, $allAnalysedFiles),
+			false,
+		)->getErrors();
 		foreach ($errors as $error) {
 			$this->assertSame($file, $error->getFilePath());
 		}

diff --git a/tests/PHPStan/Analyser/data/bug-6160.php b/tests/PHPStan/Analyser/data/bug-6160.php
@@ -16,10 +16,10 @@ public static function split($flags = 0){
 
 	 public static function test(): void
 	 {
-		 self::split(94561); // should error
-		 self::split(PREG_SPLIT_NO_EMPTY); // should work
-		 self::split(PREG_SPLIT_DELIM_CAPTURE); // should work
-		 self::split(PREG_SPLIT_NO_EMPTY_COPY); // should work
-		 self::split("sdf"); // should error
+		 $a = self::split(94561); // should error
+		 $a = self::split(PREG_SPLIT_NO_EMPTY); // should work
+		 $a = self::split(PREG_SPLIT_DELIM_CAPTURE); // should work
+		 $a = self::split(PREG_SPLIT_NO_EMPTY_COPY); // should work
+		 $a = self::split("sdf"); // should error
 	 }
 }