From 5403eed09e61bbc773af56bd264529bdea83391b Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Thu, 11 Jun 2026 10:25:31 +0200 Subject: [PATCH 1/3] fix: make the unchanged-files cache dependency-aware The cache only checked each file's own content, so a clean file stayed skipped on warm runs even when one of its dependencies changed, e.g. a parent class method gaining a return type that lets a child file infer its own. A fresh run reports the new change, a warm run misses it. PHPStanNodeScopeResolver now records each file's dependencies during scope resolution using PHPStan's own DependencyResolver, the same engine behind PHPStan's result cache. Cache entries store the file's own hash plus one hash per dependency, all re-validated on load; legacy string entries self-upgrade on the next write. A failed capture skips caching entirely rather than caching a partial set. Function calls memoize their dependency files per resolved name, as signature dependencies are identical at every call site. Selective runs (--only, --only-suffix) bypass the cache write, same guard as #8029. Co-Authored-By: Claude Opus 4.8 --- src/Application/ApplicationFileProcessor.php | 6 +- src/Caching/Detector/ChangedFilesDetector.php | 54 ++++-- src/Caching/FileDependencyCollector.php | 154 ++++++++++++++++++ .../LazyContainerFactory.php | 5 + .../Scope/PHPStanNodeScopeResolver.php | 60 ++++++- .../Detector/ChangedFilesDetectorTest.php | 43 +++++ 6 files changed, 307 insertions(+), 15 deletions(-) create mode 100644 src/Caching/FileDependencyCollector.php diff --git a/src/Application/ApplicationFileProcessor.php b/src/Application/ApplicationFileProcessor.php index 932593d032f..98f75c0d14e 100644 --- a/src/Application/ApplicationFileProcessor.php +++ b/src/Application/ApplicationFileProcessor.php @@ -173,7 +173,11 @@ private function processFile(File $file, Configuration $configuration): FileProc if ($fileProcessResult->getSystemErrors() !== []) { $this->changedFilesDetector->invalidateFile($file->getFilePath()); } elseif (! $configuration->isDryRun() || ! $fileProcessResult->getFileDiff() instanceof FileDiff) { - $this->changedFilesDetector->cacheFile($file->getFilePath()); + // a file clean under a subset of rules is not necessarily clean under all rules, + // caching it would hide its pending changes from the next full run + if ($configuration->getOnlyRule() === null && $configuration->getOnlySuffix() === null) { + $this->changedFilesDetector->cacheFile($file->getFilePath()); + } } return $fileProcessResult; diff --git a/src/Caching/Detector/ChangedFilesDetector.php b/src/Caching/Detector/ChangedFilesDetector.php index 46718bab661..b5d7b7225f1 100644 --- a/src/Caching/Detector/ChangedFilesDetector.php +++ b/src/Caching/Detector/ChangedFilesDetector.php @@ -7,6 +7,7 @@ use Rector\Caching\Cache; use Rector\Caching\Config\FileHashComputer; use Rector\Caching\Enum\CacheKey; +use Rector\Caching\FileDependencyCollector; use Rector\Util\FileHasher; /** @@ -24,7 +25,8 @@ final class ChangedFilesDetector public function __construct( private readonly FileHashComputer $fileHashComputer, private readonly Cache $cache, - private readonly FileHasher $fileHasher + private readonly FileHasher $fileHasher, + private readonly FileDependencyCollector $fileDependencyCollector ) { } @@ -36,9 +38,26 @@ public function cacheFile(string $filePath): void return; } - $hash = $this->hashFile($filePath); + // a failed capture means a possibly incomplete set, skip caching so the file is reprocessed + $dependencyHashes = $this->fileDependencyCollector->getDependencyFileHashes($filePath); + if ($dependencyHashes === null) { + return; + } - $this->cache->save($filePathCacheKey, CacheKey::FILE_HASH_KEY, $hash); + // the file may have just been written, recompute its hash fresh + // rather than trusting a memo entry from the pre-write filter pass + $this->fileDependencyCollector->forgetContentHash($filePath); + $ownHash = $this->fileDependencyCollector->contentHash($filePath); + if ($ownHash === null) { + return; + } + + // store the own content hash plus one per dependency, so a dependency change + // invalidates this file even when its own content is unchanged + $this->cache->save($filePathCacheKey, CacheKey::FILE_HASH_KEY, [ + 'hash' => $ownHash, + 'deps' => $dependencyHashes, + ]); } public function addCacheableFile(string $filePath): void @@ -52,13 +71,27 @@ public function hasFileChanged(string $filePath): bool $fileInfoCacheKey = $this->getFilePathCacheKey($filePath); $cachedValue = $this->cache->load($fileInfoCacheKey, CacheKey::FILE_HASH_KEY); - if ($cachedValue !== null) { - $currentFileHash = $this->hashFile($filePath); - return $currentFileHash !== $cachedValue; + // no value to compare against → be defensive and assume changed + if ($cachedValue === null) { + return true; + } + + // legacy string entry → own-hash comparison only, rewritten in the new format on next cacheFile() + if (is_string($cachedValue)) { + return $this->fileDependencyCollector->contentHash($filePath) !== $cachedValue; } - // we don't have a value to compare against. Be defensive and assume its changed - return true; + if (! is_array($cachedValue)) { + return true; + } + + // own content changed + if (($cachedValue['hash'] ?? null) !== $this->fileDependencyCollector->contentHash($filePath)) { + return true; + } + + // any recorded dependency changed + return $this->fileDependencyCollector->hasAnyChangedDependency($cachedValue['deps'] ?? []); } public function invalidateFile(string $filePath): void @@ -98,11 +131,6 @@ private function getFilePathCacheKey(string $filePath): string return $this->fileHasher->hash($this->resolvePath($filePath)); } - private function hashFile(string $filePath): string - { - return $this->fileHasher->hashFiles([$this->resolvePath($filePath)]); - } - private function storeConfigurationDataHash(string $filePath, string $configurationHash): void { $key = CacheKey::CONFIGURATION_HASH_KEY . '_' . $this->getFilePathCacheKey($filePath); diff --git a/src/Caching/FileDependencyCollector.php b/src/Caching/FileDependencyCollector.php new file mode 100644 index 00000000000..79dde5a2071 --- /dev/null +++ b/src/Caching/FileDependencyCollector.php @@ -0,0 +1,154 @@ +> + */ + private array $dependenciesByFile = []; + + /** + * files whose capture threw, their possibly partial set must never be cached + * + * @var array + */ + private array $failedFiles = []; + + /** + * keyed by the given path, so memo hits skip realpath() as well + * + * @var array + */ + private array $contentHashMemo = []; + + /** + * a function's signature dependencies are identical at every call site + * + * @var array + */ + private array $functionDependencyFilesMemo = []; + + public function __construct( + private readonly FileHasher $fileHasher, + ) { + } + + public function record(string $filePath, string $dependencyFilePath): void + { + if ($filePath === $dependencyFilePath) { + return; + } + + $this->dependenciesByFile[$filePath][$dependencyFilePath] = true; + } + + public function markFailed(string $filePath): void + { + $this->failedFiles[$filePath] = true; + } + + /** + * @return string[]|null + */ + public function getMemoizedFunctionDependencyFiles(string $functionKey): ?array + { + return $this->functionDependencyFilesMemo[$functionKey] ?? null; + } + + /** + * @param string[] $dependencyFiles + */ + public function memoizeFunctionDependencyFiles(string $functionKey, array $dependencyFiles): void + { + $this->functionDependencyFilesMemo[$functionKey] = $dependencyFiles; + } + + /** + * @return array|null null when capture failed and the set cannot be trusted + */ + public function getDependencyFileHashes(string $filePath): ?array + { + if (isset($this->failedFiles[$filePath])) { + return null; + } + + $dependencyHashes = []; + foreach (array_keys($this->dependenciesByFile[$filePath] ?? []) as $dependencyFile) { + $dependencyHash = $this->contentHash($dependencyFile); + if ($dependencyHash !== null) { + $dependencyHashes[$dependencyFile] = $dependencyHash; + } + } + + return $dependencyHashes; + } + + /** + * @param array $recordedDependencyHashes + */ + public function hasAnyChangedDependency(array $recordedDependencyHashes): bool + { + foreach ($recordedDependencyHashes as $dependencyFile => $recordedHash) { + if ($this->contentHash($dependencyFile) !== $recordedHash) { + return true; + } + } + + return false; + } + + /** + * null when the file does not exist, e.g. a deleted dependency, which callers treat as changed + */ + public function contentHash(string $filePath): ?string + { + if (array_key_exists($filePath, $this->contentHashMemo)) { + return $this->contentHashMemo[$filePath]; + } + + $resolvedPath = $this->resolvePath($filePath); + if (! is_file($resolvedPath)) { + return $this->contentHashMemo[$filePath] = null; + } + + return $this->contentHashMemo[$filePath] = $this->fileHasher->hashFiles([$resolvedPath]); + } + + /** + * drop a memoized hash, e.g. after the file has been written mid-run + */ + public function forgetContentHash(string $filePath): void + { + unset($this->contentHashMemo[$filePath]); + } + + public function reset(): void + { + $this->dependenciesByFile = []; + $this->failedFiles = []; + $this->contentHashMemo = []; + $this->functionDependencyFilesMemo = []; + } + + private function resolvePath(string $filePath): string + { + $realPath = realpath($filePath); + if ($realPath === false) { + return $filePath; + } + + return $realPath; + } +} diff --git a/src/DependencyInjection/LazyContainerFactory.php b/src/DependencyInjection/LazyContainerFactory.php index 04d7da27c22..b74f6de1971 100644 --- a/src/DependencyInjection/LazyContainerFactory.php +++ b/src/DependencyInjection/LazyContainerFactory.php @@ -10,6 +10,7 @@ use PhpParser\Lexer; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\ScopeFactory; +use PHPStan\Dependency\DependencyResolver; use PHPStan\Parser\Parser; use PHPStan\PhpDoc\TypeNodeResolver; use PHPStan\PhpDocParser\ParserConfig; @@ -38,6 +39,7 @@ use Rector\Caching\CacheFactory; use Rector\Caching\Config\FileHashComputer; use Rector\Caching\Contract\CacheMetaExtensionInterface; +use Rector\Caching\FileDependencyCollector; use Rector\ChangesReporting\Contract\Output\OutputFormatterInterface; use Rector\ChangesReporting\Output\ConsoleOutputFormatter; use Rector\ChangesReporting\Output\GitHubOutputFormatter; @@ -347,6 +349,7 @@ final class LazyContainerFactory TypeNodeResolver::class, NodeScopeResolver::class, ReflectionProvider::class, + DependencyResolver::class, ]; /** @@ -452,6 +455,7 @@ public function create(): RectorConfig $rectorConfig->singleton(FileProcessor::class); $rectorConfig->singleton(PostFileProcessor::class); + $rectorConfig->singleton(FileDependencyCollector::class); $rectorConfig->when(RectorNodeTraverser::class) ->needs('$rectors') @@ -476,6 +480,7 @@ static function (Container $container): DynamicSourceLocatorProvider { // resettable $rectorConfig->tag(DynamicSourceLocatorProvider::class, ResettableInterface::class); $rectorConfig->tag(RenamedClassesDataCollector::class, ResettableInterface::class); + $rectorConfig->tag(FileDependencyCollector::class, ResettableInterface::class); // caching $rectorConfig->singleton(Cache::class, static function (Container $container): Cache { diff --git a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php index d66d08bd15d..ae742b0c1bf 100644 --- a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php +++ b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php @@ -89,6 +89,7 @@ use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\ScopeContext; use PHPStan\Analyser\UndefinedVariableException; +use PHPStan\Dependency\DependencyResolver; use PHPStan\Node\FunctionCallableNode; use PHPStan\Node\InstantiationCallableNode; use PHPStan\Node\MethodCallableNode; @@ -102,12 +103,14 @@ use PHPStan\ShouldNotHappenException; use PHPStan\Type\ObjectType; use PHPStan\Type\TypeCombinator; +use Rector\Caching\FileDependencyCollector; use Rector\Contract\PhpParser\DecoratingNodeVisitorInterface; use Rector\NodeAnalyzer\ClassAnalyzer; use Rector\NodeNameResolver\NodeNameResolver; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\PhpParser\Node\FileNode; use Rector\Util\Reflection\PrivatesAccessor; +use Throwable; use Webmozart\Assert\Assert; /** @@ -130,7 +133,9 @@ public function __construct( private ScopeFactory $scopeFactory, private PrivatesAccessor $privatesAccessor, private NodeNameResolver $nodeNameResolver, - private ClassAnalyzer $classAnalyzer + private ClassAnalyzer $classAnalyzer, + private DependencyResolver $dependencyResolver, + private FileDependencyCollector $fileDependencyCollector ) { // @todo make use of immutable, to avoid tedious traversing $this->nodeTraverser = new NodeTraverser(...$decoratingNodeVisitors); @@ -162,6 +167,10 @@ public function processNodes( $mutatingScope = $mutatingScope->toMutatingScope(); } + // capture the files this file depends on, so the unchanged-files cache + // invalidates when a dependency changes, see captureNodeDependencies() + $this->captureNodeDependencies($node, $mutatingScope, $filePath); + // the class reflection is resolved AFTER entering to class node // so we need to get it from the first after this one if ($node instanceof Class_ || $node instanceof Interface_ || $node instanceof Enum_) { @@ -730,4 +739,53 @@ private function processTrait(Trait_ $trait, MutatingScope $mutatingScope, calla $this->nodeScopeResolverProcessNodes($trait->stmts, $traitScope, $nodeCallback); $this->decorateNodeAttrGroups($trait, $traitScope, $nodeCallback); } + + /** + * Record the dependency files PHPStan surfaces for this node, with a memo per + * function call as signature dependencies are identical at every call site. + * The memo key is the resolved function name, so two calls only share an entry + * when they resolve to the same function. + */ + private function captureNodeDependencies(Node $node, MutatingScope $mutatingScope, string $filePath): void + { + $functionMemoKey = null; + if ($node instanceof FuncCall) { + $functionName = $this->nodeNameResolver->getName($node); + if (is_string($functionName)) { + $functionMemoKey = strtolower($functionName); + } + } + + $memoizedFiles = $functionMemoKey !== null + ? $this->fileDependencyCollector->getMemoizedFunctionDependencyFiles($functionMemoKey) + : null; + + if ($memoizedFiles !== null) { + foreach ($memoizedFiles as $memoizedFile) { + $this->fileDependencyCollector->record($filePath, $memoizedFile); + } + + return; + } + + try { + $nodeDependencies = $this->dependencyResolver->resolveDependencies($node, $mutatingScope); + $dependencyFiles = []; + foreach ($nodeDependencies->getReflections() as $dependencyReflection) { + $dependencyFile = $dependencyReflection->getFileName(); + if ($dependencyFile !== null) { + $dependencyFiles[] = $dependencyFile; + $this->fileDependencyCollector->record($filePath, $dependencyFile); + } + } + + if ($functionMemoKey !== null) { + $this->fileDependencyCollector->memoizeFunctionDependencyFiles($functionMemoKey, $dependencyFiles); + } + } catch (Throwable) { + // a failed capture leaves a possibly partial dependency set, mark the file + // so it is never cached with it and gets reprocessed on every run instead + $this->fileDependencyCollector->markFailed($filePath); + } + } } diff --git a/tests/Caching/Detector/ChangedFilesDetectorTest.php b/tests/Caching/Detector/ChangedFilesDetectorTest.php index dfc95a3176a..a3022a470de 100644 --- a/tests/Caching/Detector/ChangedFilesDetectorTest.php +++ b/tests/Caching/Detector/ChangedFilesDetectorTest.php @@ -5,17 +5,22 @@ namespace Rector\Tests\Caching\Detector; use Rector\Caching\Detector\ChangedFilesDetector; +use Rector\Caching\FileDependencyCollector; use Rector\Testing\PHPUnit\AbstractLazyTestCase; final class ChangedFilesDetectorTest extends AbstractLazyTestCase { private ChangedFilesDetector $changedFilesDetector; + private FileDependencyCollector $fileDependencyCollector; + protected function setUp(): void { parent::setUp(); $this->changedFilesDetector = $this->make(ChangedFilesDetector::class); + $this->fileDependencyCollector = $this->make(FileDependencyCollector::class); + $this->fileDependencyCollector->reset(); } protected function tearDown(): void @@ -37,6 +42,44 @@ public function testHasFileChanged(): void $this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath)); } + public function testDependencyChangeInvalidatesFile(): void + { + $filePath = __DIR__ . '/Source/file.php'; + $dependencyFilePath = sys_get_temp_dir() . '/rector_changed_files_detector_dependency_test.php'; + file_put_contents($dependencyFilePath, "fileDependencyCollector->record($filePath, $dependencyFilePath); + $this->changedFilesDetector->addCacheableFile($filePath); + $this->changedFilesDetector->cacheFile($filePath); + + // simulate a fresh process run with unchanged files + $this->fileDependencyCollector->reset(); + $this->assertFalse($this->changedFilesDetector->hasFileChanged($filePath)); + + // a dependency change must invalidate the file, even though its own content is unchanged + file_put_contents( + $dependencyFilePath, + "fileDependencyCollector->reset(); + $this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath)); + + unlink($dependencyFilePath); + } + + public function testFailedDependencyCapturePreventsCaching(): void + { + $filePath = __DIR__ . '/Source/file.php'; + + // a failed capture means the dependency set may be incomplete → never cache + $this->fileDependencyCollector->markFailed($filePath); + $this->changedFilesDetector->addCacheableFile($filePath); + $this->changedFilesDetector->cacheFile($filePath); + + $this->fileDependencyCollector->reset(); + $this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath)); + } + public function provideConfigFilePath(): string { return __DIR__ . '/config.php'; From 14172da4c1eb7e40a48676ef2f0c3e5c1bc21278 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Wed, 10 Jun 2026 21:30:17 +0200 Subject: [PATCH 2/3] perf: replay cached dry-run diffs for unchanged files Files with a pending diff are never marked clean in dry-run mode, the diff must keep being reported, so every warm dry-run reprocessed them from scratch. On a 4,400-file project with 37 pending diffs that was ~11s per run. Cache the FileDiff with the file's own hash plus one hash per captured dependency; when all still match, replay the cached diff instead of reprocessing, skipping scope resolution entirely. Dry-run only: write mode always computes fresh. --no-diffs results never cross into normal entries, and the original hasChanged flag is replayed, as a rule can report line changes while printing identical content. Warm dry-run on the same project: ~9x faster single process, ~3.5x parallel. Output stays byte-identical in every cache state. Co-Authored-By: Claude Opus 4.8 --- src/Application/ApplicationFileProcessor.php | 24 ++- src/Caching/DryRunDiffCache.php | 108 ++++++++++++ src/Caching/Enum/CacheKey.php | 2 + .../LazyContainerFactory.php | 2 + tests/Caching/DryRunDiffCacheTest.php | 154 ++++++++++++++++++ 5 files changed, 286 insertions(+), 4 deletions(-) create mode 100644 src/Caching/DryRunDiffCache.php create mode 100644 tests/Caching/DryRunDiffCacheTest.php diff --git a/src/Application/ApplicationFileProcessor.php b/src/Application/ApplicationFileProcessor.php index 98f75c0d14e..34a15b124ec 100644 --- a/src/Application/ApplicationFileProcessor.php +++ b/src/Application/ApplicationFileProcessor.php @@ -8,6 +8,7 @@ use PHPStan\Parser\ParserErrorsException; use Rector\Application\Provider\CurrentFileProvider; use Rector\Caching\Detector\ChangedFilesDetector; +use Rector\Caching\DryRunDiffCache; use Rector\Configuration\Option; use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\FileSystem\FilesFinder; @@ -49,6 +50,7 @@ public function __construct( private readonly FileProcessor $fileProcessor, private readonly ArrayParametersMerger $arrayParametersMerger, private readonly MissConfigurationReporter $missConfigurationReporter, + private readonly DryRunDiffCache $dryRunDiffCache, ) { } @@ -168,16 +170,30 @@ private function processFile(File $file, Configuration $configuration): FileProc { $this->currentFileProvider->setFile($file); + // a selective run applies a subset of rules, so its results are not + // interchangeable with full runs → bypass both caches entirely + $isSelectiveRun = $configuration->getOnlyRule() !== null || $configuration->getOnlySuffix() !== null; + $useDiffCache = $configuration->isDryRun() && ! $isSelectiveRun; + + if ($useDiffCache) { + $cachedFileProcessResult = $this->dryRunDiffCache->load($file, $configuration); + if ($cachedFileProcessResult instanceof FileProcessResult) { + return $cachedFileProcessResult; + } + } + $fileProcessResult = $this->fileProcessor->processFile($file, $configuration); + $fileDiff = $fileProcessResult->getFileDiff(); if ($fileProcessResult->getSystemErrors() !== []) { $this->changedFilesDetector->invalidateFile($file->getFilePath()); - } elseif (! $configuration->isDryRun() || ! $fileProcessResult->getFileDiff() instanceof FileDiff) { - // a file clean under a subset of rules is not necessarily clean under all rules, - // caching it would hide its pending changes from the next full run - if ($configuration->getOnlyRule() === null && $configuration->getOnlySuffix() === null) { + } elseif (! $configuration->isDryRun() || ! $fileDiff instanceof FileDiff) { + // a file clean under a subset of rules is not necessarily clean under all rules + if (! $isSelectiveRun) { $this->changedFilesDetector->cacheFile($file->getFilePath()); } + } elseif ($useDiffCache) { + $this->dryRunDiffCache->save($file, $configuration, $fileDiff, $fileProcessResult->hasChanged()); } return $fileProcessResult; diff --git a/src/Caching/DryRunDiffCache.php b/src/Caching/DryRunDiffCache.php new file mode 100644 index 00000000000..613bdd96a9b --- /dev/null +++ b/src/Caching/DryRunDiffCache.php @@ -0,0 +1,108 @@ +cache->load($this->key($file), CacheKey::FILE_DIFF_KEY); + if (! is_array($cached)) { + return null; + } + + // own content + config must match + if (($cached['hash'] ?? null) !== $this->contentHash($file, $configuration)) { + return null; + } + + // every dependency captured last run must still hash to the same value + $cachedDependencyHashes = $cached['deps'] ?? null; + if (! is_array($cachedDependencyHashes)) { + return null; + } + + if ($this->fileDependencyCollector->hasAnyChangedDependency($cachedDependencyHashes)) { + return null; + } + + $diffJson = $cached['diff'] ?? null; + if (! is_array($diffJson)) { + return null; + } + + // a rule can report line changes while printing identical content, so a diff + // does not imply a changed file → replay the original flag or warm runs + // would report phantom changed files + $hasChanged = $cached['changed'] ?? null; + if (! is_bool($hasChanged)) { + return null; + } + + return new FileProcessResult([], FileDiff::decode($diffJson), $hasChanged); + } + + public function save(File $file, Configuration $configuration, FileDiff $fileDiff, bool $hasChanged): void + { + // a failed capture means a possibly incomplete set, skip caching so the file is reprocessed + $dependencyHashes = $this->fileDependencyCollector->getDependencyFileHashes($file->getFilePath()); + if ($dependencyHashes === null) { + return; + } + + $diffJson = $fileDiff->jsonSerialize(); + // decompose objects to plain arrays so the var_export-based file cache can round-trip them + $diffJson[BridgeItem::RECTORS_WITH_LINE_CHANGES] = array_map( + static fn (RectorWithLineChange $rectorWithLineChange): array => $rectorWithLineChange->jsonSerialize(), + $diffJson[BridgeItem::RECTORS_WITH_LINE_CHANGES] + ); + + $this->cache->save($this->key($file), CacheKey::FILE_DIFF_KEY, [ + 'hash' => $this->contentHash($file, $configuration), + 'deps' => $dependencyHashes, + 'diff' => $diffJson, + 'changed' => $hasChanged, + ]); + } + + private function key(File $file): string + { + return 'diff_' . $this->fileHasher->hash($file->getFilePath()); + } + + private function contentHash(File $file, Configuration $configuration): string + { + // --no-diffs changes the produced FileDiff content but is not part of the + // parameter hash, include it so entries do not cross-replay + return $this->fileHasher->hash($file->getOriginalFileContent()) + . '_' . SimpleParameterProvider::hash() + . ($configuration->shouldShowDiffs() ? '' : '_no-diffs'); + } +} diff --git a/src/Caching/Enum/CacheKey.php b/src/Caching/Enum/CacheKey.php index eaffc1c6c09..cdcd9c9e105 100644 --- a/src/Caching/Enum/CacheKey.php +++ b/src/Caching/Enum/CacheKey.php @@ -12,4 +12,6 @@ final class CacheKey public const string CONFIGURATION_HASH_KEY = 'configuration_hash'; public const string FILE_HASH_KEY = 'file_hash'; + + public const string FILE_DIFF_KEY = 'file_diff'; } diff --git a/src/DependencyInjection/LazyContainerFactory.php b/src/DependencyInjection/LazyContainerFactory.php index b74f6de1971..d9a8b548c41 100644 --- a/src/DependencyInjection/LazyContainerFactory.php +++ b/src/DependencyInjection/LazyContainerFactory.php @@ -39,6 +39,7 @@ use Rector\Caching\CacheFactory; use Rector\Caching\Config\FileHashComputer; use Rector\Caching\Contract\CacheMetaExtensionInterface; +use Rector\Caching\DryRunDiffCache; use Rector\Caching\FileDependencyCollector; use Rector\ChangesReporting\Contract\Output\OutputFormatterInterface; use Rector\ChangesReporting\Output\ConsoleOutputFormatter; @@ -456,6 +457,7 @@ public function create(): RectorConfig $rectorConfig->singleton(FileProcessor::class); $rectorConfig->singleton(PostFileProcessor::class); $rectorConfig->singleton(FileDependencyCollector::class); + $rectorConfig->singleton(DryRunDiffCache::class); $rectorConfig->when(RectorNodeTraverser::class) ->needs('$rectors') diff --git a/tests/Caching/DryRunDiffCacheTest.php b/tests/Caching/DryRunDiffCacheTest.php new file mode 100644 index 00000000000..ecd65cc4bbe --- /dev/null +++ b/tests/Caching/DryRunDiffCacheTest.php @@ -0,0 +1,154 @@ +cacheDirectory = sys_get_temp_dir() . '/rector_dry_run_diff_cache_test_' . getmypid(); + + $this->sourceFilePath = $this->cacheDirectory . '/Source.php'; + $this->dependencyFilePath = $this->cacheDirectory . '/Dependency.php'; + + $filesystem = new Filesystem(); + $filesystem->mkdir($this->cacheDirectory); + $filesystem->dumpFile($this->sourceFilePath, "dumpFile($this->dependencyFilePath, "fileDependencyCollector = new FileDependencyCollector($fileHasher); + + $this->dryRunDiffCache = new DryRunDiffCache( + new Cache(new FileCacheStorage($this->cacheDirectory, $filesystem)), + $fileHasher, + $this->fileDependencyCollector + ); + } + + protected function tearDown(): void + { + (new Filesystem())->remove($this->cacheDirectory); + } + + public function testSaveLoadRoundTrip(): void + { + $file = $this->createFile(); + $configuration = new Configuration(isDryRun: true); + + $this->assertNull($this->dryRunDiffCache->load($file, $configuration)); + + $this->fileDependencyCollector->record($this->sourceFilePath, $this->dependencyFilePath); + $this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff(), true); + + $cachedFileProcessResult = $this->dryRunDiffCache->load($file, $configuration); + $this->assertInstanceOf(FileProcessResult::class, $cachedFileProcessResult); + $this->assertTrue($cachedFileProcessResult->hasChanged()); + + $loadedFileDiff = $cachedFileProcessResult->getFileDiff(); + $this->assertInstanceOf(FileDiff::class, $loadedFileDiff); + $this->assertSame('some diff', $loadedFileDiff->getDiff()); + $this->assertSame([RemoveUnusedPrivateMethodRector::class], $loadedFileDiff->getRectorClasses()); + } + + public function testReplayKeepsUnchangedFlag(): void + { + $file = $this->createFile(); + $configuration = new Configuration(isDryRun: true); + + // a rule can report line changes while the printed content stays identical + $this->fileDependencyCollector->record($this->sourceFilePath, $this->dependencyFilePath); + $this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff(), false); + + $cachedFileProcessResult = $this->dryRunDiffCache->load($file, $configuration); + $this->assertInstanceOf(FileProcessResult::class, $cachedFileProcessResult); + $this->assertFalse($cachedFileProcessResult->hasChanged()); + } + + public function testChangedOwnContentInvalidates(): void + { + $file = $this->createFile(); + $configuration = new Configuration(isDryRun: true); + $this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff(), true); + + $changedFile = new File($this->sourceFilePath, "assertNull($this->dryRunDiffCache->load($changedFile, $configuration)); + } + + public function testChangedDependencyInvalidates(): void + { + $file = $this->createFile(); + $configuration = new Configuration(isDryRun: true); + + $this->fileDependencyCollector->record($this->sourceFilePath, $this->dependencyFilePath); + $this->dryRunDiffCache->save($file, $configuration, $this->createFileDiff(), true); + $this->assertInstanceOf(FileProcessResult::class, $this->dryRunDiffCache->load($file, $configuration)); + + (new Filesystem())->dumpFile( + $this->dependencyFilePath, + "cacheDirectory, new Filesystem())), + $freshFileHasher, + new FileDependencyCollector($freshFileHasher) + ); + + $this->assertNull($freshDryRunDiffCache->load($file, $configuration)); + } + + public function testNoDiffsConfigurationDoesNotCrossReplay(): void + { + $file = $this->createFile(); + + $this->dryRunDiffCache->save( + $file, + new Configuration(isDryRun: true, showDiffs: false), + $this->createFileDiff(), + true + ); + + $this->assertNull($this->dryRunDiffCache->load($file, new Configuration(isDryRun: true, showDiffs: true))); + } + + private function createFile(): File + { + return new File($this->sourceFilePath, (string) file_get_contents($this->sourceFilePath)); + } + + private function createFileDiff(): FileDiff + { + return new FileDiff($this->sourceFilePath, 'some diff', 'some diff formatted', [ + new RectorWithLineChange(RemoveUnusedPrivateMethodRector::class, 7), + ]); + } +} From bd25a20bd74baa0ac4b0864fbf7e369e78cd0d97 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Thu, 11 Jun 2026 09:38:01 +0200 Subject: [PATCH 3/3] perf: memoize the parameter hash in DryRunDiffCache SimpleParameterProvider::hash() serializes the whole parameter bag and contentHash() runs per file, so a warm run paid the serialization once per file (~46ms per 3,200 calls with a 300-entry skip list). Co-Authored-By: Claude Opus 4.8 --- src/Caching/DryRunDiffCache.php | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Caching/DryRunDiffCache.php b/src/Caching/DryRunDiffCache.php index 613bdd96a9b..42ae1bcf4b4 100644 --- a/src/Caching/DryRunDiffCache.php +++ b/src/Caching/DryRunDiffCache.php @@ -22,12 +22,17 @@ * * @see \Rector\Tests\Caching\DryRunDiffCacheTest */ -final readonly class DryRunDiffCache +final class DryRunDiffCache { + /** + * memoized: hashing serializes the whole parameter bag, and this runs per file + */ + private ?string $parameterHash = null; + public function __construct( - private Cache $cache, - private FileHasher $fileHasher, - private FileDependencyCollector $fileDependencyCollector, + private readonly Cache $cache, + private readonly FileHasher $fileHasher, + private readonly FileDependencyCollector $fileDependencyCollector, ) { } @@ -101,8 +106,10 @@ private function contentHash(File $file, Configuration $configuration): string { // --no-diffs changes the produced FileDiff content but is not part of the // parameter hash, include it so entries do not cross-replay + $this->parameterHash ??= SimpleParameterProvider::hash(); + return $this->fileHasher->hash($file->getOriginalFileContent()) - . '_' . SimpleParameterProvider::hash() + . '_' . $this->parameterHash . ($configuration->shouldShowDiffs() ? '' : '_no-diffs'); } }