Re-analyse files with errors of rules on collected data when a symbol appears

.github/workflows/e2e-tests.yml

@@ -1622,6 +1622,20 @@ jobs:
               OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
               echo "$OUTPUT"
               ../bashunit assert contains 'register a class in the container; re-analysing everything.' "$OUTPUT"
+          - script: |
+              cd e2e/result-cache-collector-sweep
+              # Two errors: one from an ordinary rule, one from a rule built on collected data.
+              OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
+              echo "$OUTPUT"
+              ../bashunit assert contains 'PlainCaller.php:8:Method ResultCacheE2ECollectorSweep\PlainCaller::doPlainCaller() has invalid return type ResultCacheE2ECollectorSweep\Missing.' "$OUTPUT"
+              ../bashunit assert contains 'Caller.php:7:Call to function resultCacheE2ECollectorHelper() on a separate line has no effect.' "$OUTPUT"
+              # One new file defines both missing symbols. The files with errors are re-analysed - including
+              # the file holding the error of the collector-backed rule, which is not in the cached error map.
+              cp appears/nsfns.php src/nsfns.php
+              OUTPUT=$(../bashunit assert exit_code "0" "../../bin/phpstan analyse -vv --error-format raw")
+              echo "$OUTPUT"
+              ../bashunit assert not_contains 'has invalid return type' "$OUTPUT"
+              ../bashunit assert not_contains 'on a separate line has no effect' "$OUTPUT"
           - script: |
               cd e2e/result-cache-extension-update
               composer install

e2e/result-cache-collector-sweep/appears/nsfns.php

@@ -0,0 +1,15 @@
+<?php declare(strict_types = 1);
+
+namespace ResultCacheE2ECollectorSweep;
+
+function resultCacheE2ECollectorHelper(): int
+{
+	echo 'side effect';
+
+	return 1;
+}
+
+class Missing
+{
+
+}

e2e/result-cache-collector-sweep/phpstan.neon

@@ -0,0 +1,5 @@
+parameters:
+	level: 4
+	tmpDir: tmp
+	paths:
+		- src

e2e/result-cache-collector-sweep/src/Caller.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types = 1);
+
+namespace ResultCacheE2ECollectorSweep;
+
+function caller(): void
+{
+	resultCacheE2ECollectorHelper();
+}

e2e/result-cache-collector-sweep/src/PlainCaller.php

@@ -0,0 +1,13 @@
+<?php declare(strict_types = 1);
+
+namespace ResultCacheE2ECollectorSweep;
+
+class PlainCaller
+{
+
+	public function doPlainCaller(): ?Missing
+	{
+		return null;
+	}
+
+}

e2e/result-cache-collector-sweep/src/globalfns.php

@@ -0,0 +1,6 @@
+<?php declare(strict_types = 1);
+
+function resultCacheE2ECollectorHelper(): int
+{
+	return 1;
+}

e2e/result-cache-collector-sweep/tmp/.gitignore

@@ -0,0 +1,2 @@
+*
+!.*

src/Analyser/ResultCache/ResultCache.php

@@ -31,6 +31,7 @@ final class ResultCache
 	 * @param array<string, array{string, bool, string}> $projectExtensionFiles
 	 * @param array<string, string> $currentFileHashes
 	 * @param array<string, CollectedDataRuleResult> $collectedDataRuleResults
+	 * @param list<string> $filesWithCollectedDataRuleErrors the files with errors of the rules on CollectedDataNode
 	 */
 	public function __construct(
 		private array $filesToAnalyse,
@@ -52,6 +53,7 @@ public function __construct(
 		private array $projectExtensionFiles,
 		private array $currentFileHashes,
 		private array $collectedDataRuleResults = [],
+		private array $filesWithCollectedDataRuleErrors = [],
 	)
 	{
 	}
@@ -203,4 +205,12 @@ public function getCollectedDataRuleResults(): array
 		return $this->collectedDataRuleResults;
 	}
 
+	/**
+	 * @return list<string>
+	 */
+	public function getFilesWithCollectedDataRuleErrors(): array
+	{
+		return $this->filesWithCollectedDataRuleErrors;
+	}
+
 }

src/Analyser/ResultCache/ResultCacheManager.php

