diff --git a/src/Caching/Config/FileHashComputer.php b/src/Caching/Config/FileHashComputer.php index 64148db5e4b..37fe42a7d33 100644 --- a/src/Caching/Config/FileHashComputer.php +++ b/src/Caching/Config/FileHashComputer.php @@ -17,7 +17,7 @@ public function compute(string $filePath): string { $this->ensureIsPhp($filePath); - $parametersHash = SimpleParameterProvider::hash(); + $parametersHash = SimpleParameterProvider::hashForCacheInvalidation(); return sha1($filePath . $parametersHash . VersionResolver::PACKAGE_VERSION); } diff --git a/src/Caching/Detector/ChangedFilesDetector.php b/src/Caching/Detector/ChangedFilesDetector.php index 93bc528806f..525b59d2f81 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\Configuration\Parameter\SimpleParameterProvider; use Rector\Util\FileHasher; /** @@ -90,8 +91,8 @@ public function clear(): void public function setFirstResolvedConfigFileInfo(string $filePath): void { // the first config is core to all → if it was changed, just invalidate it - $configHash = $this->fileHashComputer->compute($filePath); - $this->storeConfigurationDataHash($filePath, $configHash); + $configurationSnapshot = $this->createConfigurationSnapshot($filePath); + $this->storeConfigurationDataHash($filePath, $configurationSnapshot); } private function resolvePath(string $filePath): string @@ -114,26 +115,99 @@ private function hashFile(string $filePath): string return $this->fileHasher->hashFiles([$this->resolvePath($filePath)]); } - private function storeConfigurationDataHash(string $filePath, string $configurationHash): void + /** + * @return array{hash: string, rules: string[], sets: string[], skip: string[]} + */ + private function createConfigurationSnapshot(string $filePath): array + { + $directionalParameters = SimpleParameterProvider::provideCacheDirectionalParameters(); + + return [ + 'hash' => $this->fileHashComputer->compute($filePath), + 'rules' => $this->hashEach($directionalParameters['rules']), + 'sets' => $this->hashEach($directionalParameters['sets']), + 'skip' => $this->hashEach($directionalParameters['skip']), + ]; + } + + /** + * @param array{hash: string, rules: string[], sets: string[], skip: string[]} $configurationSnapshot + */ + private function storeConfigurationDataHash(string $filePath, array $configurationSnapshot): void { $key = CacheKey::CONFIGURATION_HASH_KEY . '_' . $this->getFilePathCacheKey($filePath); - $this->invalidateCacheIfConfigurationChanged($key, $configurationHash); + $this->invalidateCacheIfConfigurationChanged($key, $configurationSnapshot); - $this->cache->save($key, CacheKey::CONFIGURATION_HASH_KEY, $configurationHash); + $this->cache->save($key, CacheKey::CONFIGURATION_HASH_KEY, $configurationSnapshot); } - private function invalidateCacheIfConfigurationChanged(string $key, string $configurationHash): void + /** + * @param array{hash: string, rules: string[], sets: string[], skip: string[]} $configurationSnapshot + */ + private function invalidateCacheIfConfigurationChanged(string $key, array $configurationSnapshot): void { $oldCachedValue = $this->cache->load($key, CacheKey::CONFIGURATION_HASH_KEY); + + // first run, nothing to compare against if ($oldCachedValue === null) { return; } - if ($oldCachedValue === $configurationHash) { + // legacy string format from an older Rector version → be safe and reset once + if (! is_array($oldCachedValue)) { + $this->clear(); return; } - // should be unique per getcwd() - $this->clear(); + if ($this->shouldInvalidateCache($oldCachedValue, $configurationSnapshot)) { + // should be unique per getcwd() + $this->clear(); + } + } + + /** + * @param array $oldSnapshot + * @param array{hash: string, rules: string[], sets: string[], skip: string[]} $newSnapshot + */ + private function shouldInvalidateCache(array $oldSnapshot, array $newSnapshot): bool + { + // an output-affecting parameter changed (php version, import names, indent, configured rule value, ...) + if (($oldSnapshot['hash'] ?? null) !== $newSnapshot['hash']) { + return true; + } + + // a rule or set was added → files clean so far may now be refactored + if ($this->hasAddedEntry($oldSnapshot['rules'] ?? [], $newSnapshot['rules'])) { + return true; + } + + if ($this->hasAddedEntry($oldSnapshot['sets'] ?? [], $newSnapshot['sets'])) { + return true; + } + + // a skip was removed → previously skipped transformations may now apply + return $this->hasAddedEntry($newSnapshot['skip'], $oldSnapshot['skip'] ?? []); + } + + /** + * Is there any entry present in $comparedEntries but missing from $baseEntries? + * A non-array on either side means an unexpected shape, treated as changed to stay safe. + */ + private function hasAddedEntry(mixed $baseEntries, mixed $comparedEntries): bool + { + if (! is_array($baseEntries) || ! is_array($comparedEntries)) { + return true; + } + + return array_any($comparedEntries, fn ($comparedEntry): bool => ! in_array($comparedEntry, $baseEntries, true)); + } + + /** + * @param mixed[] $values + * @return string[] + */ + private function hashEach(array $values): array + { + return array_map(static fn (mixed $value): string => sha1(serialize($value)), array_values($values)); } } diff --git a/src/Configuration/Parameter/SimpleParameterProvider.php b/src/Configuration/Parameter/SimpleParameterProvider.php index 2382c339465..5f0be8a24b5 100644 --- a/src/Configuration/Parameter/SimpleParameterProvider.php +++ b/src/Configuration/Parameter/SimpleParameterProvider.php @@ -13,6 +13,54 @@ */ final class SimpleParameterProvider { + /** + * Parameters that never change the refactored output - runtime tuning and reporting only. + * They are excluded from the cache invalidation hash, so toggling e.g. parallel or memory + * limit does not drop the whole cache. + * + * @var array + */ + private const array CACHE_IGNORED_PARAMETER_NAMES = [ + Option::PARALLEL, + Option::PARALLEL_JOB_SIZE, + Option::PARALLEL_MAX_NUMBER_OF_PROCESSES, + Option::PARALLEL_JOB_TIMEOUT_IN_SECONDS, + Option::MEMORY_LIMIT, + Option::NO_DIFFS, + Option::CACHE_DIR, + Option::CONTAINER_CACHE_DIRECTORY, + Option::EDITOR_URL, + Option::ABSOLUTE_FILE_PATH, + Option::REPORT_UNUSED_SKIPS, + Option::IS_RECTORCONFIG_BUILDER_RECREATED, + Option::IS_RUN_NARROWED, + Option::IS_CACHED_RUN, + Option::SKIPPED_RECTOR_RULES, + Option::SKIPPED_NON_RECTOR_CLASSES, + Option::SKIPPED_START_WITH_SHORT_OPEN_TAG_FILES, + Option::DEPRECATED_PHP_SETS_METHODS, + Option::DEPRECATED_ATTRIBUTES_SETS_ARGS, + Option::DEPRECATED_COMPOSER_BASED_ARGS, + Option::LEVEL_OVERFLOWS, + Option::CACHE_META_EXTENSIONS, + Option::COMPOSER_BOUND_RULE_CONFIGURATIONS, + Option::ROOT_STANDALONE_REGISTERED_RULES, + Option::SET_REGISTERED_RULES, + ]; + + /** + * Parameters compared by direction instead of the strict hash: adding a rule/set or removing a + * skip means more work and must drop the cache, while removing a rule/set or adding a skip is + * safe and keeps it. Handled in ChangedFilesDetector, so they are excluded from the strict hash. + * + * @var array + */ + private const array CACHE_DIRECTIONAL_PARAMETER_NAMES = [ + Option::REGISTERED_RECTOR_RULES, + Option::REGISTERED_RECTOR_SETS, + Option::SKIP, + ]; + /** * @var array */ @@ -96,12 +144,32 @@ public static function provideBoolParameter(string $name, ?bool $default = null) /** * @api - * For cache invalidation + * Strict hash for cache invalidation. Ignored and directionally compared parameters are left + * out, so only a real change to an output-affecting parameter drops the cache. + */ + public static function hashForCacheInvalidation(): string + { + $strictParameters = self::$parameters; + foreach ([...self::CACHE_IGNORED_PARAMETER_NAMES, ...self::CACHE_DIRECTIONAL_PARAMETER_NAMES] as $ignoredName) { + unset($strictParameters[$ignoredName]); + } + + ksort($strictParameters); + + return sha1(serialize($strictParameters)); + } + + /** + * @api + * @return array{rules: mixed[], sets: mixed[], skip: mixed[]} */ - public static function hash(): string + public static function provideCacheDirectionalParameters(): array { - $parameterKeys = self::$parameters; - return sha1(serialize($parameterKeys)); + return [ + 'rules' => self::$parameters[Option::REGISTERED_RECTOR_RULES] ?? [], + 'sets' => self::$parameters[Option::REGISTERED_RECTOR_SETS] ?? [], + 'skip' => self::$parameters[Option::SKIP] ?? [], + ]; } /** diff --git a/tests/Caching/Config/FileHashComputer/FileHashComputerTest.php b/tests/Caching/Config/FileHashComputer/FileHashComputerTest.php index 2d06d77d60b..403aee7fd51 100644 --- a/tests/Caching/Config/FileHashComputer/FileHashComputerTest.php +++ b/tests/Caching/Config/FileHashComputer/FileHashComputerTest.php @@ -13,52 +13,57 @@ final class FileHashComputerTest extends AbstractLazyTestCase { private FileHashComputer $fileHashComputer; + /** + * @var mixed[] + */ + private array $originalRules = []; + + /** + * @var mixed[] + */ + private array $originalFileExtensions = []; + protected function setUp(): void { parent::setUp(); $this->fileHashComputer = $this->make(FileHashComputer::class); + + // the parameter bag is a global static shared across the whole test process, restore it after + $this->originalRules = SimpleParameterProvider::provideArrayParameter(Option::REGISTERED_RECTOR_RULES); + $this->originalFileExtensions = SimpleParameterProvider::provideArrayParameter(Option::FILE_EXTENSIONS); } - public function testRectorPhpChanged(): void + protected function tearDown(): void { - SimpleParameterProvider::setParameter(Option::REGISTERED_RECTOR_RULES, null); - - $this->bootFromConfigFiles([__DIR__ . '/Fixture/rector.php']); - - $hashedFile = $this->fileHashComputer->compute(__DIR__ . '/Fixture/rector.php'); - - copy(__DIR__ . '/Fixture/rector.php', __DIR__ . '/Fixture/rector_temp.php'); - copy(__DIR__ . '/Fixture/updated_rector_rule.php', __DIR__ . '/Fixture/rector.php'); - - SimpleParameterProvider::setParameter(Option::REGISTERED_RECTOR_RULES, null); - - $this->bootFromConfigFiles([__DIR__ . '/Fixture/rector.php']); - - $newHashedFile = $this->fileHashComputer->compute(__DIR__ . '/Fixture/rector.php'); - rename(__DIR__ . '/Fixture/rector_temp.php', __DIR__ . '/Fixture/rector.php'); - - $this->assertNotSame($newHashedFile, $hashedFile); + SimpleParameterProvider::setParameter(Option::REGISTERED_RECTOR_RULES, $this->originalRules); + SimpleParameterProvider::setParameter(Option::FILE_EXTENSIONS, $this->originalFileExtensions); } - public function testRectorPhpNotChanged(): void + public function testOutputAffectingParameterChangesHash(): void { - SimpleParameterProvider::setParameter(Option::REGISTERED_RECTOR_RULES, null); + $configFilePath = __DIR__ . '/Fixture/rector.php'; - $this->bootFromConfigFiles([__DIR__ . '/Fixture/rector.php']); + SimpleParameterProvider::setParameter(Option::FILE_EXTENSIONS, ['php']); + $hashBefore = $this->fileHashComputer->compute($configFilePath); - $hashedFile = $this->fileHashComputer->compute(__DIR__ . '/Fixture/rector.php'); + SimpleParameterProvider::setParameter(Option::FILE_EXTENSIONS, ['php', 'phtml']); + $hashAfter = $this->fileHashComputer->compute($configFilePath); - copy(__DIR__ . '/Fixture/rector.php', __DIR__ . '/Fixture/rector_temp_equal.php'); - copy(__DIR__ . '/Fixture/rector_rule_equals.php', __DIR__ . '/Fixture/rector.php'); + $this->assertNotSame($hashBefore, $hashAfter); + } - SimpleParameterProvider::setParameter(Option::REGISTERED_RECTOR_RULES, null); + public function testRuleChangeIsExcludedFromHash(): void + { + $configFilePath = __DIR__ . '/Fixture/rector.php'; - $this->bootFromConfigFiles([__DIR__ . '/Fixture/rector.php']); + SimpleParameterProvider::setParameter(Option::REGISTERED_RECTOR_RULES, ['Rector\\SomeRule']); + $hashBefore = $this->fileHashComputer->compute($configFilePath); - $newHashedFile = $this->fileHashComputer->compute(__DIR__ . '/Fixture/rector.php'); - rename(__DIR__ . '/Fixture/rector_temp_equal.php', __DIR__ . '/Fixture/rector.php'); + // registered rules are compared directionally, not by the strict hash + SimpleParameterProvider::setParameter(Option::REGISTERED_RECTOR_RULES, ['Rector\\SomeRule', 'Rector\\OtherRule']); + $hashAfter = $this->fileHashComputer->compute($configFilePath); - $this->assertSame($newHashedFile, $hashedFile); + $this->assertSame($hashBefore, $hashAfter); } } diff --git a/tests/Caching/Config/FileHashComputer/Fixture/rector_rule_equals.php b/tests/Caching/Config/FileHashComputer/Fixture/rector_rule_equals.php deleted file mode 100644 index 545b045cddc..00000000000 --- a/tests/Caching/Config/FileHashComputer/Fixture/rector_rule_equals.php +++ /dev/null @@ -1,15 +0,0 @@ -rules([ - - // only spaced/comment added, no need to clear cache - DeclareStrictTypesRector::class - - ]); -}; diff --git a/tests/Caching/Config/FileHashComputer/Fixture/updated_rector_rule.php b/tests/Caching/Config/FileHashComputer/Fixture/updated_rector_rule.php deleted file mode 100644 index 06f08d7ead7..00000000000 --- a/tests/Caching/Config/FileHashComputer/Fixture/updated_rector_rule.php +++ /dev/null @@ -1,14 +0,0 @@ -rules([ - DeclareStrictTypesRector::class, - RemoveDeadStmtRector::class, - ]); -}; diff --git a/tests/Caching/Detector/ChangedFilesDetectorTest.php b/tests/Caching/Detector/ChangedFilesDetectorTest.php index dfc95a3176a..71b2a30ee3c 100644 --- a/tests/Caching/Detector/ChangedFilesDetectorTest.php +++ b/tests/Caching/Detector/ChangedFilesDetectorTest.php @@ -5,21 +5,47 @@ namespace Rector\Tests\Caching\Detector; use Rector\Caching\Detector\ChangedFilesDetector; +use Rector\Configuration\Option; +use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\Testing\PHPUnit\AbstractLazyTestCase; final class ChangedFilesDetectorTest extends AbstractLazyTestCase { private ChangedFilesDetector $changedFilesDetector; + /** + * @var mixed[] + */ + private array $originalRules = []; + + /** + * @var mixed[] + */ + private array $originalSkip = []; + + /** + * @var mixed[] + */ + private array $originalFileExtensions = []; + protected function setUp(): void { parent::setUp(); $this->changedFilesDetector = $this->make(ChangedFilesDetector::class); + + // the parameter bag is a global static shared across the whole test process, restore it after + $this->originalRules = SimpleParameterProvider::provideArrayParameter(Option::REGISTERED_RECTOR_RULES); + $this->originalSkip = SimpleParameterProvider::provideArrayParameter(Option::SKIP); + $this->originalFileExtensions = SimpleParameterProvider::provideArrayParameter(Option::FILE_EXTENSIONS); } protected function tearDown(): void { + SimpleParameterProvider::setParameter(Option::REGISTERED_RECTOR_RULES, $this->originalRules); + SimpleParameterProvider::setParameter(Option::SKIP, $this->originalSkip); + SimpleParameterProvider::setParameter(Option::FILE_EXTENSIONS, $this->originalFileExtensions); + $this->changedFilesDetector->clear(); } @@ -37,8 +63,90 @@ public function testHasFileChanged(): void $this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath)); } - public function provideConfigFilePath(): string + public function testCacheKeptWhenRuleRemoved(): void + { + $filePath = $this->cacheFileUnderRules(['Rector\\RuleA', 'Rector\\RuleB']); + + // removing a rule means strictly less work, cache stays valid + $this->applyRules(['Rector\\RuleA']); + + $this->assertFalse($this->changedFilesDetector->hasFileChanged($filePath)); + } + + public function testCacheClearedWhenRuleAdded(): void + { + $filePath = $this->cacheFileUnderRules(['Rector\\RuleA']); + + // adding a rule may refactor files that were clean so far, cache must drop + $this->applyRules(['Rector\\RuleA', 'Rector\\RuleB']); + + $this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath)); + } + + public function testCacheKeptWhenSkipAdded(): void + { + SimpleParameterProvider::setParameter(Option::SKIP, ['Rector\\RuleA']); + $filePath = $this->cacheFileUnderRules(['Rector\\RuleA', 'Rector\\RuleB']); + + // adding a skip means strictly less work, cache stays valid + SimpleParameterProvider::setParameter(Option::SKIP, ['Rector\\RuleA', 'Rector\\RuleB']); + $this->snapshotConfiguration(); + + $this->assertFalse($this->changedFilesDetector->hasFileChanged($filePath)); + } + + public function testCacheClearedWhenSkipRemoved(): void + { + SimpleParameterProvider::setParameter(Option::SKIP, ['Rector\\RuleA', 'Rector\\RuleB']); + $filePath = $this->cacheFileUnderRules(['Rector\\RuleA', 'Rector\\RuleB']); + + // removing a skip re-enables transformations, cache must drop + SimpleParameterProvider::setParameter(Option::SKIP, ['Rector\\RuleA']); + $this->snapshotConfiguration(); + + $this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath)); + } + + public function testCacheClearedWhenOutputAffectingParameterChanged(): void + { + SimpleParameterProvider::setParameter(Option::FILE_EXTENSIONS, ['php']); + $filePath = $this->cacheFileUnderRules(['Rector\\RuleA']); + + SimpleParameterProvider::setParameter(Option::FILE_EXTENSIONS, ['php', 'phtml']); + $this->snapshotConfiguration(); + + $this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath)); + } + + /** + * @param string[] $rules + */ + private function cacheFileUnderRules(array $rules): string + { + $filePath = __DIR__ . '/Source/file.php'; + + $this->applyRules($rules); + + $this->changedFilesDetector->addCacheableFile($filePath); + $this->changedFilesDetector->cacheFile($filePath); + + // sanity: the file is cached as clean before the configuration change under test + $this->assertFalse($this->changedFilesDetector->hasFileChanged($filePath)); + + return $filePath; + } + + /** + * @param string[] $rules + */ + private function applyRules(array $rules): void + { + SimpleParameterProvider::setParameter(Option::REGISTERED_RECTOR_RULES, $rules); + $this->snapshotConfiguration(); + } + + private function snapshotConfiguration(): void { - return __DIR__ . '/config.php'; + $this->changedFilesDetector->setFirstResolvedConfigFileInfo(__DIR__ . '/config.php'); } }