Skip to content

Commit 0592a5f

Browse files
committed
Cache --only / --only-suffix runs under a rule-scoped key
#8029 stopped caching selective runs to fix full-run poisoning (a file clean under one rule was cached as clean for all). That also removed the cache on repeated --only / --only-suffix runs — a drawback raised in that thread, and what #7641 set out to solve. Scope the per-file cache key by the active rule selection instead. Selective and full runs then use distinct, coexisting cache entries: a repeated --only run is served from cache, a full run is never poisoned by it, and nothing is cleared, so there is no back-and-forth thrash (the behaviour that closed #7641). The scope is set in both run() and processFiles() because parallel workers invoke processFiles() directly via WorkerCommand, bypassing run(), and must write entries under the same scope the main process reads.
1 parent 2328ea6 commit 0592a5f

3 files changed

Lines changed: 32 additions & 9 deletions

File tree

src/Application/ApplicationFileProcessor.php

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ public function __construct(
5656

5757
public function run(Configuration $configuration, InputInterface $input): ProcessResult
5858
{
59+
// scope the cache to this run's --only / --only-suffix selection before any cache read/write
60+
$this->changedFilesDetector->setActiveScope($configuration->getOnlyRule(), $configuration->getOnlySuffix());
61+
5962
$filePaths = $this->filesFinder->findFilesInPaths($configuration->getPaths(), $configuration);
6063

6164
// no files found
@@ -121,6 +124,9 @@ public function processFiles(
121124
?callable $preFileCallback = null,
122125
?callable $postFileCallback = null
123126
): ProcessResult {
127+
// also set here: parallel workers reach processFiles() via WorkerCommand, bypassing run()
128+
$this->changedFilesDetector->setActiveScope($configuration->getOnlyRule(), $configuration->getOnlySuffix());
129+
124130
/** @var SystemError[] $systemErrors */
125131
$systemErrors = [];
126132

@@ -179,11 +185,8 @@ private function processFile(File $file, Configuration $configuration): FileProc
179185
if ($fileProcessResult->getSystemErrors() !== []) {
180186
$this->changedFilesDetector->invalidateFile($file->getFilePath());
181187
} elseif (! $configuration->isDryRun() || ! $fileProcessResult->getFileDiff() instanceof FileDiff) {
182-
// a file clean under a subset of rules is not necessarily clean under all rules,
183-
// caching it would hide its pending changes from the next full run
184-
if ($configuration->getOnlyRule() === null && $configuration->getOnlySuffix() === null) {
185-
$this->changedFilesDetector->cacheFile($file->getFilePath());
186-
}
188+
// selective runs are safe to cache now — the key is scoped to the rule selection
189+
$this->changedFilesDetector->cacheFile($file->getFilePath());
187190
}
188191

189192
return $fileProcessResult;

src/Caching/Detector/ChangedFilesDetector.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,24 @@ final class ChangedFilesDetector
2121
*/
2222
private array $cacheableFiles = [];
2323

24+
// scopes the per-file cache key to the active --only / --only-suffix selection (empty = full run)
25+
private string $scopeSuffix = '';
26+
2427
public function __construct(
2528
private readonly FileHashComputer $fileHashComputer,
2629
private readonly Cache $cache,
2730
private readonly FileHasher $fileHasher
2831
) {
2932
}
3033

34+
public function setActiveScope(?string $onlyRule, ?string $onlySuffix): void
35+
{
36+
// each selection gets its own cache key, so --only and full runs coexist without clearing or poisoning
37+
$this->scopeSuffix = ($onlyRule === null && $onlySuffix === null)
38+
? ''
39+
: '|only:' . ($onlyRule ?? '') . '|suffix:' . ($onlySuffix ?? '');
40+
}
41+
3142
public function cacheFile(string $filePath): void
3243
{
3344
$filePathCacheKey = $this->getFilePathCacheKey($filePath);
@@ -95,7 +106,7 @@ private function resolvePath(string $filePath): string
95106

96107
private function getFilePathCacheKey(string $filePath): string
97108
{
98-
return $this->fileHasher->hash($this->resolvePath($filePath));
109+
return $this->fileHasher->hash($this->resolvePath($filePath) . $this->scopeSuffix);
99110
}
100111

101112
private function hashFile(string $filePath): string

tests/Application/ApplicationFileProcessor/ApplicationFileProcessorTest.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function testCleanFileIsCachedAsUnchanged(): void
3838
$this->assertFalse($this->changedFilesDetector->hasFileChanged($filePath));
3939
}
4040

41-
public function testOnlyRuleRunDoesNotCacheFileAsUnchanged(): void
41+
public function testOnlyRuleRunCachesUnderOwnScopeWithoutPoisoningFullRun(): void
4242
{
4343
$filePath = __DIR__ . '/Source/CleanFile.php';
4444

@@ -47,11 +47,16 @@ public function testOnlyRuleRunDoesNotCacheFileAsUnchanged(): void
4747
onlyRule: RemoveEmptyClassMethodRector::class
4848
));
4949

50-
// a file clean under one rule is not necessarily clean under all rules
50+
// a repeated --only run hits its own scoped cache entry
51+
$this->changedFilesDetector->setActiveScope(RemoveEmptyClassMethodRector::class, null);
52+
$this->assertFalse($this->changedFilesDetector->hasFileChanged($filePath));
53+
54+
// a full run uses a different scope key, so it is not poisoned
55+
$this->changedFilesDetector->setActiveScope(null, null);
5156
$this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath));
5257
}
5358

54-
public function testOnlySuffixRunDoesNotCacheFileAsUnchanged(): void
59+
public function testOnlySuffixRunCachesUnderOwnScopeWithoutPoisoningFullRun(): void
5560
{
5661
$filePath = __DIR__ . '/Source/CleanFile.php';
5762

@@ -60,6 +65,10 @@ public function testOnlySuffixRunDoesNotCacheFileAsUnchanged(): void
6065
onlySuffix: 'Controller.php'
6166
));
6267

68+
$this->changedFilesDetector->setActiveScope(null, 'Controller.php');
69+
$this->assertFalse($this->changedFilesDetector->hasFileChanged($filePath));
70+
71+
$this->changedFilesDetector->setActiveScope(null, null);
6372
$this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath));
6473
}
6574
}

0 commit comments

Comments
 (0)