From e12dbfc8964f79fed3474d176a24d2c0b776b1b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 15:19:12 +0000 Subject: [PATCH 1/3] Deprecate cacheClass(), select cache storage internally Move the file-vs-memory cache storage decision into CacheFactory: an explicit cacheClass() still wins, otherwise file cache is used locally and in-memory cache in CI, where the ephemeral workspace makes writing a cache that is never re-read wasted IO. Previously this CI branch lived in config/config.php. Mark RectorConfig::cacheClass() and the withCache(cacheClass:) argument as deprecated; they stay functional for the rare case that needs to force a specific storage (e.g. the e2e cache tests). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Ti6vHRo3xLSHw84rxUw6Lb --- config/config.php | 8 ------ .../rector.php | 1 + e2e/timeout-file-not-cached/rector.php | 1 + src/Caching/CacheFactory.php | 27 +++++++++++++------ src/Config/RectorConfig.php | 5 ++++ src/Configuration/Option.php | 4 +-- src/Configuration/RectorConfigBuilder.php | 6 ++++- tests/Caching/Detector/config.php | 2 -- tests/Caching/ValueObject/Storage/config.php | 2 -- 9 files changed, 33 insertions(+), 23 deletions(-) diff --git a/config/config.php b/config/config.php index b1afe8533af..db093de4e03 100644 --- a/config/config.php +++ b/config/config.php @@ -2,9 +2,7 @@ declare(strict_types=1); -use OndraM\CiDetector\CiDetector; use Rector\Bootstrap\ExtensionConfigResolver; -use Rector\Caching\ValueObject\Storage\MemoryCacheStorage; use Rector\Config\RectorConfig; return static function (RectorConfig $rectorConfig): void { @@ -27,12 +25,6 @@ $rectorConfig->cacheDirectory(sys_get_temp_dir() . '/rector_cached_files'); $rectorConfig->containerCacheDirectory(sys_get_temp_dir()); - // use faster in-memory cache in CI. - // CI always starts from scratch, therefore IO intensive caching is not worth it - if (new CiDetector()->isCiDetected()) { - $rectorConfig->cacheClass(MemoryCacheStorage::class); - } - // load internal rector-* extension configs $extensionConfigResolver = new ExtensionConfigResolver(); foreach ($extensionConfigResolver->provide() as $extensionConfigFile) { diff --git a/e2e/applied-rule-removed-node-with-cache/rector.php b/e2e/applied-rule-removed-node-with-cache/rector.php index 84cdd17281d..34222c564d1 100644 --- a/e2e/applied-rule-removed-node-with-cache/rector.php +++ b/e2e/applied-rule-removed-node-with-cache/rector.php @@ -8,6 +8,7 @@ use Rector\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector; return static function (RectorConfig $rectorConfig): void { + // force file cache to verify the persisted cache across runs, even in CI $rectorConfig->cacheClass(FileCacheStorage::class); $rectorConfig->paths([ diff --git a/e2e/timeout-file-not-cached/rector.php b/e2e/timeout-file-not-cached/rector.php index b6053d61914..40356f0d51c 100644 --- a/e2e/timeout-file-not-cached/rector.php +++ b/e2e/timeout-file-not-cached/rector.php @@ -7,6 +7,7 @@ use Rector\Set\ValueObject\LevelSetList; return static function (RectorConfig $rectorConfig): void { + // force file cache to verify the persisted cache across runs, even in CI $rectorConfig->cacheClass(FileCacheStorage::class); $rectorConfig->parallel(0); diff --git a/src/Caching/CacheFactory.php b/src/Caching/CacheFactory.php index 5277e982734..11fce046712 100644 --- a/src/Caching/CacheFactory.php +++ b/src/Caching/CacheFactory.php @@ -4,6 +4,7 @@ namespace Rector\Caching; +use OndraM\CiDetector\CiDetector; use Rector\Caching\ValueObject\Storage\FileCacheStorage; use Rector\Caching\ValueObject\Storage\MemoryCacheStorage; use Rector\Configuration\Option; @@ -22,15 +23,9 @@ public function __construct( */ public function create(): Cache { - $cacheDirectory = SimpleParameterProvider::provideStringParameter(Option::CACHE_DIR); + if ($this->resolveCacheClass() === FileCacheStorage::class) { + $cacheDirectory = SimpleParameterProvider::provideStringParameter(Option::CACHE_DIR); - $cacheClass = FileCacheStorage::class; - - if (SimpleParameterProvider::hasParameter(Option::CACHE_CLASS)) { - $cacheClass = SimpleParameterProvider::provideStringParameter(Option::CACHE_CLASS); - } - - if ($cacheClass === FileCacheStorage::class) { // ensure cache directory exists if (! $this->fileSystem->exists($cacheDirectory)) { $this->fileSystem->mkdir($cacheDirectory); @@ -42,4 +37,20 @@ public function create(): Cache return new Cache(new MemoryCacheStorage()); } + + private function resolveCacheClass(): string + { + // explicit storage choice via the deprecated cacheClass() wins + if (SimpleParameterProvider::hasParameter(Option::CACHE_CLASS)) { + return SimpleParameterProvider::provideStringParameter(Option::CACHE_CLASS); + } + + // in CI the workspace is ephemeral and usually starts from scratch, + // so a file cache that is never read again is only wasted IO → use faster in-memory cache + if (new CiDetector()->isCiDetected()) { + return MemoryCacheStorage::class; + } + + return FileCacheStorage::class; + } } diff --git a/src/Config/RectorConfig.php b/src/Config/RectorConfig.php index 1f4581bb286..56857a5461b 100644 --- a/src/Config/RectorConfig.php +++ b/src/Config/RectorConfig.php @@ -431,6 +431,11 @@ public function containerCacheDirectory(string $directoryPath): void /** * @param class-string $cacheClass */ + #[Deprecated(message: <<<'TXT' + Cache storage is selected automatically: file cache locally, in-memory cache in CI, + where the ephemeral workspace makes writing a cache that is never re-read wasted IO. + Kept only for the rare case that needs to force a specific storage. + TXT)] public function cacheClass(string $cacheClass): void { Assert::isAOf($cacheClass, CacheStorageInterface::class); diff --git a/src/Configuration/Option.php b/src/Configuration/Option.php index 6c3ddb126fe..a0f989e0a93 100644 --- a/src/Configuration/Option.php +++ b/src/Configuration/Option.php @@ -146,8 +146,8 @@ final class Option public const string CACHE_DIR = 'cache_dir'; /** - * Cache backend. Most of the time we cache in files, but in ephemeral environment (e.g. CI), a faster `MemoryCacheStorage` can be useful. - * @internal Use RectorConfig::cacheClass() instead + * Cache backend override. Selected automatically by CacheFactory (file locally, in-memory in CI); + * only set when forcing a specific storage via the deprecated RectorConfig::cacheClass(). * * @var class-string * @internal diff --git a/src/Configuration/RectorConfigBuilder.php b/src/Configuration/RectorConfigBuilder.php index 97411cb1c03..cf01e97cbc8 100644 --- a/src/Configuration/RectorConfigBuilder.php +++ b/src/Configuration/RectorConfigBuilder.php @@ -315,7 +315,8 @@ public function __invoke(RectorConfig $rectorConfig): void } if ($this->cacheClass !== null) { - $rectorConfig->cacheClass($this->cacheClass); + // set directly, as RectorConfig::cacheClass() is deprecated + SimpleParameterProvider::setParameter(Option::CACHE_CLASS, $this->cacheClass); } if ($this->cacheDirectory !== null) { @@ -830,6 +831,9 @@ public function withFileExtensions(array $fileExtensions): self } /** + * The $cacheClass argument is deprecated. Cache storage is selected automatically: + * file cache locally, in-memory cache in CI. Pass it only to force a specific storage. + * * @param class-string|null $cacheClass */ public function withCache( diff --git a/tests/Caching/Detector/config.php b/tests/Caching/Detector/config.php index 5e535c343a8..5e5a9c61284 100644 --- a/tests/Caching/Detector/config.php +++ b/tests/Caching/Detector/config.php @@ -2,10 +2,8 @@ declare(strict_types=1); -use Rector\Caching\ValueObject\Storage\MemoryCacheStorage; use Rector\Config\RectorConfig; return static function (RectorConfig $rectorConfig): void { $rectorConfig->cacheDirectory(sys_get_temp_dir() . '/_rector_cached_files_test'); - $rectorConfig->cacheClass(MemoryCacheStorage::class); }; diff --git a/tests/Caching/ValueObject/Storage/config.php b/tests/Caching/ValueObject/Storage/config.php index 5e535c343a8..5e5a9c61284 100644 --- a/tests/Caching/ValueObject/Storage/config.php +++ b/tests/Caching/ValueObject/Storage/config.php @@ -2,10 +2,8 @@ declare(strict_types=1); -use Rector\Caching\ValueObject\Storage\MemoryCacheStorage; use Rector\Config\RectorConfig; return static function (RectorConfig $rectorConfig): void { $rectorConfig->cacheDirectory(sys_get_temp_dir() . '/_rector_cached_files_test'); - $rectorConfig->cacheClass(MemoryCacheStorage::class); }; From a6c95a7f201138a5075d72301de03f0c494bf2b9 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 6 Aug 2026 23:23:43 +0200 Subject: [PATCH 2/3] remove unneded cache options --- .github/workflows/e2e_with_cache.yaml | 54 ----------------------- phpstan.neon | 1 - src/Caching/CacheFactory.php | 34 +++++--------- src/Config/RectorConfig.php | 4 +- src/Configuration/Option.php | 12 ----- src/Configuration/RectorConfigBuilder.php | 15 +------ 6 files changed, 13 insertions(+), 107 deletions(-) delete mode 100644 .github/workflows/e2e_with_cache.yaml diff --git a/.github/workflows/e2e_with_cache.yaml b/.github/workflows/e2e_with_cache.yaml deleted file mode 100644 index 7c04221333c..00000000000 --- a/.github/workflows/e2e_with_cache.yaml +++ /dev/null @@ -1,54 +0,0 @@ -# This workflow runs system tests: Use the Rector application from the source -# checkout to process "fixture" projects in e2e/ directory -# to see if those can be processed successfully -name: End to End tests with cache - -on: - pull_request: - branches: - - main - push: - branches: - - main - -env: - # see https://github.com/composer/composer/issues/9368#issuecomment-718112361 - COMPOSER_ROOT_VERSION: "dev-main" - -jobs: - end_to_end: - runs-on: ubuntu-latest - timeout-minutes: 3 - strategy: - fail-fast: false - matrix: - php_version: ['8.4'] - directory: - - 'e2e/applied-rule-removed-node-with-cache' - - 'e2e/timeout-file-not-cached' - - name: End to end test - ${{ matrix.directory }} - - steps: - - uses: actions/checkout@v5 - - - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php_version }} - coverage: none - - # run in root rector-src - - run: composer install --ansi - - # run in e2e subdir - - - run: composer install --ansi - working-directory: ${{ matrix.directory }} - - # run e2e test - - run: php ../e2eTestRunner.php - working-directory: ${{ matrix.directory }} - - # this tests that a 2nd run with cache and "--dry-run" gives same results, see https://github.com/rectorphp/rector-src/pull/3614#issuecomment-1507742338 - - run: php ../e2eTestRunnerWithCache.php - working-directory: ${{ matrix.directory }} diff --git a/phpstan.neon b/phpstan.neon index 5e74f14907b..ed4572029f8 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -46,7 +46,6 @@ parameters: - rules-tests - utils - scripts - - e2e/e2eTestRunnerWithCache.php - e2e/e2eTestRunner.php scanDirectories: diff --git a/src/Caching/CacheFactory.php b/src/Caching/CacheFactory.php index 11fce046712..61d2dfd0dc4 100644 --- a/src/Caching/CacheFactory.php +++ b/src/Caching/CacheFactory.php @@ -23,34 +23,20 @@ public function __construct( */ public function create(): Cache { - if ($this->resolveCacheClass() === FileCacheStorage::class) { - $cacheDirectory = SimpleParameterProvider::provideStringParameter(Option::CACHE_DIR); - - // ensure cache directory exists - if (! $this->fileSystem->exists($cacheDirectory)) { - $this->fileSystem->mkdir($cacheDirectory); - } - - $fileCacheStorage = new FileCacheStorage($cacheDirectory, $this->fileSystem); - return new Cache($fileCacheStorage); - } - - return new Cache(new MemoryCacheStorage()); - } - - private function resolveCacheClass(): string - { - // explicit storage choice via the deprecated cacheClass() wins - if (SimpleParameterProvider::hasParameter(Option::CACHE_CLASS)) { - return SimpleParameterProvider::provideStringParameter(Option::CACHE_CLASS); - } - // in CI the workspace is ephemeral and usually starts from scratch, // so a file cache that is never read again is only wasted IO → use faster in-memory cache if (new CiDetector()->isCiDetected()) { - return MemoryCacheStorage::class; + return new Cache(new MemoryCacheStorage()); + } + + $cacheDirectory = SimpleParameterProvider::provideStringParameter(Option::CACHE_DIR); + + // ensure cache directory exists + if (! $this->fileSystem->exists($cacheDirectory)) { + $this->fileSystem->mkdir($cacheDirectory); } - return FileCacheStorage::class; + $fileCacheStorage = new FileCacheStorage($cacheDirectory, $this->fileSystem); + return new Cache($fileCacheStorage); } } diff --git a/src/Config/RectorConfig.php b/src/Config/RectorConfig.php index 56857a5461b..ef492c30300 100644 --- a/src/Config/RectorConfig.php +++ b/src/Config/RectorConfig.php @@ -434,13 +434,11 @@ public function containerCacheDirectory(string $directoryPath): void #[Deprecated(message: <<<'TXT' Cache storage is selected automatically: file cache locally, in-memory cache in CI, where the ephemeral workspace makes writing a cache that is never re-read wasted IO. - Kept only for the rare case that needs to force a specific storage. + The passed value is ignored. TXT)] public function cacheClass(string $cacheClass): void { Assert::isAOf($cacheClass, CacheStorageInterface::class); - - SimpleParameterProvider::setParameter(Option::CACHE_CLASS, $cacheClass); } /** diff --git a/src/Configuration/Option.php b/src/Configuration/Option.php index a0f989e0a93..e5fd0c98457 100644 --- a/src/Configuration/Option.php +++ b/src/Configuration/Option.php @@ -4,9 +4,6 @@ namespace Rector\Configuration; -use Rector\Caching\Contract\ValueObject\Storage\CacheStorageInterface; -use Rector\Caching\ValueObject\Storage\FileCacheStorage; - final class Option { public const string SOURCE = 'source'; @@ -145,15 +142,6 @@ final class Option */ public const string CACHE_DIR = 'cache_dir'; - /** - * Cache backend override. Selected automatically by CacheFactory (file locally, in-memory in CI); - * only set when forcing a specific storage via the deprecated RectorConfig::cacheClass(). - * - * @var class-string - * @internal - */ - public const string CACHE_CLASS = FileCacheStorage::class; - public const string DEBUG = 'debug'; public const string XDEBUG = 'xdebug'; diff --git a/src/Configuration/RectorConfigBuilder.php b/src/Configuration/RectorConfigBuilder.php index cf01e97cbc8..cb62b024e18 100644 --- a/src/Configuration/RectorConfigBuilder.php +++ b/src/Configuration/RectorConfigBuilder.php @@ -103,11 +103,6 @@ final class RectorConfigBuilder */ private array $fileExtensions = []; - /** - * @var null|class-string - */ - private ?string $cacheClass = null; - private ?string $cacheDirectory = null; private ?string $containerCacheDirectory = null; @@ -314,11 +309,6 @@ public function __invoke(RectorConfig $rectorConfig): void $rectorConfig->fileExtensions($this->fileExtensions); } - if ($this->cacheClass !== null) { - // set directly, as RectorConfig::cacheClass() is deprecated - SimpleParameterProvider::setParameter(Option::CACHE_CLASS, $this->cacheClass); - } - if ($this->cacheDirectory !== null) { $rectorConfig->cacheDirectory($this->cacheDirectory); } @@ -831,8 +821,8 @@ public function withFileExtensions(array $fileExtensions): self } /** - * The $cacheClass argument is deprecated. Cache storage is selected automatically: - * file cache locally, in-memory cache in CI. Pass it only to force a specific storage. + * The $cacheClass argument is deprecated and ignored. Cache storage is selected automatically: + * file cache locally, in-memory cache in CI. * * @param class-string|null $cacheClass */ @@ -842,7 +832,6 @@ public function withCache( ?string $containerCacheDirectory = null ): self { $this->cacheDirectory = $cacheDirectory; - $this->cacheClass = $cacheClass; $this->containerCacheDirectory = $containerCacheDirectory; return $this; From 63cf914653e4cd9fa28016c1f3cefc2e48a3b81b Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 10 Aug 2026 22:42:16 +0200 Subject: [PATCH 3/3] Remove dead e2e cache assets The e2e_with_cache workflow was the only consumer of e2eTestRunnerWithCache.php, applied-rule-removed-node-with-cache/ and timeout-file-not-cached/. They forced FileCacheStorage via cacheClass() to verify the persisted cache across two runs on CI; with the storage resolved internally that is no longer possible, so the workflow and its fixtures go away together. --- .../.gitignore | 1 - .../composer.json | 7 --- .../expected-output.diff | 40 --------------- .../rector.php | 20 -------- .../src/AlwaysTrue.php | 12 ----- .../src/DeadConstructor.php | 8 --- e2e/e2eTestRunnerWithCache.php | 49 ------------------- e2e/timeout-file-not-cached/.gitignore | 1 - e2e/timeout-file-not-cached/composer.json | 7 --- .../expected-output.diff | 7 --- e2e/timeout-file-not-cached/rector.php | 19 ------- .../src/SomeFixturePrinter.php | 9 ---- 12 files changed, 180 deletions(-) delete mode 100644 e2e/applied-rule-removed-node-with-cache/.gitignore delete mode 100644 e2e/applied-rule-removed-node-with-cache/composer.json delete mode 100644 e2e/applied-rule-removed-node-with-cache/expected-output.diff delete mode 100644 e2e/applied-rule-removed-node-with-cache/rector.php delete mode 100644 e2e/applied-rule-removed-node-with-cache/src/AlwaysTrue.php delete mode 100644 e2e/applied-rule-removed-node-with-cache/src/DeadConstructor.php delete mode 100644 e2e/e2eTestRunnerWithCache.php delete mode 100644 e2e/timeout-file-not-cached/.gitignore delete mode 100644 e2e/timeout-file-not-cached/composer.json delete mode 100644 e2e/timeout-file-not-cached/expected-output.diff delete mode 100644 e2e/timeout-file-not-cached/rector.php delete mode 100644 e2e/timeout-file-not-cached/src/SomeFixturePrinter.php diff --git a/e2e/applied-rule-removed-node-with-cache/.gitignore b/e2e/applied-rule-removed-node-with-cache/.gitignore deleted file mode 100644 index 61ead86667c..00000000000 --- a/e2e/applied-rule-removed-node-with-cache/.gitignore +++ /dev/null @@ -1 +0,0 @@ -/vendor diff --git a/e2e/applied-rule-removed-node-with-cache/composer.json b/e2e/applied-rule-removed-node-with-cache/composer.json deleted file mode 100644 index 5468cd74606..00000000000 --- a/e2e/applied-rule-removed-node-with-cache/composer.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "require": { - "php": "^8.1" - }, - "minimum-stability": "dev", - "prefer-stable": true -} diff --git a/e2e/applied-rule-removed-node-with-cache/expected-output.diff b/e2e/applied-rule-removed-node-with-cache/expected-output.diff deleted file mode 100644 index 94f3ee2fa9d..00000000000 --- a/e2e/applied-rule-removed-node-with-cache/expected-output.diff +++ /dev/null @@ -1,40 +0,0 @@ -2 files with changes -==================== - -1) src/AlwaysTrue.php:4 - - ---------- begin diff ---------- -@@ Line 4 @@ - { - public function run() - { -- if (1 === 1) { -- } -- - return 'no'; - } - } - ----------- end diff ----------- - -Applied rules: - * RemoveAlwaysTrueIfConditionRector - - -2) src/DeadConstructor.php:2 - - ---------- begin diff ---------- -@@ Line 2 @@ - - final class DeadConstructor - { -- public function __construct() -- { -- } - } - ----------- end diff ----------- - -Applied rules: - * RemoveEmptyClassMethodRector - - - [OK] 2 files would have been changed (dry-run) by Rector diff --git a/e2e/applied-rule-removed-node-with-cache/rector.php b/e2e/applied-rule-removed-node-with-cache/rector.php deleted file mode 100644 index 34222c564d1..00000000000 --- a/e2e/applied-rule-removed-node-with-cache/rector.php +++ /dev/null @@ -1,20 +0,0 @@ -cacheClass(FileCacheStorage::class); - - $rectorConfig->paths([ - __DIR__ . '/src', - ]); - - $rectorConfig->rule(RemoveEmptyClassMethodRector::class); - $rectorConfig->rule(RemoveAlwaysTrueIfConditionRector::class); -}; diff --git a/e2e/applied-rule-removed-node-with-cache/src/AlwaysTrue.php b/e2e/applied-rule-removed-node-with-cache/src/AlwaysTrue.php deleted file mode 100644 index 7b70b9e9ccd..00000000000 --- a/e2e/applied-rule-removed-node-with-cache/src/AlwaysTrue.php +++ /dev/null @@ -1,12 +0,0 @@ -create(); - -$matchedExpectedOutput = false; -$expectedOutput = trim((string) file_get_contents($expectedDiff)); -if ($output === $expectedOutput) { - $symfonyStyle->success('End-to-end test successfully completed'); - exit(Command::SUCCESS); -} - -// print color diff, to make easy find the differences -$defaultDiffer = new DefaultDiffer(); -$colorConsoleDiffFormatter = new ColorConsoleDiffFormatter(); -$diff = $colorConsoleDiffFormatter->format($defaultDiffer->diff($output, $expectedOutput)); -$symfonyStyle->writeln($diff); - -exit(Command::FAILURE); diff --git a/e2e/timeout-file-not-cached/.gitignore b/e2e/timeout-file-not-cached/.gitignore deleted file mode 100644 index 61ead86667c..00000000000 --- a/e2e/timeout-file-not-cached/.gitignore +++ /dev/null @@ -1 +0,0 @@ -/vendor diff --git a/e2e/timeout-file-not-cached/composer.json b/e2e/timeout-file-not-cached/composer.json deleted file mode 100644 index 5468cd74606..00000000000 --- a/e2e/timeout-file-not-cached/composer.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "require": { - "php": "^8.1" - }, - "minimum-stability": "dev", - "prefer-stable": true -} diff --git a/e2e/timeout-file-not-cached/expected-output.diff b/e2e/timeout-file-not-cached/expected-output.diff deleted file mode 100644 index 26cd1063c43..00000000000 --- a/e2e/timeout-file-not-cached/expected-output.diff +++ /dev/null @@ -1,7 +0,0 @@ -[ERROR] Could not process - "/home/runner/work/rector-src/rector-src/vendor/symplify/easy-parallel/ - src/ValueObject/ParallelProcess.php" file, due to: - "Child process timed out after 0 seconds". On line: 105 - - [ERROR] Could not process some files, due to: - "Reached system errors count limit of 50, exiting...". diff --git a/e2e/timeout-file-not-cached/rector.php b/e2e/timeout-file-not-cached/rector.php deleted file mode 100644 index 40356f0d51c..00000000000 --- a/e2e/timeout-file-not-cached/rector.php +++ /dev/null @@ -1,19 +0,0 @@ -cacheClass(FileCacheStorage::class); - $rectorConfig->parallel(0); - - $rectorConfig->paths([ - __DIR__ . '/src', - ]); - - $rectorConfig->sets([LevelSetList::UP_TO_PHP_82]); -}; diff --git a/e2e/timeout-file-not-cached/src/SomeFixturePrinter.php b/e2e/timeout-file-not-cached/src/SomeFixturePrinter.php deleted file mode 100644 index 69ed3a47227..00000000000 --- a/e2e/timeout-file-not-cached/src/SomeFixturePrinter.php +++ /dev/null @@ -1,9 +0,0 @@ -