From 47ddf72a25fcedfbbef63f517a438e00d131cfd1 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 ee05a0314ab..24416ba0885 100644 --- a/src/Configuration/Option.php +++ b/src/Configuration/Option.php @@ -138,8 +138,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 c3d114e7687..af56c3eedbd 100644 --- a/src/Configuration/RectorConfigBuilder.php +++ b/src/Configuration/RectorConfigBuilder.php @@ -296,7 +296,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) { @@ -806,6 +807,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 9a404d7a4f556a8822b76ce6d5724848e78d97e0 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Thu, 6 Aug 2026 15:33:44 +0000 Subject: [PATCH 2/3] [ci-review] Rector Rectify --- bin/rector.php | 1 + tests/Bin/RectorTest.php | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/bin/rector.php b/bin/rector.php index ef8161e1ffa..532970222d8 100755 --- a/bin/rector.php +++ b/bin/rector.php @@ -147,6 +147,7 @@ public function loadIfExistsAndNotLoadedYet(string $filePath): void do { $errors[] = $throwable->getMessage(); } while ($throwable = $throwable->getPrevious()); + echo Json::encode([ 'fatal_errors' => $errors, ]); diff --git a/tests/Bin/RectorTest.php b/tests/Bin/RectorTest.php index 11f37ff522e..bd9e85de090 100644 --- a/tests/Bin/RectorTest.php +++ b/tests/Bin/RectorTest.php @@ -17,15 +17,15 @@ final class RectorTest extends TestCase public static function outputProvider(): Iterator { yield 'Version' => [ - 'command' => PHP_BINARY . ' bin/rector --version', - 'expectedOutput' => "Rector @package_version@" . PHP_EOL, + 'command' => PHP_BINARY . ' bin/rector --version', + 'expectedOutput' => 'Rector @package_version@' . PHP_EOL, ]; yield 'Exception with previous console output' => [ - 'command' => PHP_BINARY . ' bin/rector -c tests/Bin/config/incorrect-phpstan-files.php', - 'expectedOutput' => PHP_EOL . " [ERROR] Rector\\NodeTypeResolver\\DependencyInjection\\PHPStanServicesFactory " . PHP_EOL . PHP_EOL . " [ERROR] Unexpected item 'parameters › invalidParameters'. " . PHP_EOL . PHP_EOL, + 'command' => PHP_BINARY . ' bin/rector -c tests/Bin/config/incorrect-phpstan-files.php', + 'expectedOutput' => PHP_EOL . ' [ERROR] Rector\\NodeTypeResolver\\DependencyInjection\\PHPStanServicesFactory ' . PHP_EOL . PHP_EOL . " [ERROR] Unexpected item 'parameters › invalidParameters'. " . PHP_EOL . PHP_EOL, ]; yield 'Exception with previous console output in JSON format' => [ - 'command' => PHP_BINARY . ' bin/rector -c tests/Bin/config/incorrect-phpstan-files.php --output-format json', + 'command' => PHP_BINARY . ' bin/rector -c tests/Bin/config/incorrect-phpstan-files.php --output-format json', 'expectedOutput' => '{"fatal_errors":["Rector\\\\NodeTypeResolver\\\\DependencyInjection\\\\PHPStanServicesFactory","Unexpected item \'parameters › invalidParameters\'."]}', ]; } @@ -35,6 +35,6 @@ public function testConsoleOutput(string $command, string $expectedOutput): void { $process = Process::fromShellCommandline($command); $process->run(); - $this->assertSame($expectedOutput, preg_replace("/ +/", " ", $process->getOutput())); + $this->assertSame($expectedOutput, preg_replace('/ +/', ' ', $process->getOutput())); } } From b9b3b51398c1a4b0f92322a66a6b341b497b86d5 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 6 Aug 2026 23:23:43 +0200 Subject: [PATCH 3/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 f969fdc3fff..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@v4 - - - 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 04115d03e40..339216bf51d 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 24416ba0885..cdd77d5e69b 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'; @@ -137,15 +134,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 af56c3eedbd..34c5b3dc396 100644 --- a/src/Configuration/RectorConfigBuilder.php +++ b/src/Configuration/RectorConfigBuilder.php @@ -93,11 +93,6 @@ final class RectorConfigBuilder */ private array $fileExtensions = []; - /** - * @var null|class-string - */ - private ?string $cacheClass = null; - private ?string $cacheDirectory = null; private ?string $containerCacheDirectory = null; @@ -295,11 +290,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); } @@ -807,8 +797,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 */ @@ -818,7 +808,6 @@ public function withCache( ?string $containerCacheDirectory = null ): self { $this->cacheDirectory = $cacheDirectory; - $this->cacheClass = $cacheClass; $this->containerCacheDirectory = $containerCacheDirectory; return $this;