diff --git a/src/Analyser/ResultCache/ResultCacheManager.php b/src/Analyser/ResultCache/ResultCacheManager.php index 1e8215dd56a..7281cd86a80 100644 --- a/src/Analyser/ResultCache/ResultCacheManager.php +++ b/src/Analyser/ResultCache/ResultCacheManager.php @@ -1398,11 +1398,8 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache // The files changed during the analysis may have been read in another version than the one // their hash and exported nodes describe, and so may the files using them, which are analysed - // again too. A run that analysed nothing read nothing. - $filesChangedDuringAnalysis = []; - if ($resultCache->isFullAnalysis() || $resultCache->getFilesToAnalyse() !== []) { - $filesChangedDuringAnalysis = $this->getFilesChangedSinceHashed($currentFileHashes); - } + // again too. + $filesChangedDuringAnalysis = $this->getFilesChangedDuringAnalysis($resultCache); foreach (array_keys($filesChangedDuringAnalysis) as $file) { if ($output->isVeryVerbose()) { $output->writeLineFormatted(sprintf('Result cache will re-analyse %s and the files using it because it changed during the analysis.', $file)); @@ -3258,6 +3255,21 @@ private function fileStatSignaturesDiffer(array $cachedDependencies): bool return false; } + /** + * The analysed files written to since restore() hashed them, for a result that may not describe + * what they contain now. A run that analysed nothing read nothing. + * + * @return array + */ + public function getFilesChangedDuringAnalysis(ResultCache $resultCache): array + { + if (!$resultCache->isFullAnalysis() && $resultCache->getFilesToAnalyse() === []) { + return []; + } + + return $this->getFilesChangedSinceHashed($resultCache->getCurrentFileHashes()); + } + /** * The analysed files written to after they were hashed. The analysis may have read them before * or after the write, or the files depending on them may have read the other version, so neither diff --git a/src/Command/FixerApplication.php b/src/Command/FixerApplication.php index 001a3d95a53..b45f98ed134 100644 --- a/src/Command/FixerApplication.php +++ b/src/Command/FixerApplication.php @@ -75,6 +75,19 @@ final class FixerApplication /** Files changed during the analysis in progress, another one starts once it ends. */ private bool $analysisRequested = false; + /** + * The last analysis may have read files in another version than they have now. The file monitor + * cannot tell when a file is back to the contents it last saw, like an edit undone between two polls. + */ + private bool $filesChangedWhileAnalysed = false; + + /** + * Nothing changed since the last analysis but the files it reported. Whatever writes to the + * analysed files during every analysis, like a generator in a bootstrap file, would otherwise + * keep the analyses going, so an analysis that was started for this cannot start another one. + */ + private bool $analysingOnlyFilesChangedWhileAnalysed = false; + private bool $fileMonitorActive = true; /** @var string[] watched files besides the analysed ones and the config files */ @@ -431,7 +444,16 @@ private function monitorFileChanges(LoopInterface $loop, callable $hasChangesCal $changes = $fileMonitor->getChanges(); $now = microtime(true); + // not during an analysis, the one starting after it might already be for something else + if ($this->filesChangedWhileAnalysed && $this->processInProgress === null) { + $this->filesChangedWhileAnalysed = false; + $this->analysingOnlyFilesChangedWhileAnalysed = true; + $firstChangeAt ??= $now; + $lastChangeAt = $now; + } + if ($changes->hasAnyChanges()) { + $this->analysingOnlyFilesChangedWhileAnalysed = false; $firstChangeAt ??= $now; $lastChangeAt = $now; $hasChangesCallback($changes); @@ -464,6 +486,9 @@ private function analyse( Encoder $phpstanFixerEncoder, ): void { + // this analysis reads the files as they are now + $this->filesChangedWhileAnalysed = false; + // TCP server for fixer:worker (TCP client) $server = new TcpServer('127.0.0.1:0', $loop); /** @var string $serverAddress */ @@ -486,6 +511,11 @@ private function analyse( return; } + if ($data['action'] === 'filesChangedWhileAnalysed') { + $this->filesChangedWhileAnalysed = !$this->analysingOnlyFilesChangedWhileAnalysed; + return; + } + $phpstanFixerEncoder->write($data); }); }); diff --git a/src/Command/FixerWorkerRunner.php b/src/Command/FixerWorkerRunner.php index 7ae27b061f1..ff6af16786a 100644 --- a/src/Command/FixerWorkerRunner.php +++ b/src/Command/FixerWorkerRunner.php @@ -232,6 +232,11 @@ function (array $errors, array $locallyIgnoredErrors, array $analysedFiles) use $finalizerResult = $this->analyserResultFinalizer->finalize($resultCacheResult->getAnalyserResult(), $isOnlyFiles, false, null, null, $resultCache->getCollectedDataRuleResults()); $resultCacheResult->save($finalizerResult); + // FixerApplication's file monitor misses a file changed and changed back between two polls + if ($resultCacheManager->getFilesChangedDuringAnalysis($resultCache) !== []) { + $out->write(['action' => 'filesChangedWhileAnalysed', 'result' => []]); + } + $internalErrors = []; foreach ($finalizerResult->getAnalyserResult()->getInternalErrors() as $internalError) { $internalErrors[] = new InternalError( diff --git a/tests/PHPStan/Analyser/ResultCache/ResultCacheManagerCollectedDataTest.php b/tests/PHPStan/Analyser/ResultCache/ResultCacheManagerCollectedDataTest.php index 30bf9746beb..078221f5aef 100644 --- a/tests/PHPStan/Analyser/ResultCache/ResultCacheManagerCollectedDataTest.php +++ b/tests/PHPStan/Analyser/ResultCache/ResultCacheManagerCollectedDataTest.php @@ -234,11 +234,31 @@ public function testFileChangedAndChangedBackDuringTheAnalysisIsAnalysedAgain(): file_put_contents($a, $hashed); $processed = $manager->process($this->analyserResult([], [], [$a]), $resultCache, $this->nullOutput(), false, true); $this->assertTrue($processed->save(new FinalizerResult($processed->getAnalyserResult(), [], []))); + $this->assertSame([$a => true], $manager->getFilesChangedDuringAnalysis($resultCache)); $resultCache = $this->createManager()->restore($this->files, false, false, null, $this->nullOutput()); $this->assertSame([$a], $resultCache->getFilesToAnalyse()); } + public function testNoFilesChangedDuringTheAnalysis(): void + { + $a = $this->files[0]; + $this->saveFullAnalysis([]); + $this->waitUntilTheFilesAreInThePast(); + + $manager = $this->createManager(); + $resultCache = $manager->restore($this->files, false, false, null, $this->nullOutput()); + $this->assertSame([], $resultCache->getFilesToAnalyse()); + // a run that analysed nothing read nothing + file_put_contents($a, "assertSame([], $manager->getFilesChangedDuringAnalysis($resultCache)); + + $manager = $this->createManager(); + $resultCache = $manager->restore($this->files, false, false, null, $this->nullOutput()); + $this->assertSame([$a], $resultCache->getFilesToAnalyse()); + $this->assertSame([], $manager->getFilesChangedDuringAnalysis($resultCache)); + } + /** * A save keeps the stat signature only of a file not modified in the second the save began, * so without waiting, saves on both sides of a second boundary differ in their signatures.