diff --git a/src/Application/FileProcessor.php b/src/Application/FileProcessor.php index 00303a8fc42..1fc05375392 100644 --- a/src/Application/FileProcessor.php +++ b/src/Application/FileProcessor.php @@ -44,7 +44,6 @@ public function __construct( private RectorParser $rectorParser, private NodeScopeAndMetadataDecorator $nodeScopeAndMetadataDecorator, private UsedImportsResolver $usedImportsResolver, - private RectorRegistry $rectorRegistry, ) { } @@ -60,9 +59,6 @@ public function processFile(File $file, Configuration $configuration): FileProce $fileHasChanged = false; $filePath = $file->getFilePath(); - $currentRectors = $this->rectorRegistry->getRectors(); - $this->rectorNodeTraverser->refreshPhpRectors($currentRectors); - do { $file->changeHasChanged(false); diff --git a/src/Application/RectorRegistry.php b/src/Application/RectorRegistry.php index 0989c4892d7..d5dd8d8f0fb 100644 --- a/src/Application/RectorRegistry.php +++ b/src/Application/RectorRegistry.php @@ -4,37 +4,92 @@ namespace Rector\Application; +use PhpParser\Node; +use Rector\Configuration\ConfigurationRuleFilter; use Rector\Contract\Rector\RectorInterface; +use Rector\VersionBonding\ComposerPackageConstraintFilter; +use Rector\VersionBonding\PhpVersionedFilter; /** * @see \Rector\Tests\Application\RectorRegistryTest */ final class RectorRegistry { + private bool $areRectorsPrepared = false; + + /** + * @var RectorInterface[] + */ + private array $activeRectors = []; + + /** + * @var array, RectorInterface[]> + */ + private array $cache = []; + /** * @param RectorInterface[] $rectors */ public function __construct( + private readonly PhpVersionedFilter $phpVersionedFilter, + private readonly ComposerPackageConstraintFilter $composerPackageConstraintFilter, + private readonly ConfigurationRuleFilter $configurationRuleFilter, private array $rectors, ) { } /** * @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; + $this->areRectorsPrepared = false; + $this->cache = []; } /** - * @return array + * @return RectorInterface[] */ - public function getRectors(): array + public function forNode(Node $node): array { - return $this->rectors; + $this->prepareRectors(); + + $nodeClass = $node::class; + + if (! isset($this->cache[$nodeClass])) { + $this->cache[$nodeClass] = []; + + foreach ($this->activeRectors as $rector) { + foreach ($rector->getNodeTypes() as $nodeType) { + if (is_a($nodeClass, $nodeType, true)) { + $this->cache[$nodeClass][] = $rector; + continue 2; + } + } + } + } + + return $this->cache[$nodeClass]; + } + + /** + * Filtering must happen lazily, after the configuration is set in ProcessCommand::execute(), + * otherwise options like --only are not yet available and get ignored. + */ + private function prepareRectors(): void + { + if ($this->areRectorsPrepared) { + return; + } + + $activeRectors = $this->phpVersionedFilter->filter($this->rectors); + $activeRectors = $this->composerPackageConstraintFilter->filter($activeRectors); + + $this->activeRectors = $this->configurationRuleFilter->filter($activeRectors); + + $this->areRectorsPrepared = true; } } diff --git a/src/Configuration/ConfigurationRuleFilter.php b/src/Configuration/ConfigurationRuleFilter.php index 798a4fa188b..c8e993f2195 100644 --- a/src/Configuration/ConfigurationRuleFilter.php +++ b/src/Configuration/ConfigurationRuleFilter.php @@ -13,6 +13,8 @@ use Rector\VersionBonding\ValueObject\ComposerBoundRuleConfiguration; /** + * @todo merge to RectorRegistry, as it is only used there + * * Modify available rector rules based on the configuration options * @see \Rector\Tests\Configuration\ConfigurationRuleFilterTest */ diff --git a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php index b26aa8a54b7..a4f85d9bb7d 100644 --- a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php @@ -9,41 +9,16 @@ use PhpParser\Node\Expr; use PhpParser\Node\Stmt; use PhpParser\NodeVisitor; -use Rector\Configuration\ConfigurationRuleFilter; -use Rector\Contract\Rector\RectorInterface; +use Rector\Application\RectorRegistry; use Rector\Rector\RectorRunner; -use Rector\VersionBonding\ComposerPackageConstraintFilter; -use Rector\VersionBonding\PhpVersionedFilter; -use Webmozart\Assert\Assert; -/** - * @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest - */ final class RectorNodeTraverser { - /** - * @var RectorInterface[] - */ - private array $visitors = []; - private bool $stopTraversal; - private bool $areNodeVisitorsPrepared = false; - - /** - * @var array, RectorInterface[]> - */ - private array $visitorsPerNodeClass = []; - - /** - * @param RectorInterface[] $rectors - */ public function __construct( - private array $rectors, - private readonly PhpVersionedFilter $phpVersionedFilter, - private readonly ComposerPackageConstraintFilter $composerPackageConstraintFilter, - private readonly ConfigurationRuleFilter $configurationRuleFilter, private readonly RectorRunner $rectorRunner, + private readonly RectorRegistry $rectorRegistry, ) { } @@ -53,57 +28,11 @@ public function __construct( */ public function traverse(array $nodes): array { - $this->prepareNodeVisitors(); - $this->stopTraversal = false; return $this->traverseArray($nodes); } - /** - * @param RectorInterface[] $rectors - * - * @internal Used only in Rector core, not supported outside. Might change any time. - */ - public function refreshPhpRectors(array $rectors): void - { - Assert::allIsInstanceOf($rectors, RectorInterface::class); - - $this->rectors = $rectors; - $this->visitors = []; - $this->visitorsPerNodeClass = []; - - $this->areNodeVisitorsPrepared = false; - - $this->prepareNodeVisitors(); - } - - /** - * @return RectorInterface[] - * - * @api used in tests - */ - public function getVisitorsForNode(Node $node): array - { - $nodeClass = $node::class; - - if (! isset($this->visitorsPerNodeClass[$nodeClass])) { - $this->visitorsPerNodeClass[$nodeClass] = []; - - /** @var RectorInterface $visitor */ - foreach ($this->visitors as $visitor) { - foreach ($visitor->getNodeTypes() as $nodeType) { - if (is_a($nodeClass, $nodeType, true)) { - $this->visitorsPerNodeClass[$nodeClass][] = $visitor; - continue 2; - } - } - } - } - - return $this->visitorsPerNodeClass[$nodeClass]; - } - private function traverseNode(Node $node): void { foreach ($node->getSubNodeNames() as $name) { @@ -121,7 +50,7 @@ private function traverseNode(Node $node): void continue; } - $currentNodeVisitors = $this->getVisitorsForNode($subNode); + $currentNodeVisitors = $this->rectorRegistry->forNode($subNode); foreach ($currentNodeVisitors as $currentNodeVisitor) { $return = $this->rectorRunner->run($currentNodeVisitor, $subNode); @@ -168,8 +97,7 @@ private function traverseArray(array $nodes): array continue; } - $traverseChildren = true; - $currentNodeVisitors = $this->getVisitorsForNode($node); + $currentNodeVisitors = $this->rectorRegistry->forNode($node); foreach ($currentNodeVisitors as $currentNodeVisitor) { $return = $this->rectorRunner->run($currentNodeVisitor, $node); @@ -234,27 +162,4 @@ private function ensureReplacementReasonable(Node $old, Node $new): void ); } } - - /** - * This must happen after $this->configuration is set after ProcessCommand::execute() is run, otherwise we get default false positives. - * - * This should be removed after https://github.com/rectorphp/rector/issues/5584 is resolved - */ - private function prepareNodeVisitors(): void - { - if ($this->areNodeVisitorsPrepared) { - return; - } - - // filter out by PHP version - $this->visitors = $this->phpVersionedFilter->filter($this->rectors); - - // filter out by composer package constraint - $this->visitors = $this->composerPackageConstraintFilter->filter($this->visitors); - - // filter by configuration - $this->visitors = $this->configurationRuleFilter->filter($this->visitors); - - $this->areNodeVisitorsPrepared = true; - } } diff --git a/src/Testing/PHPUnit/AbstractRectorTestCase.php b/src/Testing/PHPUnit/AbstractRectorTestCase.php index 828d0528714..7036ee4a373 100644 --- a/src/Testing/PHPUnit/AbstractRectorTestCase.php +++ b/src/Testing/PHPUnit/AbstractRectorTestCase.php @@ -21,7 +21,6 @@ use Rector\Exception\ShouldNotHappenException; use Rector\NodeTypeResolver\DependencyInjection\PHPStanServicesFactory; use Rector\NodeTypeResolver\Reflection\BetterReflection\SourceLocatorProvider\DynamicSourceLocatorProvider; -use Rector\PhpParser\NodeTraverser\RectorNodeTraverser; use Rector\Testing\Contract\RectorTestInterface; use Rector\Testing\Fixture\FixtureFileFinder; use Rector\Testing\Fixture\FixtureFileUpdater; @@ -109,10 +108,6 @@ protected function setUp(): void // no rules at all, e.g. in case of only post rector run, yields an empty array $rectors = $rectorConfig->findByContract(RectorInterface::class); - /** @var RectorNodeTraverser $rectorNodeTraverser */ - $rectorNodeTraverser = $rectorConfig->make(RectorNodeTraverser::class); - $rectorNodeTraverser->refreshPhpRectors($rectors); - // keep the shared registry in sync with the current test's rules /** @var RectorRegistry $rectorRegistry */ $rectorRegistry = $rectorConfig->make(RectorRegistry::class); diff --git a/tests/Application/RectorRegistryTest.php b/tests/Application/RectorRegistryTest.php index 51b2628a64e..294afab94c2 100644 --- a/tests/Application/RectorRegistryTest.php +++ b/tests/Application/RectorRegistryTest.php @@ -4,38 +4,72 @@ namespace Rector\Tests\Application; +use PhpParser\Node\Stmt\Class_; use Rector\Application\RectorRegistry; -use Rector\Configuration\Option; -use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\Testing\PHPUnit\AbstractLazyTestCase; -use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement; +use Rector\Tests\PhpParser\NodeTraverser\Class_\RuleUsingClassRector; +use Rector\Tests\PhpParser\NodeTraverser\ClassLike\RuleUsingClassLikeRector; +use Rector\Tests\PhpParser\NodeTraverser\Function_\RuleUsingFunctionRector; final class RectorRegistryTest extends AbstractLazyTestCase { private RectorRegistry $rectorRegistry; + private RuleUsingFunctionRector $ruleUsingFunctionRector; + + private RuleUsingClassRector $ruleUsingClassRector; + + private RuleUsingClassLikeRector $ruleUsingClassLikeRector; + protected function setUp(): void { parent::setUp(); - SimpleParameterProvider::setParameter(Option::SKIP, [ - FifthElement::class => ['*/skipped_directory/*'], - ]); - $this->rectorRegistry = $this->make(RectorRegistry::class); - $this->rectorRegistry->refreshRectors([new FifthElement()]); + $this->rectorRegistry->refreshRectors([]); + + $this->ruleUsingFunctionRector = new RuleUsingFunctionRector(); + $this->ruleUsingClassRector = new RuleUsingClassRector(); + $this->ruleUsingClassLikeRector = new RuleUsingClassLikeRector(); + } + + public function testForNodeWhenNoRectorsAvailable(): void + { + $rectors = $this->rectorRegistry->forNode(new Class_('test')); + + $this->assertSame([], $rectors); } - protected function tearDown(): void + public function testForNodeWhenNoRectorsMatch(): void { - SimpleParameterProvider::setParameter(Option::SKIP, []); + $this->rectorRegistry->refreshRectors([$this->ruleUsingFunctionRector]); + + $rectors = $this->rectorRegistry->forNode(new Class_('test')); + + $this->assertSame([], $rectors); + } + + public function testForNodeWhenSomeRectorsMatch(): void + { + $this->rectorRegistry->refreshRectors([ + $this->ruleUsingFunctionRector, + $this->ruleUsingClassRector, + ]); + + $rectors = $this->rectorRegistry->forNode(new Class_('test')); + + $this->assertEquals([$this->ruleUsingClassRector], $rectors); } - public function testGetRectors(): void + public function testForNodeWhenAllRectorsMatch(): void { - $rectors = $this->rectorRegistry->getRectors(); + $this->rectorRegistry->refreshRectors([ + $this->ruleUsingClassRector, + $this->ruleUsingClassLikeRector, + ]); + + $rectors = $this->rectorRegistry->forNode(new Class_('test')); - $this->assertCount(1, $rectors); - $this->assertInstanceOf(FifthElement::class, $rectors[0]); + $this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $rectors); } } diff --git a/tests/PhpParser/NodeTraverser/ClassLike/RuleUsingClassLikeRector.php b/tests/PhpParser/NodeTraverser/ClassLike/RuleUsingClassLikeRector.php index 8764b077f45..6c65c69dd46 100644 --- a/tests/PhpParser/NodeTraverser/ClassLike/RuleUsingClassLikeRector.php +++ b/tests/PhpParser/NodeTraverser/ClassLike/RuleUsingClassLikeRector.php @@ -11,7 +11,7 @@ use Rector\RuleDoc\RuleDefinition; /** - * @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest + * @see \Rector\Tests\Application\RectorRegistryTest */ final class RuleUsingClassLikeRector extends AbstractRector { diff --git a/tests/PhpParser/NodeTraverser/Class_/RuleUsingClassRector.php b/tests/PhpParser/NodeTraverser/Class_/RuleUsingClassRector.php index 1e4c272b6e9..c6436d4c113 100644 --- a/tests/PhpParser/NodeTraverser/Class_/RuleUsingClassRector.php +++ b/tests/PhpParser/NodeTraverser/Class_/RuleUsingClassRector.php @@ -11,7 +11,7 @@ use Rector\RuleDoc\RuleDefinition; /** - * @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest + * @see \Rector\Tests\Application\RectorRegistryTest */ final class RuleUsingClassRector extends AbstractRector { diff --git a/tests/PhpParser/NodeTraverser/Function_/RuleUsingFunctionRector.php b/tests/PhpParser/NodeTraverser/Function_/RuleUsingFunctionRector.php index 602118a2206..c3e13a4d156 100644 --- a/tests/PhpParser/NodeTraverser/Function_/RuleUsingFunctionRector.php +++ b/tests/PhpParser/NodeTraverser/Function_/RuleUsingFunctionRector.php @@ -11,7 +11,7 @@ use Rector\RuleDoc\RuleDefinition; /** - * @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest + * @see \Rector\Tests\Application\RectorRegistryTest */ final class RuleUsingFunctionRector extends AbstractRector { diff --git a/tests/PhpParser/NodeTraverser/RectorNodeTraverserTest.php b/tests/PhpParser/NodeTraverser/RectorNodeTraverserTest.php deleted file mode 100644 index d7e9208ae7d..00000000000 --- a/tests/PhpParser/NodeTraverser/RectorNodeTraverserTest.php +++ /dev/null @@ -1,83 +0,0 @@ -rectorNodeTraverser = $this->make(RectorNodeTraverser::class); - $this->rectorNodeTraverser->refreshPhpRectors([]); - - $this->ruleUsingFunctionRector = new RuleUsingFunctionRector(); - $this->ruleUsingClassRector = new RuleUsingClassRector(); - $this->ruleUsingClassLikeRector = new RuleUsingClassLikeRector(); - } - - public function testGetVisitorsForNodeWhenNoVisitorsAvailable(): void - { - $class = new Class_('test'); - - $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class); - - $this->assertSame([], $visitors); - } - - public function testGetVisitorsForNodeWhenNoVisitorsMatch(): void - { - $class = new Class_('test'); - $this->rectorNodeTraverser->refreshPhpRectors([$this->ruleUsingFunctionRector]); - - $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class); - - $this->assertSame([], $visitors); - } - - public function testGetVisitorsForNodeWhenSomeVisitorsMatch(): void - { - $class = new Class_('test'); - $this->rectorNodeTraverser->refreshPhpRectors([ - $this->ruleUsingFunctionRector, - $this->ruleUsingClassRector, - ]); - - $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class); - - $this->assertEquals([$this->ruleUsingClassRector], $visitors); - } - - public function testGetVisitorsForNodeWhenAllVisitorsMatch(): void - { - $class = new Class_('test'); - $this->rectorNodeTraverser->refreshPhpRectors([ - $this->ruleUsingClassRector, - $this->ruleUsingClassLikeRector, - ]); - - $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class); - - $this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $visitors); - } -} diff --git a/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/StopTraverseOnTypeChangeTest.php b/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/StopTraverseOnTypeChangeTest.php index 53a63257e28..264eef77dd4 100644 --- a/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/StopTraverseOnTypeChangeTest.php +++ b/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/StopTraverseOnTypeChangeTest.php @@ -7,6 +7,7 @@ use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\Trait_; use PhpParser\NodeFinder; +use Rector\Application\RectorRegistry; use Rector\PhpParser\NodeTraverser\RectorNodeTraverser; use Rector\Testing\PHPUnit\AbstractLazyTestCase; use Rector\Testing\TestingParser\TestingParser; @@ -25,7 +26,8 @@ protected function setUp(): void $this->rectorNodeTraverser = $this->make(RectorNodeTraverser::class); - $this->rectorNodeTraverser->refreshPhpRectors([ + $rectorRegistry = $this->make(RectorRegistry::class); + $rectorRegistry->refreshRectors([ $this->make(RuleChangingClassToTraitRector::class), $this->make(RuleCheckingClassRector::class), ]);