Skip to content

Commit ab82a87

Browse files
SanderMullerclaude
andcommitted
fix: replay the original hasChanged flag from the dry-run diff cache
A rule can report line changes while printing identical content. Such a file produces a FileDiff with hasChanged = false, which the diff cache stored and replayed as changed, so warm runs reported phantom changed files. Store the flag in the cache entry and replay it via FileProcessResult. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 8da4a6a commit ab82a87

3 files changed

Lines changed: 43 additions & 13 deletions

File tree

‎src/Application/ApplicationFileProcessor.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,9 @@ private function processFile(File $file, Configuration $configuration): FileProc
176176
$useDiffCache = $configuration->isDryRun() && ! $isSelectiveRun;
177177

178178
if ($useDiffCache) {
179-
$cachedDiff = $this->dryRunDiffCache->load($file, $configuration);
180-
if ($cachedDiff instanceof FileDiff) {
181-
return new FileProcessResult([], $cachedDiff, true);
179+
$cachedFileProcessResult = $this->dryRunDiffCache->load($file, $configuration);
180+
if ($cachedFileProcessResult instanceof FileProcessResult) {
181+
return $cachedFileProcessResult;
182182
}
183183
}
184184

@@ -193,7 +193,7 @@ private function processFile(File $file, Configuration $configuration): FileProc
193193
$this->changedFilesDetector->cacheFile($file->getFilePath());
194194
}
195195
} elseif ($useDiffCache) {
196-
$this->dryRunDiffCache->save($file, $configuration, $fileDiff);
196+
$this->dryRunDiffCache->save($file, $configuration, $fileDiff, $fileProcessResult->hasChanged());
197197
}
198198

199199
return $fileProcessResult;

‎src/Caching/DryRunDiffCache.php‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Rector\Util\FileHasher;
1212
use Rector\ValueObject\Application\File;
1313
use Rector\ValueObject\Configuration;
14+
use Rector\ValueObject\FileProcessResult;
1415
use Rector\ValueObject\Reporting\FileDiff;
1516

1617
/**
@@ -30,7 +31,7 @@ public function __construct(
3031
) {
3132
}
3233

33-
public function load(File $file, Configuration $configuration): ?FileDiff
34+
public function load(File $file, Configuration $configuration): ?FileProcessResult
3435
{
3536
$cached = $this->cache->load($this->key($file), CacheKey::FILE_DIFF_KEY);
3637
if (! is_array($cached)) {
@@ -57,10 +58,18 @@ public function load(File $file, Configuration $configuration): ?FileDiff
5758
return null;
5859
}
5960

60-
return FileDiff::decode($diffJson);
61+
// a rule can report line changes while printing identical content, so a diff
62+
// does not imply a changed file → replay the original flag or warm runs
63+
// would report phantom changed files
64+
$hasChanged = $cached['changed'] ?? null;
65+
if (! is_bool($hasChanged)) {
66+
return null;
67+
}
68+
69+
return new FileProcessResult([], FileDiff::decode($diffJson), $hasChanged);
6170
}
6271

63-
public function save(File $file, Configuration $configuration, FileDiff $fileDiff): void
72+
public function save(File $file, Configuration $configuration, FileDiff $fileDiff, bool $hasChanged): void
6473
{
6574
// a failed capture means a possibly incomplete set, skip caching so the file is reprocessed
6675
$dependencyHashes = $this->fileDependencyCollector->getDependencyFileHashes($file->getFilePath());
@@ -79,6 +88,7 @@ public function save(File $file, Configuration $configuration, FileDiff $fileDif
7988
'hash' => $this->contentHash($file, $configuration),
8089
'deps' => $dependencyHashes,
8190
'diff' => $diffJson,
91+
'changed' => $hasChanged,
8292
]);
8393
}
8494

‎tests/Caching/DryRunDiffCacheTest.php‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Rector\Util\FileHasher;
1616
use Rector\ValueObject\Application\File;
1717
use Rector\ValueObject\Configuration;
18+
use Rector\ValueObject\FileProcessResult;
1819
use Rector\ValueObject\Reporting\FileDiff;
1920
use Symfony\Component\Filesystem\Filesystem;
2021

@@ -68,19 +69,37 @@ public function testSaveLoadRoundTrip(): void
6869
$this->assertNull($this->dryRunDiffCache->load($file, $configuration));
6970

7071
$this->fileDependencyCollector->record($this->sourceFilePath, $this->dependencyFilePath);
71-
$this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff());
72+
$this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff(), true);
7273

73-
$loadedFileDiff = $this->dryRunDiffCache->load($file, $configuration);
74+
$cachedFileProcessResult = $this->dryRunDiffCache->load($file, $configuration);
75+
$this->assertInstanceOf(FileProcessResult::class, $cachedFileProcessResult);
76+
$this->assertTrue($cachedFileProcessResult->hasChanged());
77+
78+
$loadedFileDiff = $cachedFileProcessResult->getFileDiff();
7479
$this->assertInstanceOf(FileDiff::class, $loadedFileDiff);
7580
$this->assertSame('some diff', $loadedFileDiff->getDiff());
7681
$this->assertSame([RemoveUnusedPrivateMethodRector::class], $loadedFileDiff->getRectorClasses());
7782
}
7883

84+
public function testReplayKeepsUnchangedFlag(): void
85+
{
86+
$file = $this->createFile();
87+
$configuration = new Configuration(isDryRun: true);
88+
89+
// a rule can report line changes while the printed content stays identical
90+
$this->fileDependencyCollector->record($this->sourceFilePath, $this->dependencyFilePath);
91+
$this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff(), false);
92+
93+
$cachedFileProcessResult = $this->dryRunDiffCache->load($file, $configuration);
94+
$this->assertInstanceOf(FileProcessResult::class, $cachedFileProcessResult);
95+
$this->assertFalse($cachedFileProcessResult->hasChanged());
96+
}
97+
7998
public function testChangedOwnContentInvalidates(): void
8099
{
81100
$file = $this->createFile();
82101
$configuration = new Configuration(isDryRun: true);
83-
$this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff());
102+
$this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff(), true);
84103

85104
$changedFile = new File($this->sourceFilePath, "<?php\n\nclass Source\n{\n public int \$added;\n}\n");
86105
$this->assertNull($this->dryRunDiffCache->load($changedFile, $configuration));
@@ -92,8 +111,8 @@ public function testChangedDependencyInvalidates(): void
92111
$configuration = new Configuration(isDryRun: true);
93112

94113
$this->fileDependencyCollector->record($this->sourceFilePath, $this->dependencyFilePath);
95-
$this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff());
96-
$this->assertInstanceOf(FileDiff::class, $this->dryRunDiffCache->load($file, $configuration));
114+
$this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff(), true);
115+
$this->assertInstanceOf(FileProcessResult::class, $this->dryRunDiffCache->load($file, $configuration));
97116

98117
(new Filesystem())->dumpFile(
99118
$this->dependencyFilePath,
@@ -121,7 +140,8 @@ public function testNoDiffsConfigurationDoesNotCrossReplay(): void
121140
$this->dryRunDiffCache->save(
122141
$file,
123142
new Configuration(isDryRun: true, showDiffs: false),
124-
$this->createFileDiff()
143+
$this->createFileDiff(),
144+
true
125145
);
126146

127147
$this->assertNull($this->dryRunDiffCache->load($file, new Configuration(isDryRun: true, showDiffs: true)));

0 commit comments

Comments
 (0)