From 1f5c42a90909d3a75d62876d04fd90416a4a61f4 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Thu, 11 Jun 2026 10:25:31 +0200 Subject: [PATCH] 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 | 59 ++++++- .../Detector/ChangedFilesDetectorTest.php | 43 +++++ 6 files changed, 306 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..b4458c40427 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,52 @@ 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 prefers the resolvedName attribute, so two files importing + * different functions under the same unqualified name can never collide. + */ + private function captureNodeDependencies(Node $node, MutatingScope $mutatingScope, string $filePath): void + { + $functionMemoKey = null; + if ($node instanceof FuncCall && $node->name instanceof Name) { + $resolvedName = $node->name->getAttribute('resolvedName'); + $nameForMemoKey = $resolvedName instanceof Name ? $resolvedName : $node->name; + $functionMemoKey = $mutatingScope->getNamespace() . '|' . strtolower($nameForMemoKey->toCodeString()); + } + + $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';