From e0758ca3aaff080dedec5f7dace919d452a0b267 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 4 Nov 2026 13:59:18 +0100 Subject: [PATCH] PHPStan Pro: analyse again the files changed while an analysis read them A file changed and changed back between two polls of the file monitor looks unchanged to it. When an analysis read the file in between, nothing started another one, and the errors of the code that existed only during the analysis stayed until the next edit. The hashing monitor polls once a second, so this took an undo within a second. Windows always uses it, and so does everybody without FFI. The result cache already knows these files when it saves and records them to be analysed again. The worker now reports them too, and the file monitoring treats that like a change it saw itself, so pausing and waiting for a burst of changes to end apply to it. Something writing to the analysed files during every analysis, like a generator in a bootstrap file, would keep the analyses going, so an analysis started only for this does not start another one. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SLV3fAj54R19CE8XoFyu9Z --- .../ResultCache/ResultCacheManager.php | 22 ++++++++++---- src/Command/FixerApplication.php | 30 +++++++++++++++++++ src/Command/FixerWorkerRunner.php | 5 ++++ .../ResultCacheManagerCollectedDataTest.php | 20 +++++++++++++ 4 files changed, 72 insertions(+), 5 deletions(-) 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.