@@ -7,6 +7,7 @@
 use PHPStan\Analyser\CollectedDataRuleResult;
 use PHPStan\Analyser\Error;
 use PHPStan\Analyser\FileAnalyserResult;
+use PHPStan\Analyser\FinalizerResult;
 use PHPStan\Analyser\ValueDependencyCollector;
 use PHPStan\Collectors\CollectedData;
 use PHPStan\Command\Output;
@@ -42,6 +43,7 @@
 use function array_intersect;
 use function array_key_exists;
 use function array_keys;
+use function array_map;
 use function array_merge;
 use function array_unique;
 use function array_values;
@@ -106,7 +108,7 @@ final class ResultCacheManager
 	 */
 	private const EXTENSIONS_NOT_INVALIDATING_CACHE = ['xdebug', 'blackfire', 'phpstan_turbo'];
 
-	private const CACHE_VERSION = 'v23-collectedDataRuleResults';
+	private const CACHE_VERSION = 'v24-filesWithCollectedDataRuleErrors';
 
 	/**
 	 * The recorded hash of a dependency that does not exist. A rule can depend on a path rather than on
@@ -716,6 +718,18 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
 				}
 			}
 			$collectedDataRuleResults = $transformer->absolutizeCollectedDataRuleResults($collectedDataRuleResults);
+			$storedFilesWithCollectedDataRuleErrors = $data['filesWithCollectedDataRuleErrors'] ?? [];
+			if (!is_array($storedFilesWithCollectedDataRuleErrors)) {
+				throw new RuntimeException('The cached files with errors of the rules on collected data are not an array.');
+			}
+			$filesWithCollectedDataRuleErrors = [];
+			foreach ($storedFilesWithCollectedDataRuleErrors as $fileWithCollectedDataRuleError) {
+				if (!is_string($fileWithCollectedDataRuleError)) {
+					throw new RuntimeException('A cached file with errors of the rules on collected data is not a string.');
+				}
+
+				$filesWithCollectedDataRuleErrors[] = $transformer->absolutizePath($fileWithCollectedDataRuleError);
+			}
 		} catch (Throwable $e) {
 			@unlink($cacheFilePath);
 
@@ -775,10 +789,20 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
 
 			$changedAnalysedFiles[$analysedFile] = true;
 		}
+		// The errors of the rules on CollectedDataNode are not in $errors: those rules run on the merged
+		// collected data on every run, a cached copy would be reported twice. But such an error waits
+		// for a symbol to appear just like any other, and re-analysing its file renews the file's share
+		// of the collected data.
+		$cachedFilesWithCollectedDataRuleErrors = array_fill_keys($filesWithCollectedDataRuleErrors, true);
+		$filteredFilesWithCollectedDataRuleErrors = [];
+
 		// only the stub files with errors so far, which are never analysed
 		$filesWithErrorsAllChanged = $filteredErrors === [];
 		foreach ($allAnalysedFiles as $analysedFile) {
-			if (!array_key_exists($analysedFile, $errors) || array_key_exists($analysedFile, $changedAnalysedFiles)) {
+			if (
+				(!array_key_exists($analysedFile, $errors) && !array_key_exists($analysedFile, $cachedFilesWithCollectedDataRuleErrors))
+				|| array_key_exists($analysedFile, $changedAnalysedFiles)
+			) {
 				continue;
 			}
 
@@ -790,6 +814,9 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
 			if (array_key_exists($analysedFile, $errors)) {
 				$filteredErrors[$analysedFile] = $errors[$analysedFile];
 			}
+			if (array_key_exists($analysedFile, $cachedFilesWithCollectedDataRuleErrors)) {
+				$filteredFilesWithCollectedDataRuleErrors[] = $analysedFile;
+			}
 			if (array_key_exists($analysedFile, $locallyIgnoredErrors)) {
 				$filteredLocallyIgnoredErrors[$analysedFile] = $locallyIgnoredErrors[$analysedFile];
 			}
@@ -881,6 +908,9 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
 			}
 		}
 
+		// in the order process() compares them in
+		sort($filteredFilesWithCollectedDataRuleErrors);
+
 		// the freshly computed metadata, so a file that stopped being scanned is not counted
 		$scannedFiles = is_array($meta['scannedFiles'] ?? null) ? $meta['scannedFiles'] : [];
 		foreach (array_keys($notAnalysedFiles) as $notAnalysedFile) {
@@ -1036,6 +1066,9 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
 
 				$filesToAnalyse[] = $fileWithError;
 			}
+			foreach ($filteredFilesWithCollectedDataRuleErrors as $fileWithError) {
+				$filesToAnalyse[] = $fileWithError;
+			}
 		}
 
 		foreach ($packageSeededFiles as $packageSeededFile) {
@@ -1105,6 +1138,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
 			projectExtensionFiles: $data['projectExtensionFiles'],
 			currentFileHashes: $currentFileHashes,
 			collectedDataRuleResults: $collectedDataRuleResults,
+			filesWithCollectedDataRuleErrors: $filteredFilesWithCollectedDataRuleErrors,
 		);
 	}
 
@@ -1251,7 +1285,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache
 			$projectConfigArray = $this->getPathTransformer()->relativizeProjectConfig($projectConfigArray);
 			$meta['projectConfig'] = Neon::encode($projectConfigArray);
 		}
-		$doSave = function (array $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, ?array $dependencies, ?array $usedTraitDependencies, ?array $valueDependencies, ?array $packageDependencies, array $exportedNodes, CachedExportedNodes $cachedExportedNodes, array $projectExtensionFiles, array $collectedDataRuleResults) use ($internalErrors, $resultCache, $output, $onlyFiles, $meta): bool {
+		$doSave = function (array $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, ?array $dependencies, ?array $usedTraitDependencies, ?array $valueDependencies, ?array $packageDependencies, array $exportedNodes, CachedExportedNodes $cachedExportedNodes, array $projectExtensionFiles, FinalizerResult $finalizerResult) use ($internalErrors, $resultCache, $output, $onlyFiles, $meta): bool {
 			if ($onlyFiles) {
 				if ($output->isVeryVerbose()) {
 					$output->writeLineFormatted('Result cache was not saved because only files were passed as analysed paths.');
@@ -1337,8 +1371,16 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache
 			}
 
 			// in the order save() writes them, which is the order they are restored in
+			$collectedDataRuleResults = $finalizerResult->getCollectedDataRuleResults();
 			ksort($collectedDataRuleResults);
 
+			$filesWithCollectedDataRuleErrors = [];
+			foreach ($finalizerResult->getCollectorErrors() as $collectorError) {
+				$filesWithCollectedDataRuleErrors[$collectorError->getFilePath()] = true;
+			}
+			ksort($filesWithCollectedDataRuleErrors);
+			$filesWithCollectedDataRuleErrors = array_keys($filesWithCollectedDataRuleErrors);
+
 			$stubFiles = $this->getStubFiles();
 			if (
 				!$resultCache->isFullAnalysis()
@@ -1350,6 +1392,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache
 				&& $unmatchedLineIgnores === $resultCache->getUnmatchedLineIgnores()
 				&& $collectedDataByFile === $resultCache->getCollectedData()
 				&& $collectedDataRuleResults === $resultCache->getCollectedDataRuleResults()
+				&& $filesWithCollectedDataRuleErrors === $resultCache->getFilesWithCollectedDataRuleErrors()
 				&& $packageDependencies === $resultCache->getPackageDependencies()
 				&& $exportedNodes === $resultCache->getExportedNodes()
 				&& $cachedExportedNodes->getFiles() === $resultCache->getCachedExportedNodes()->getFiles()
@@ -1368,7 +1411,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache
 				$currentFileHashes[$file] = self::REANALYSE_FILE_HASH_PREFIX . $currentFileHashes[$file];
 			}
 
-			$this->save($resultCache->getLastFullAnalysisTime(), $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $valueDependencies, $packageDependencies, $exportedNodes, $cachedExportedNodes, $projectExtensionFiles, $currentFileHashes, $meta, $stubFiles, $collectedDataRuleResults);
+			$this->save($resultCache->getLastFullAnalysisTime(), $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $valueDependencies, $packageDependencies, $exportedNodes, $cachedExportedNodes, $projectExtensionFiles, $currentFileHashes, $meta, $stubFiles, $collectedDataRuleResults, $filesWithCollectedDataRuleErrors);
 
 			if ($output->isVeryVerbose()) {
 				$output->writeLineFormatted('Result cache is saved.');
@@ -1378,7 +1421,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache
 		};
 
 		if ($resultCache->isFullAnalysis()) {
-			return new ResultCacheProcessResult($analyserResult, function (array $collectedDataRuleResults) use ($save, $output, $analyserResult, $projectConfigArray, $doSave, $freshErrorsByFile, $freshLocallyIgnoredErrorsByFile, $freshCollectedDataByFile): bool {
+			return new ResultCacheProcessResult($analyserResult, function (FinalizerResult $finalizerResult) use ($save, $output, $analyserResult, $projectConfigArray, $doSave, $freshErrorsByFile, $freshLocallyIgnoredErrorsByFile, $freshCollectedDataByFile): bool {
 				if ($save === false) {
 					if ($output->isVeryVerbose()) {
 						$output->writeLineFormatted('Result cache was not saved because it was not requested.');
@@ -1392,7 +1435,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache
 					$projectExtensionFiles = $this->getProjectExtensionFiles($projectConfigArray, $analyserResult->getDependencies());
 				}
 
-				return $doSave($freshErrorsByFile, $freshLocallyIgnoredErrorsByFile, $analyserResult->getLinesToIgnore(), $analyserResult->getUnmatchedLineIgnores(), $freshCollectedDataByFile, $analyserResult->getDependencies(), $analyserResult->getUsedTraitDependencies(), $analyserResult->getValueDependencies(), $analyserResult->getPackageDependencies(), $this->addNonAnalysedExportedNodes($analyserResult->getExportedNodes(), $analyserResult->getDependencies(), $analyserResult->getUsedTraitDependencies(), CachedExportedNodes::createEmpty()), CachedExportedNodes::createEmpty(), $projectExtensionFiles, $collectedDataRuleResults);
+				return $doSave($freshErrorsByFile, $freshLocallyIgnoredErrorsByFile, $analyserResult->getLinesToIgnore(), $analyserResult->getUnmatchedLineIgnores(), $freshCollectedDataByFile, $analyserResult->getDependencies(), $analyserResult->getUsedTraitDependencies(), $analyserResult->getValueDependencies(), $analyserResult->getPackageDependencies(), $this->addNonAnalysedExportedNodes($analyserResult->getExportedNodes(), $analyserResult->getDependencies(), $analyserResult->getUsedTraitDependencies(), CachedExportedNodes::createEmpty()), CachedExportedNodes::createEmpty(), $projectExtensionFiles, $finalizerResult);
 			});
 		}
 
@@ -1409,7 +1452,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache
 		$linesToIgnore = $this->mergeLinesToIgnore($resultCache, $analyserResult->getLinesToIgnore());
 		$unmatchedLineIgnores = $this->mergeUnmatchedLineIgnores($resultCache, $analyserResult->getUnmatchedLineIgnores());
 
-		$saveCallback = function (array $collectedDataRuleResults) use ($save, $resultCache, $projectConfigArray, $doSave, $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $valueDependencies, $packageDependencies, $exportedNodes, $cachedExportedNodes): bool {
+		$saveCallback = function (FinalizerResult $finalizerResult) use ($save, $resultCache, $projectConfigArray, $doSave, $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $valueDependencies, $packageDependencies, $exportedNodes, $cachedExportedNodes): bool {
 			if ($save === false) {
 				return false;
 			}
@@ -1434,7 +1477,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache
 					$projectExtensionFiles[$file] = [$hash, true, $className];
 				}
 			}
-			return $doSave($errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $valueDependencies, $packageDependencies, $exportedNodes, $cachedExportedNodes, $projectExtensionFiles, $collectedDataRuleResults);
+			return $doSave($errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $valueDependencies, $packageDependencies, $exportedNodes, $cachedExportedNodes, $projectExtensionFiles, $finalizerResult);
 		};
 
 		$flatErrors = [];
@@ -1894,6 +1937,7 @@ private function mergeUnmatchedLineIgnores(ResultCache $resultCache, array $fres
 	 * @param mixed[] $meta
 	 * @param array<string, string> $stubFiles
 	 * @param array<string, CollectedDataRuleResult> $collectedDataRuleResults
+	 * @param list<string> $filesWithCollectedDataRuleErrors
 	 */
 	private function save(
 		int $lastFullAnalysisTime,
@@ -1913,6 +1957,7 @@ private function save(
 		array $meta,
 		array $stubFiles,
 		array $collectedDataRuleResults,
+		array $filesWithCollectedDataRuleErrors,
 	): void
 	{
 		$invertedDependencies = [];
@@ -1999,6 +2044,7 @@ private function save(
 		$packageDependencies = $transformer->relativizeFileKeyed($packageDependencies);
 		$projectExtensionFiles = $transformer->relativizeFileKeyed($projectExtensionFiles);
 		$collectedDataRuleResults = $transformer->relativizeCollectedDataRuleResults($collectedDataRuleResults);
+		$filesWithCollectedDataRuleErrors = array_map(static fn (string $path): string => $transformer->relativizePath($path), $filesWithCollectedDataRuleErrors);
 
 		$file = $this->cacheFilePath;
 
@@ -2036,6 +2082,7 @@ private function save(
 			$this->writeArrayFrame($handle, $file, 'unmatchedLineIgnores', $unmatchedLineIgnores);
 			$this->writeArrayFrame($handle, $file, 'collectedData', $collectedData);
 			$this->writeValueFrame($handle, $file, 'collectedDataRuleResults', $collectedDataRuleResults);
+			$this->writeValueFrame($handle, $file, 'filesWithCollectedDataRuleErrors', $filesWithCollectedDataRuleErrors);
 			// An older PHPStan reading this file absolutizes the dependencies before it gets to the
 			// cacheVersion check that makes it discard the file, and fails on a missing section.
 			$this->writeArrayFrame($handle, $file, 'dependencies', []);

src/Analyser/ResultCache/ResultCacheProcessResult.php

@@ -4,13 +4,13 @@
 
 use Closure;
 use PHPStan\Analyser\AnalyserResult;
-use PHPStan\Analyser\CollectedDataRuleResult;
+use PHPStan\Analyser\FinalizerResult;
 
 final class ResultCacheProcessResult
 {
 
 	/**
-	 * @param Closure(array<string, CollectedDataRuleResult>): bool $saveCallback
+	 * @param Closure(FinalizerResult): bool $saveCallback
 	 */
 	public function __construct(private AnalyserResult $analyserResult, private Closure $saveCallback)
 	{
@@ -25,13 +25,11 @@ public function getAnalyserResult(): AnalyserResult
 	 * Writes the result cache, and says whether it was written.
 	 *
 	 * It is separate from ResultCacheManager::process() so that the caller can run the rules on
-	 * CollectedDataNode on the merged result first, and save their results too.
-	 *
-	 * @param array<string, CollectedDataRuleResult> $collectedDataRuleResults
+	 * CollectedDataNode on the merged result first, and save what they found too.
 	 */
-	public function save(array $collectedDataRuleResults): bool
+	public function save(FinalizerResult $finalizerResult): bool
 	{
-		return ($this->saveCallback)($collectedDataRuleResults);
+		return ($this->saveCallback)($finalizerResult);
 	}
 
 }

src/Command/AnalyseApplication.php

@@ -153,7 +153,7 @@ public function analyse(
 					count($finalizerResult->getCollectedDataRuleResults()),
 				));
 			}
-			$savedResultCache = $resultCacheResult->save($finalizerResult->getCollectedDataRuleResults());
+			$savedResultCache = $resultCacheResult->save($finalizerResult);
 			$analyserResult = $finalizerResult->getAnalyserResult();
 			$internalErrors = $analyserResult->getInternalErrors();
 			$errors = array_merge(

src/Command/FixerWorkerRunner.php

@@ -198,7 +198,7 @@ function (array $errors, array $locallyIgnoredErrors, array $analysedFiles) use
 					true,
 				);
 				$finalizerResult = $this->analyserResultFinalizer->finalize($resultCacheResult->getAnalyserResult(), $isOnlyFiles, false, null, null, $resultCache->getCollectedDataRuleResults());
-				$resultCacheResult->save($finalizerResult->getCollectedDataRuleResults());
+				$resultCacheResult->save($finalizerResult);
 
 				$internalErrors = [];
 				foreach ($finalizerResult->getAnalyserResult()->getInternalErrors() as $internalError) {

tests/PHPStan/File/SimpleRelativePathHelperTest.php

@@ -35,7 +35,7 @@ public static function dataGetRelativePath(): array
 				'/project/src',
 				'/project/src/HelloWorld.php',
 				'HelloWorld.php',
-			]
+			],
 		];
 	}