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 new file mode 100644 index 00000000000..66aeb9e70f5 --- /dev/null +++ b/tests/Application/ApplicationFileProcessor/CacheFileDependencyTest.php @@ -0,0 +1,108 @@ +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 testChildIsReconsideredWhenParentChanges(): 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()); + + // 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) + ); + + // 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')); + + // 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 + { + return <<cacheDirectory(sys_get_temp_dir() . '/_rector_cache_file_dependency_test'); + $rectorConfig->rule(AddReturnTypeDeclarationBasedOnParentClassMethodRector::class); +}; 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 @@ +