From 8e82430b223dcd45febce375e5f3d5be384ca1d9 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 10 Oct 2026 15:56:44 +0200 Subject: [PATCH 1/6] [trav] move skipping from enterNode() to FileProcessor to run just once per file --- src/Application/FileProcessor.php | 5 ++ src/Application/RectorRegistry.php | 60 +++++++++++++++++++ .../PHPUnit/AbstractRectorTestCase.php | 7 +++ .../MissConfigurationReporterTest.php | 1 + .../Skipper/Fixture/Element/FifthElement.php | 19 +++++- .../Fixture/Element/NotSkippedClass.php | 26 ++++++++ tests/Skipper/Skipper/SkipperTest.php | 16 +++-- .../Skipper/Source/AnotherClassToSkip.php | 2 + .../Skipper/Source/NotSkippedClass.php | 9 --- .../Skipper/Skipper/UsedSkipCollectorTest.php | 1 - 10 files changed, 126 insertions(+), 20 deletions(-) create mode 100644 tests/Skipper/Skipper/Fixture/Element/NotSkippedClass.php delete mode 100644 tests/Skipper/Skipper/Source/NotSkippedClass.php diff --git a/src/Application/FileProcessor.php b/src/Application/FileProcessor.php index 1fc05375392..e9bd2c71644 100644 --- a/src/Application/FileProcessor.php +++ b/src/Application/FileProcessor.php @@ -44,6 +44,7 @@ public function __construct( private RectorParser $rectorParser, private NodeScopeAndMetadataDecorator $nodeScopeAndMetadataDecorator, private UsedImportsResolver $usedImportsResolver, + private RectorRegistry $rectorRegistry ) { } @@ -59,6 +60,10 @@ public function processFile(File $file, Configuration $configuration): FileProce $fileHasChanged = false; $filePath = $file->getFilePath(); + $this->rectorNodeTraverser->refreshPhpRectors( + $this->rectorRegistry->forPath($filePath) + ); + do { $file->changeHasChanged(false); diff --git a/src/Application/RectorRegistry.php b/src/Application/RectorRegistry.php index 4e2679f3b57..7d1ac7bc86c 100644 --- a/src/Application/RectorRegistry.php +++ b/src/Application/RectorRegistry.php @@ -6,10 +6,18 @@ use Rector\Contract\Rector\RectorInterface; use Rector\Skipper\Skipper\Skipper; +<<<<<<< HEAD +<<<<<<< HEAD /** * @see \Rector\Tests\Application\RectorRegistryTest */ +======= +use Rector\Skipper\ValueObject\SkipMatch; +======= +>>>>>>> 0ab0a04085 (udpate tests) + +>>>>>>> 484595fafb ([trav] move skipping from enterNode() to FileProcessor to run just once per file) final class RectorRegistry { /** @@ -17,6 +25,8 @@ final class RectorRegistry */ public function __construct( private array $rectors, +<<<<<<< HEAD +<<<<<<< HEAD private readonly Skipper $skipper ) { } @@ -45,9 +55,59 @@ public function forPath(string $filePath): array continue; } +======= + private Skipper $skipper +======= + private readonly Skipper $skipper +>>>>>>> 0ab0a04085 (udpate tests) + ) { + } + + /** + * @param RectorInterface[] $rectors + * @api used in tests to update the active rules + * + * @internal Used only in Rector core, not supported outside. Might change any time. + */ + public function refreshRectors(array $rectors): void + { + $this->rectors = $rectors; + } + + /** + * @return array + */ + public function forPath(string $filePath): array + { + $rectorsForPath = []; + foreach ($this->rectors as $rector) { + if ($this->skipper->shouldSkipRectorAndFile($rector, $filePath)) { + continue; + } + +<<<<<<< HEAD + // $skipMatch = $this->skipper->matchSkip($rector, $filePath); + // if ($skipMatch instanceof SkipMatch) { + // if ($rector->refactor($this->cloneNode($node)) !== null) { + // $this->skipper->markSkipUsed($skipMatch); + // } + + // return null; + // } + +>>>>>>> 484595fafb ([trav] move skipping from enterNode() to FileProcessor to run just once per file) +======= +>>>>>>> 0ab0a04085 (udpate tests) $rectorsForPath[] = $rector; } return $rectorsForPath; +<<<<<<< HEAD +<<<<<<< HEAD +======= + +>>>>>>> 484595fafb ([trav] move skipping from enterNode() to FileProcessor to run just once per file) +======= +>>>>>>> 0ab0a04085 (udpate tests) } } diff --git a/src/Testing/PHPUnit/AbstractRectorTestCase.php b/src/Testing/PHPUnit/AbstractRectorTestCase.php index 65917eb0415..37cbecc50be 100644 --- a/src/Testing/PHPUnit/AbstractRectorTestCase.php +++ b/src/Testing/PHPUnit/AbstractRectorTestCase.php @@ -9,6 +9,7 @@ use Nette\Utils\Strings; use PHPUnit\Framework\ExpectationFailedException; use Rector\Application\ApplicationFileProcessor; +use Rector\Application\RectorRegistry; use Rector\Autoloading\AdditionalAutoloader; use Rector\Autoloading\BootstrapFilesIncluder; use Rector\Composer\InstalledPackageResolver; @@ -112,6 +113,12 @@ protected function setUp(): void $rectorNodeTraverser = $rectorConfig->make(RectorNodeTraverser::class); $rectorNodeTraverser->refreshPhpRectors($rectors); + // the file processor re-pushes the registry rules onto the traverser per file, + // so the shared registry must carry the current test's rules too + /** @var RectorRegistry $rectorRegistry */ + $rectorRegistry = $rectorConfig->make(RectorRegistry::class); + $rectorRegistry->refreshRectors($rectors); + // store cache self::$cacheByRuleAndConfig[$cacheKey] = true; } diff --git a/tests/Reporting/MissConfigurationReporterTest.php b/tests/Reporting/MissConfigurationReporterTest.php index e05cb978d77..d7487ef5a9a 100644 --- a/tests/Reporting/MissConfigurationReporterTest.php +++ b/tests/Reporting/MissConfigurationReporterTest.php @@ -81,6 +81,7 @@ public function testReportsSkippedNonRectorClass(): void $output = $this->bufferedOutput->fetch(); $this->assertStringContainsString('AnotherClassToSkip', $output); + $this->assertStringNotContainsString('OrdSingleByteRector', $output); $this->assertStringNotContainsString('NameImportingPostRector', $output); } diff --git a/tests/Skipper/Skipper/Fixture/Element/FifthElement.php b/tests/Skipper/Skipper/Fixture/Element/FifthElement.php index 2b49be13b67..0835815f472 100644 --- a/tests/Skipper/Skipper/Fixture/Element/FifthElement.php +++ b/tests/Skipper/Skipper/Fixture/Element/FifthElement.php @@ -4,6 +4,23 @@ namespace Rector\Tests\Skipper\Skipper\Fixture\Element; -final class FifthElement +use PhpParser\Node; +use Rector\RuleDoc\RuleDefinition; + +final class FifthElement implements \Rector\Contract\Rector\RectorInterface { + public function getRuleDefinition(): RuleDefinition + { + // TODO: Implement getRuleDefinition() method. + } + + public function getNodeTypes(): array + { + // TODO: Implement getNodeTypes() method. + } + + public function refactor(Node $node) + { + // TODO: Implement refactor() method. + } } diff --git a/tests/Skipper/Skipper/Fixture/Element/NotSkippedClass.php b/tests/Skipper/Skipper/Fixture/Element/NotSkippedClass.php new file mode 100644 index 00000000000..80d961fa0c7 --- /dev/null +++ b/tests/Skipper/Skipper/Fixture/Element/NotSkippedClass.php @@ -0,0 +1,26 @@ +skipper->shouldSkipRectorAndFile($rector, $filePath); $this->assertSame($expectedSkip, $resolvedSkip); } - /** - * @return Iterator, mixed>> - */ - public static function provideCheckerAndFile(): Iterator + public static function provideRectorAndFile(): Iterator { yield [new FifthElement(), __DIR__ . '/Fixture', true]; - yield [new AnotherClassToSkip(), __DIR__ . '/Fixture/someFile', true]; - yield [new AnotherClassToSkip(), __DIR__ . '/Fixture/someDirectory/anotherFile.php', true]; + yield [new FifthElement(), __DIR__ . '/Fixture/someFile', true]; + yield [new FifthElement(), __DIR__ . '/Fixture/someDirectory/anotherFile.php', true]; yield [new NotSkippedClass(), __DIR__ . '/Fixture/someFile', false]; yield [new NotSkippedClass(), __DIR__ . '/Fixture/someOtherFile', false]; diff --git a/tests/Skipper/Skipper/Source/AnotherClassToSkip.php b/tests/Skipper/Skipper/Source/AnotherClassToSkip.php index 150276ff3f3..221bebc7c1c 100644 --- a/tests/Skipper/Skipper/Source/AnotherClassToSkip.php +++ b/tests/Skipper/Skipper/Source/AnotherClassToSkip.php @@ -4,6 +4,8 @@ namespace Rector\Tests\Skipper\Skipper\Source; +use PhpParser\Node; + final class AnotherClassToSkip { } diff --git a/tests/Skipper/Skipper/Source/NotSkippedClass.php b/tests/Skipper/Skipper/Source/NotSkippedClass.php deleted file mode 100644 index a3d21f44597..00000000000 --- a/tests/Skipper/Skipper/Source/NotSkippedClass.php +++ /dev/null @@ -1,9 +0,0 @@ -skipper->shouldSkipFilePath('tests/Skipper/Skipper/Fixture/SomeSkippedPath/any.txt'); - $this->skipper->shouldSkipRectorAndFile(new FifthElement(), __FILE__); $usedSkips = $this->usedSkipCollector->provide(); From a4efc0467e8b3101b05e93f7f9335d5e84d9e63b Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 10 Oct 2026 18:29:38 +0200 Subject: [PATCH 2/6] trigger element skip in UsedSkipCollectorTest --- tests/Skipper/Skipper/UsedSkipCollectorTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Skipper/Skipper/UsedSkipCollectorTest.php b/tests/Skipper/Skipper/UsedSkipCollectorTest.php index 954a98d691b..a64611a9ce3 100644 --- a/tests/Skipper/Skipper/UsedSkipCollectorTest.php +++ b/tests/Skipper/Skipper/UsedSkipCollectorTest.php @@ -50,6 +50,7 @@ protected function tearDown(): void public function testCollectsOnlyMatchedSkips(): void { $this->skipper->shouldSkipFilePath('tests/Skipper/Skipper/Fixture/SomeSkippedPath/any.txt'); + $this->skipper->shouldSkipRectorAndFile(new FifthElement(), __DIR__ . '/Fixture/someFile'); $usedSkips = $this->usedSkipCollector->provide(); From 4aca9d8d69cc2cf00e491f921cf272ed5a9bdb55 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 10 Oct 2026 18:42:48 +0200 Subject: [PATCH 3/6] update SkipUsedTrackingTest to file-level skip semantics --- .../FixtureSkipUsedTracking/skip_unused_no_old_class.php | 7 +++++++ .../Name/RenameClassRector/SkipUsedTrackingTest.php | 5 +++-- src/PhpParser/NodeTraverser/RectorNodeTraverser.php | 8 -------- 3 files changed, 10 insertions(+), 10 deletions(-) create mode 100644 rules-tests/Renaming/Rector/Name/RenameClassRector/FixtureSkipUsedTracking/skip_unused_no_old_class.php diff --git a/rules-tests/Renaming/Rector/Name/RenameClassRector/FixtureSkipUsedTracking/skip_unused_no_old_class.php b/rules-tests/Renaming/Rector/Name/RenameClassRector/FixtureSkipUsedTracking/skip_unused_no_old_class.php new file mode 100644 index 00000000000..ec4ed767c3e --- /dev/null +++ b/rules-tests/Renaming/Rector/Name/RenameClassRector/FixtureSkipUsedTracking/skip_unused_no_old_class.php @@ -0,0 +1,7 @@ +assertContains('*skip_used_renames_old_class*', $usedPaths); // the skip on a file the rule would not have touched is never marked used diff --git a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php index 417f012c4ae..a076c16c92c 100644 --- a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php @@ -17,15 +17,7 @@ use Webmozart\Assert\Assert; /** - * Based on native NodeTraverser class, but heavily customized for Rector needs. - * - * The main differences are: - * - no leaveNode(), the RectorRunner calls each rule's refactor() method on enter - * - cached visitors per node class for performance, e.g. when we find rules for Class_ node, they're cached for next time - * - immutability features, register Rector rules once, then use; no changes on the fly - * * @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest - * @internal No BC promise on this class, it might change any time. */ final class RectorNodeTraverser { From a1314e7f938920957009ad8210a290683bf07e10 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 10 Oct 2026 19:42:24 +0200 Subject: [PATCH 4/6] decouple RectorRegistry from FileProcessor, cover forPath with test; restore NotSkippedClass to Source --- .../skip_unused_no_old_class.php | 7 --- src/Application/FileProcessor.php | 5 -- src/Application/RectorRegistry.php | 62 ------------------- .../PHPUnit/AbstractRectorTestCase.php | 3 +- tests/Application/RectorRegistryTest.php | 5 +- .../Fixture/Element/NotSkippedClass.php | 26 -------- tests/Skipper/Skipper/SkipperTest.php | 5 +- .../Skipper/Source/NotSkippedClass.php | 9 +++ 8 files changed, 16 insertions(+), 106 deletions(-) delete mode 100644 rules-tests/Renaming/Rector/Name/RenameClassRector/FixtureSkipUsedTracking/skip_unused_no_old_class.php delete mode 100644 tests/Skipper/Skipper/Fixture/Element/NotSkippedClass.php create mode 100644 tests/Skipper/Skipper/Source/NotSkippedClass.php diff --git a/rules-tests/Renaming/Rector/Name/RenameClassRector/FixtureSkipUsedTracking/skip_unused_no_old_class.php b/rules-tests/Renaming/Rector/Name/RenameClassRector/FixtureSkipUsedTracking/skip_unused_no_old_class.php deleted file mode 100644 index ec4ed767c3e..00000000000 --- a/rules-tests/Renaming/Rector/Name/RenameClassRector/FixtureSkipUsedTracking/skip_unused_no_old_class.php +++ /dev/null @@ -1,7 +0,0 @@ -getFilePath(); - $this->rectorNodeTraverser->refreshPhpRectors( - $this->rectorRegistry->forPath($filePath) - ); - do { $file->changeHasChanged(false); diff --git a/src/Application/RectorRegistry.php b/src/Application/RectorRegistry.php index 7d1ac7bc86c..691f8c03b8f 100644 --- a/src/Application/RectorRegistry.php +++ b/src/Application/RectorRegistry.php @@ -6,18 +6,10 @@ use Rector\Contract\Rector\RectorInterface; use Rector\Skipper\Skipper\Skipper; -<<<<<<< HEAD -<<<<<<< HEAD /** * @see \Rector\Tests\Application\RectorRegistryTest */ -======= -use Rector\Skipper\ValueObject\SkipMatch; -======= ->>>>>>> 0ab0a04085 (udpate tests) - ->>>>>>> 484595fafb ([trav] move skipping from enterNode() to FileProcessor to run just once per file) final class RectorRegistry { /** @@ -25,41 +17,7 @@ final class RectorRegistry */ public function __construct( private array $rectors, -<<<<<<< HEAD -<<<<<<< HEAD - private readonly Skipper $skipper - ) { - } - - /** - * @param RectorInterface[] $rectors - * @api used in tests to update the active rules - * - * @internal Used only in Rector core, not supported outside. Might change any time. - */ - public function refreshRectors(array $rectors): void - { - $this->rectors = $rectors; - } - - /** - * @return RectorInterface[] - * - * @api used in tests - */ - public function forPath(string $filePath): array - { - $rectorsForPath = []; - foreach ($this->rectors as $rector) { - if ($this->skipper->shouldSkipRectorAndFile($rector, $filePath)) { - continue; - } - -======= - private Skipper $skipper -======= private readonly Skipper $skipper ->>>>>>> 0ab0a04085 (udpate tests) ) { } @@ -85,29 +43,9 @@ public function forPath(string $filePath): array continue; } -<<<<<<< HEAD - // $skipMatch = $this->skipper->matchSkip($rector, $filePath); - // if ($skipMatch instanceof SkipMatch) { - // if ($rector->refactor($this->cloneNode($node)) !== null) { - // $this->skipper->markSkipUsed($skipMatch); - // } - - // return null; - // } - ->>>>>>> 484595fafb ([trav] move skipping from enterNode() to FileProcessor to run just once per file) -======= ->>>>>>> 0ab0a04085 (udpate tests) $rectorsForPath[] = $rector; } return $rectorsForPath; -<<<<<<< HEAD -<<<<<<< HEAD -======= - ->>>>>>> 484595fafb ([trav] move skipping from enterNode() to FileProcessor to run just once per file) -======= ->>>>>>> 0ab0a04085 (udpate tests) } } diff --git a/src/Testing/PHPUnit/AbstractRectorTestCase.php b/src/Testing/PHPUnit/AbstractRectorTestCase.php index 37cbecc50be..828d0528714 100644 --- a/src/Testing/PHPUnit/AbstractRectorTestCase.php +++ b/src/Testing/PHPUnit/AbstractRectorTestCase.php @@ -113,8 +113,7 @@ protected function setUp(): void $rectorNodeTraverser = $rectorConfig->make(RectorNodeTraverser::class); $rectorNodeTraverser->refreshPhpRectors($rectors); - // the file processor re-pushes the registry rules onto the traverser per file, - // so the shared registry must carry the current test's rules too + // keep the shared registry in sync with the current test's rules /** @var RectorRegistry $rectorRegistry */ $rectorRegistry = $rectorConfig->make(RectorRegistry::class); $rectorRegistry->refreshRectors($rectors); diff --git a/tests/Application/RectorRegistryTest.php b/tests/Application/RectorRegistryTest.php index f20fbd0acff..b3516f9b904 100644 --- a/tests/Application/RectorRegistryTest.php +++ b/tests/Application/RectorRegistryTest.php @@ -9,6 +9,7 @@ use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\Testing\PHPUnit\AbstractLazyTestCase; use Rector\Tests\Application\Source\SkippableRector; +use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement; final class RectorRegistryTest extends AbstractLazyTestCase { @@ -20,10 +21,11 @@ protected function setUp(): void SimpleParameterProvider::setParameter(Option::SKIP, [ SkippableRector::class => ['*/skipped_directory/*'], + FifthElement::class => ['*/skipped_directory/*'], ]); $this->rectorRegistry = $this->make(RectorRegistry::class); - $this->rectorRegistry->refreshRectors([new SkippableRector()]); + $this->rectorRegistry->refreshRectors([new FifthElement()]); } protected function tearDown(): void @@ -44,5 +46,6 @@ public function testForPathKeepsRectorForOtherPath(): void $this->assertCount(1, $rectorsForPath); $this->assertInstanceOf(SkippableRector::class, $rectorsForPath[0]); + $this->assertInstanceOf(FifthElement::class, $rectorsForPath[0]); } } diff --git a/tests/Skipper/Skipper/Fixture/Element/NotSkippedClass.php b/tests/Skipper/Skipper/Fixture/Element/NotSkippedClass.php deleted file mode 100644 index 80d961fa0c7..00000000000 --- a/tests/Skipper/Skipper/Fixture/Element/NotSkippedClass.php +++ /dev/null @@ -1,26 +0,0 @@ -skipper->shouldSkipRectorAndFile($rector, $filePath); $this->assertSame($expectedSkip, $resolvedSkip); diff --git a/tests/Skipper/Skipper/Source/NotSkippedClass.php b/tests/Skipper/Skipper/Source/NotSkippedClass.php new file mode 100644 index 00000000000..a3d21f44597 --- /dev/null +++ b/tests/Skipper/Skipper/Source/NotSkippedClass.php @@ -0,0 +1,9 @@ + Date: Sat, 10 Oct 2026 22:11:36 +0200 Subject: [PATCH 5/6] turn AnotherClassToSkip into a Rector rule used in SkipperTest; keep FifthElement-based RectorRegistryTest --- tests/Application/RectorRegistryTest.php | 3 --- tests/Application/Source/SkippableRector.php | 27 ------------------- tests/Skipper/Skipper/SkipperTest.php | 3 +++ .../Skipper/Source/AnotherClassToSkip.php | 18 ++++++++++++- 4 files changed, 20 insertions(+), 31 deletions(-) delete mode 100644 tests/Application/Source/SkippableRector.php diff --git a/tests/Application/RectorRegistryTest.php b/tests/Application/RectorRegistryTest.php index b3516f9b904..bff14754b54 100644 --- a/tests/Application/RectorRegistryTest.php +++ b/tests/Application/RectorRegistryTest.php @@ -8,7 +8,6 @@ use Rector\Configuration\Option; use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\Testing\PHPUnit\AbstractLazyTestCase; -use Rector\Tests\Application\Source\SkippableRector; use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement; final class RectorRegistryTest extends AbstractLazyTestCase @@ -20,7 +19,6 @@ protected function setUp(): void parent::setUp(); SimpleParameterProvider::setParameter(Option::SKIP, [ - SkippableRector::class => ['*/skipped_directory/*'], FifthElement::class => ['*/skipped_directory/*'], ]); @@ -45,7 +43,6 @@ public function testForPathKeepsRectorForOtherPath(): void $rectorsForPath = $this->rectorRegistry->forPath(__DIR__ . '/some_file.php'); $this->assertCount(1, $rectorsForPath); - $this->assertInstanceOf(SkippableRector::class, $rectorsForPath[0]); $this->assertInstanceOf(FifthElement::class, $rectorsForPath[0]); } } diff --git a/tests/Application/Source/SkippableRector.php b/tests/Application/Source/SkippableRector.php deleted file mode 100644 index d7a63644f10..00000000000 --- a/tests/Application/Source/SkippableRector.php +++ /dev/null @@ -1,27 +0,0 @@ - Date: Sat, 10 Oct 2026 22:14:18 +0200 Subject: [PATCH 6/6] use NotSkippedClass as the non-rector example in MissConfigurationReporterTest --- tests/Reporting/MissConfigurationReporterTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Reporting/MissConfigurationReporterTest.php b/tests/Reporting/MissConfigurationReporterTest.php index d7487ef5a9a..50c1a8cdcc5 100644 --- a/tests/Reporting/MissConfigurationReporterTest.php +++ b/tests/Reporting/MissConfigurationReporterTest.php @@ -15,6 +15,7 @@ use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement; use Rector\Tests\Skipper\Skipper\Fixture\Element\ThreeMan; use Rector\Tests\Skipper\Skipper\Source\AnotherClassToSkip; +use Rector\Tests\Skipper\Skipper\Source\NotSkippedClass; use Rector\Validation\RectorConfigValidator; use Rector\ValueObject\ProcessResult; use Symfony\Component\Console\Input\ArrayInput; @@ -69,7 +70,7 @@ public function testReportsSkippedNonRectorClass(): void { RectorConfigValidator::ensureRectorRulesExist([ // not a Rector rule, can never be skipped - AnotherClassToSkip::class => ['some/path'], + NotSkippedClass::class => ['some/path'], // Rector rule, must not be reported OrdSingleByteRector::class => ['some/path'], // post rector rule, must not be reported @@ -80,7 +81,7 @@ public function testReportsSkippedNonRectorClass(): void $output = $this->bufferedOutput->fetch(); - $this->assertStringContainsString('AnotherClassToSkip', $output); + $this->assertStringContainsString('NotSkippedClass', $output); $this->assertStringNotContainsString('OrdSingleByteRector', $output); $this->assertStringNotContainsString('NameImportingPostRector', $output);