From a74d7db77c6290786810c389a9fcec8c21e591e1 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 23 Oct 2026 10:36:09 +0200 Subject: [PATCH 1/2] [Tests] Add reproducer for cross-file cache false negative (#8493) --- .../CacheFileDependencyTest.php | 104 ++++++++++++++++++ .../config-parent-return-type.php | 11 ++ 2 files changed, 115 insertions(+) create mode 100644 tests/Application/ApplicationFileProcessor/CacheFileDependencyTest.php create mode 100644 tests/Application/ApplicationFileProcessor/config-parent-return-type.php diff --git a/tests/Application/ApplicationFileProcessor/CacheFileDependencyTest.php b/tests/Application/ApplicationFileProcessor/CacheFileDependencyTest.php new file mode 100644 index 00000000000..abb624e8d77 --- /dev/null +++ b/tests/Application/ApplicationFileProcessor/CacheFileDependencyTest.php @@ -0,0 +1,104 @@ +bootFromConfigFiles([__DIR__ . '/config-parent-return-type.php']); + + $this->applicationFileProcessor = $this->make(ApplicationFileProcessor::class); + $this->changedFilesDetector = $this->make(ChangedFilesDetector::class); + + $this->temporaryDirectory = sys_get_temp_dir() . '/_rector_cache_file_dependency_test_src'; + } + + protected function tearDown(): void + { + $this->changedFilesDetector->clear(); + FileSystem::delete($this->temporaryDirectory); + } + + public function testChildIsNotReconsideredWhenParentChanges(): void + { + $parentFilePath = $this->temporaryDirectory . '/ParentClass.php'; + $childFilePath = $this->temporaryDirectory . '/ChildClass.php'; + + // parent method has no return type yet, so the child gets nothing to copy + FileSystem::write($parentFilePath, $this->createParentContents('')); + FileSystem::write($childFilePath, $this->createChildContents()); + + $this->applicationFileProcessor->processFiles( + [$parentFilePath, $childFilePath], + new Configuration(isDryRun: true) + ); + + // both files are clean on this run and cached as such + $this->assertFalse($this->changedFilesDetector->hasFileChanged($childFilePath)); + + // the parent now declares a return type the child should inherit on the next run + FileSystem::write($parentFilePath, $this->createParentContents(': string')); + + // BUG #8493: the child's output now depends on the changed parent, yet the cache keys the + // child only by its own (untouched) contents, so it is wrongly reported as unchanged. + // Once per-file dependency tracking lands, flip this to assertTrue. + $this->assertFalse($this->changedFilesDetector->hasFileChanged($childFilePath)); + } + + private function createParentContents(string $returnType): string + { + return <<cacheDirectory(sys_get_temp_dir() . '/_rector_cache_file_dependency_test'); + $rectorConfig->rule(AddReturnTypeDeclarationBasedOnParentClassMethodRector::class); +}; From cef5e12d9e28c3778796c562895e68f7c2384526 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 23 Oct 2026 11:35:43 +0200 Subject: [PATCH 2/2] [Caching] Invalidate a file when its parent/interface/trait changes (#8493) --- src/Application/ApplicationFileProcessor.php | 5 +- src/Caching/Detector/ChangedFilesDetector.php | 68 ++++++++++++- .../Detector/FileDependencyResolver.php | 95 +++++++++++++++++++ src/Caching/Enum/CacheKey.php | 2 + .../CacheFileDependencyTest.php | 14 ++- .../Detector/FileDependencyResolverTest.php | 42 ++++++++ .../Detector/Source/ChildClassFixture.php | 9 ++ .../Detector/Source/ParentClassFixture.php | 13 +++ 8 files changed, 240 insertions(+), 8 deletions(-) create mode 100644 src/Caching/Detector/FileDependencyResolver.php create mode 100644 tests/Caching/Detector/FileDependencyResolverTest.php create mode 100644 tests/Caching/Detector/Source/ChildClassFixture.php create mode 100644 tests/Caching/Detector/Source/ParentClassFixture.php diff --git a/src/Application/ApplicationFileProcessor.php b/src/Application/ApplicationFileProcessor.php index 53b4cd884ff..4e762ec4e5c 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\Detector\FileDependencyResolver; use Rector\Configuration\Option; use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\FileSystem\FilesFinder; @@ -53,6 +54,7 @@ public function __construct( private readonly ArrayParametersMerger $arrayParametersMerger, private readonly MissConfigurationReporter $missConfigurationReporter, private readonly Skipper $skipper, + private readonly FileDependencyResolver $fileDependencyResolver, // @experimental, see --lpt private readonly LptScheduleFactory $lptScheduleFactory, private readonly ExperimentalParallelFileProcessor $experimentalParallelFileProcessor, @@ -201,7 +203,8 @@ private function processFile(File $file, Configuration $configuration): FileProc // gate on the actual content change, not on FileDiff: a FileDiff also carries reported line changes // that print identically, and such files would otherwise be re-processed on every dry run // selective runs are safe to cache now — the key is scoped to the rule selection - $this->changedFilesDetector->cacheFile($file->getFilePath()); + $dependentFilePaths = $this->fileDependencyResolver->resolveDependentFilePaths($file); + $this->changedFilesDetector->cacheFile($file->getFilePath(), $dependentFilePaths); } return $fileProcessResult; diff --git a/src/Caching/Detector/ChangedFilesDetector.php b/src/Caching/Detector/ChangedFilesDetector.php index dcc897dc09a..8be929cf1fd 100644 --- a/src/Caching/Detector/ChangedFilesDetector.php +++ b/src/Caching/Detector/ChangedFilesDetector.php @@ -46,7 +46,10 @@ public function setActiveScope(array $onlyRules, ?string $onlySuffix, array $fil : '|only:' . implode(',', $onlyRules) . '|suffix:' . ($onlySuffix ?? '') . '|filter:' . implode(',', $filters); } - public function cacheFile(string $filePath): void + /** + * @param string[] $dependentFilePaths ancestry files whose change must also invalidate this file + */ + public function cacheFile(string $filePath, array $dependentFilePaths = []): void { $filePathCacheKey = $this->getFilePathCacheKey($filePath); @@ -57,6 +60,7 @@ public function cacheFile(string $filePath): void $hash = $this->hashFile($filePath); $this->cache->save($filePathCacheKey, CacheKey::FILE_HASH_KEY, $hash); + $this->storeDependentFiles($filePath, $dependentFilePaths); } public function addCacheableFile(string $filePath): void @@ -68,17 +72,24 @@ public function addCacheableFile(string $filePath): void public function hasFileChanged(string $filePath): bool { $cachedValue = $this->cache->load($this->getFilePathCacheKey($filePath), CacheKey::FILE_HASH_KEY); + $dependentFilesCacheKey = $this->getDependentFilesCacheKey($filePath); // a scoped (--only) run reuses the full-run cache: a file left clean by all rules stays // clean under a single rule too, and the content is still compared below if ($cachedValue === null && $this->scopeSuffix !== '') { $unscopedCacheKey = $this->fileHasher->hash($this->cacheKeyPath($filePath)); $cachedValue = $this->cache->load($unscopedCacheKey, CacheKey::FILE_HASH_KEY); + $dependentFilesCacheKey = CacheKey::DEPENDENT_FILES_KEY . '_' . $unscopedCacheKey; } if ($cachedValue !== null) { $currentFileHash = $this->hashFile($filePath); - return $currentFileHash !== $cachedValue; + if ($currentFileHash !== $cachedValue) { + return true; + } + + // the file itself is unchanged, but a parent/interface/trait it relies on might have changed + return $this->haveDependentFilesChanged($dependentFilesCacheKey); } // we don't have a value to compare against. Be defensive and assume its changed @@ -89,6 +100,7 @@ public function invalidateFile(string $filePath): void { $fileInfoCacheKey = $this->getFilePathCacheKey($filePath); $this->cache->clean($fileInfoCacheKey); + $this->cache->clean($this->getDependentFilesCacheKey($filePath)); unset($this->cacheableFiles[$fileInfoCacheKey]); } @@ -107,6 +119,58 @@ public function setFirstResolvedConfigFileInfo(string $filePath): void $this->storeConfigurationDataHash($filePath, $configurationSnapshot); } + /** + * @param string[] $dependentFilePaths + */ + private function storeDependentFiles(string $filePath, array $dependentFilePaths): void + { + $dependentFilesCacheKey = $this->getDependentFilesCacheKey($filePath); + + // drop any stale dependency set from a previous run, e.g. a parent class was removed + if ($dependentFilePaths === []) { + $this->cache->clean($dependentFilesCacheKey); + return; + } + + $dependentFileHashes = []; + foreach ($dependentFilePaths as $dependentFilePath) { + $dependentFileHashes[$dependentFilePath] = $this->hashFile($dependentFilePath); + } + + $this->cache->save($dependentFilesCacheKey, CacheKey::DEPENDENT_FILES_KEY, $dependentFileHashes); + } + + private function haveDependentFilesChanged(string $dependentFilesCacheKey): bool + { + $dependentFileHashes = $this->cache->load($dependentFilesCacheKey, CacheKey::DEPENDENT_FILES_KEY); + if (! is_array($dependentFileHashes)) { + return false; + } + + foreach ($dependentFileHashes as $dependentFilePath => $dependentFileHash) { + if (! is_string($dependentFilePath) || ! is_string($dependentFileHash)) { + // unexpected shape, be defensive and treat as changed + return true; + } + + // a dependency disappeared, the file must be reconsidered + if (! is_file($dependentFilePath)) { + return true; + } + + if ($this->hashFile($dependentFilePath) !== $dependentFileHash) { + return true; + } + } + + return false; + } + + private function getDependentFilesCacheKey(string $filePath): string + { + return CacheKey::DEPENDENT_FILES_KEY . '_' . $this->getFilePathCacheKey($filePath); + } + private function getFilePathCacheKey(string $filePath): string { return $this->fileHasher->hash($this->cacheKeyPath($filePath) . $this->scopeSuffix); diff --git a/src/Caching/Detector/FileDependencyResolver.php b/src/Caching/Detector/FileDependencyResolver.php new file mode 100644 index 00000000000..dcd1bc98ea4 --- /dev/null +++ b/src/Caching/Detector/FileDependencyResolver.php @@ -0,0 +1,95 @@ +betterNodeFinder->findInstanceOf($file->getNewStmts(), ClassLike::class); + if ($classLikes === []) { + return []; + } + + $dependentFilePaths = []; + foreach ($classLikes as $classLike) { + $className = $this->nodeNameResolver->getName($classLike); + if ($className === null) { + continue; + } + + if (! $this->reflectionProvider->hasClass($className)) { + continue; + } + + $classReflection = $this->reflectionProvider->getClass($className); + + // best-effort: resolving ancestors forces full reflection (incl. docblock parsing) and may + // throw on unresolvable or malformed ancestors; a miss here only skips a cache optimisation + try { + $ancestorReflections = $this->resolveAncestors($classReflection); + } catch (Throwable) { + continue; + } + + foreach ($ancestorReflections as $ancestorReflection) { + $fileName = $ancestorReflection->getFileName(); + if ($fileName === null) { + continue; + } + + $dependentFilePaths[$this->normalizePath($fileName)] = true; + } + } + + unset($dependentFilePaths[$this->normalizePath($file->getFilePath())]); + + return array_keys($dependentFilePaths); + } + + /** + * @return ClassReflection[] + */ + private function resolveAncestors(ClassReflection $classReflection): array + { + return [ + ...$classReflection->getParents(), + ...$classReflection->getInterfaces(), + ...$classReflection->getTraits(), + ]; + } + + private function normalizePath(string $filePath): string + { + $realPath = realpath($filePath); + + return $realPath !== false ? $realPath : $filePath; + } +} diff --git a/src/Caching/Enum/CacheKey.php b/src/Caching/Enum/CacheKey.php index eaffc1c6c09..018a6c9bd42 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 DEPENDENT_FILES_KEY = 'dependent_files'; } diff --git a/tests/Application/ApplicationFileProcessor/CacheFileDependencyTest.php b/tests/Application/ApplicationFileProcessor/CacheFileDependencyTest.php index abb624e8d77..66aeb9e70f5 100644 --- a/tests/Application/ApplicationFileProcessor/CacheFileDependencyTest.php +++ b/tests/Application/ApplicationFileProcessor/CacheFileDependencyTest.php @@ -7,6 +7,7 @@ use Nette\Utils\FileSystem; use Rector\Application\ApplicationFileProcessor; use Rector\Caching\Detector\ChangedFilesDetector; +use Rector\StaticReflection\DynamicSourceLocatorDecorator; use Rector\Testing\PHPUnit\AbstractLazyTestCase; use Rector\ValueObject\Configuration; @@ -44,7 +45,7 @@ protected function tearDown(): void FileSystem::delete($this->temporaryDirectory); } - public function testChildIsNotReconsideredWhenParentChanges(): void + public function testChildIsReconsideredWhenParentChanges(): void { $parentFilePath = $this->temporaryDirectory . '/ParentClass.php'; $childFilePath = $this->temporaryDirectory . '/ChildClass.php'; @@ -53,6 +54,10 @@ public function testChildIsNotReconsideredWhenParentChanges(): void FileSystem::write($parentFilePath, $this->createParentContents('')); FileSystem::write($childFilePath, $this->createChildContents()); + // register the files for static reflection, as the process command does for a real run + $this->make(DynamicSourceLocatorDecorator::class) + ->addPaths([$this->temporaryDirectory]); + $this->applicationFileProcessor->processFiles( [$parentFilePath, $childFilePath], new Configuration(isDryRun: true) @@ -64,10 +69,9 @@ public function testChildIsNotReconsideredWhenParentChanges(): void // the parent now declares a return type the child should inherit on the next run FileSystem::write($parentFilePath, $this->createParentContents(': string')); - // BUG #8493: the child's output now depends on the changed parent, yet the cache keys the - // child only by its own (untouched) contents, so it is wrongly reported as unchanged. - // Once per-file dependency tracking lands, flip this to assertTrue. - $this->assertFalse($this->changedFilesDetector->hasFileChanged($childFilePath)); + // the child's own contents are untouched, yet its output now depends on the changed parent, + // so it must be reconsidered rather than served from the stale cache entry (#8493) + $this->assertTrue($this->changedFilesDetector->hasFileChanged($childFilePath)); } private function createParentContents(string $returnType): string diff --git a/tests/Caching/Detector/FileDependencyResolverTest.php b/tests/Caching/Detector/FileDependencyResolverTest.php new file mode 100644 index 00000000000..96c243171b9 --- /dev/null +++ b/tests/Caching/Detector/FileDependencyResolverTest.php @@ -0,0 +1,42 @@ +fileDependencyResolver = $this->make(FileDependencyResolver::class); + $this->testingParser = $this->make(TestingParser::class); + } + + public function testResolvesParentClassFile(): void + { + $childFilePath = __DIR__ . '/Source/ChildClassFixture.php'; + $file = $this->testingParser->parseFilePathToFile($childFilePath); + + $dependentFilePaths = $this->fileDependencyResolver->resolveDependentFilePaths($file); + + $this->assertContains(realpath(__DIR__ . '/Source/ParentClassFixture.php'), $dependentFilePaths); + $this->assertNotContains(realpath($childFilePath), $dependentFilePaths); + } + + public function testClassWithoutAncestorsHasNoDependencies(): void + { + $file = $this->testingParser->parseFilePathToFile(__DIR__ . '/Source/ParentClassFixture.php'); + + $this->assertSame([], $this->fileDependencyResolver->resolveDependentFilePaths($file)); + } +} diff --git a/tests/Caching/Detector/Source/ChildClassFixture.php b/tests/Caching/Detector/Source/ChildClassFixture.php new file mode 100644 index 00000000000..6a7b4e9f24b --- /dev/null +++ b/tests/Caching/Detector/Source/ChildClassFixture.php @@ -0,0 +1,9 @@ +