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';