Skip to content

Commit b31a1e5

Browse files
SanderMullerclaude
andauthored
Fix --only runs caching files as unchanged, hiding pending changes from full runs (#8029)
* fix: do not cache files as unchanged on --only and --only-suffix runs A file clean under one rule is not necessarily clean under all rules. An --only run cached such files as unchanged, so the next full run silently skipped their pending changes until --clear-cache. Covered by a new e2e scenario that fails on main: an --only run followed by a full run must still report the change pending under the other rule. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: replace e2e scenario with a unit test ApplicationFileProcessor::processFiles() with no rules registered reaches the cache-write branch directly, so the guard is testable without an e2e project: an --only / --only-suffix run must leave the file uncached, a plain dry-run caches it. Fails on main, passes with the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: PSR-4 compliant test fixture name and namespace Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent bb186b6 commit b31a1e5

3 files changed

Lines changed: 83 additions & 1 deletion

File tree

‎src/Application/ApplicationFileProcessor.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,11 @@ private function processFile(File $file, Configuration $configuration): FileProc
173173
if ($fileProcessResult->getSystemErrors() !== []) {
174174
$this->changedFilesDetector->invalidateFile($file->getFilePath());
175175
} elseif (! $configuration->isDryRun() || ! $fileProcessResult->getFileDiff() instanceof FileDiff) {
176-
$this->changedFilesDetector->cacheFile($file->getFilePath());
176+
// a file clean under a subset of rules is not necessarily clean under all rules,
177+
// caching it would hide its pending changes from the next full run
178+
if ($configuration->getOnlyRule() === null && $configuration->getOnlySuffix() === null) {
179+
$this->changedFilesDetector->cacheFile($file->getFilePath());
180+
}
177181
}
178182

179183
return $fileProcessResult;
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\Application\ApplicationFileProcessor;
6+
7+
use Rector\Application\ApplicationFileProcessor;
8+
use Rector\Caching\Detector\ChangedFilesDetector;
9+
use Rector\DeadCode\Rector\ClassMethod\RemoveEmptyClassMethodRector;
10+
use Rector\Testing\PHPUnit\AbstractLazyTestCase;
11+
use Rector\ValueObject\Configuration;
12+
13+
final class ApplicationFileProcessorTest extends AbstractLazyTestCase
14+
{
15+
private ApplicationFileProcessor $applicationFileProcessor;
16+
17+
private ChangedFilesDetector $changedFilesDetector;
18+
19+
protected function setUp(): void
20+
{
21+
parent::setUp();
22+
23+
$this->applicationFileProcessor = $this->make(ApplicationFileProcessor::class);
24+
$this->changedFilesDetector = $this->make(ChangedFilesDetector::class);
25+
}
26+
27+
protected function tearDown(): void
28+
{
29+
$this->changedFilesDetector->clear();
30+
}
31+
32+
public function testCleanFileIsCachedAsUnchanged(): void
33+
{
34+
$filePath = __DIR__ . '/Source/CleanFile.php';
35+
36+
$this->applicationFileProcessor->processFiles([$filePath], new Configuration(isDryRun: true));
37+
38+
$this->assertFalse($this->changedFilesDetector->hasFileChanged($filePath));
39+
}
40+
41+
public function testOnlyRuleRunDoesNotCacheFileAsUnchanged(): void
42+
{
43+
$filePath = __DIR__ . '/Source/CleanFile.php';
44+
45+
$this->applicationFileProcessor->processFiles([$filePath], new Configuration(
46+
isDryRun: true,
47+
onlyRule: RemoveEmptyClassMethodRector::class
48+
));
49+
50+
// a file clean under one rule is not necessarily clean under all rules
51+
$this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath));
52+
}
53+
54+
public function testOnlySuffixRunDoesNotCacheFileAsUnchanged(): void
55+
{
56+
$filePath = __DIR__ . '/Source/CleanFile.php';
57+
58+
$this->applicationFileProcessor->processFiles([$filePath], new Configuration(
59+
isDryRun: true,
60+
onlySuffix: 'Controller.php'
61+
));
62+
63+
$this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath));
64+
}
65+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\Application\ApplicationFileProcessor\Source;
6+
7+
final class CleanFile
8+
{
9+
public function run(): string
10+
{
11+
return 'nothing to change';
12+
}
13+
}

0 commit comments

Comments
 (0)