Optimise checking the node types allowed for each rule (#6232)

* Optimise checking the node types allowed for each rule

* Add tests for `getVisitorsForNode()` function

* Make getVisitorsForNode function public

* Move initialisation of `visitorsPerNodeClass` property

* Use continue 2 instead of break

* Use patch from vendor-patches instead of local patch

Author: carlos-granados

composer.json

@@ -139,7 +139,8 @@
                 "https://raw.githubusercontent.com/rectorphp/vendor-patches/main/patches/nikic-php-parser-lib-phpparser-node-stmt-if-php.patch",
                 "https://raw.githubusercontent.com/rectorphp/vendor-patches/main/patches/nikic-php-parser-lib-phpparser-node-stmt-case-php.patch",
                 "https://raw.githubusercontent.com/rectorphp/vendor-patches/main/patches/nikic-php-parser-lib-phpparser-node-stmt-elseif-php.patch",
-                "https://raw.githubusercontent.com/rectorphp/vendor-patches/main/patches/nikic-php-parser-lib-phpparser-node-stmt-namespace-php.patch"
+                "https://raw.githubusercontent.com/rectorphp/vendor-patches/main/patches/nikic-php-parser-lib-phpparser-node-stmt-namespace-php.patch",
+                "https://raw.githubusercontent.com/rectorphp/vendor-patches/main/patches/nikic-php-parser-lib-phpparser-nodetraverser-php.patch"
             ]
         },
         "composer-exit-on-patch-failure": true,

src/PhpParser/NodeTraverser/RectorNodeTraverser.php

@@ -4,15 +4,22 @@
 
 namespace Rector\PhpParser\NodeTraverser;
 
+use PhpParser\Node;
 use PhpParser\Node\Stmt;
 use PhpParser\NodeTraverser;
+use PhpParser\NodeVisitor;
 use Rector\Contract\Rector\RectorInterface;
 use Rector\VersionBonding\PhpVersionedFilter;
 
 final class RectorNodeTraverser extends NodeTraverser
 {
     private bool $areNodeVisitorsPrepared = false;
 
+    /**
+     * @var array<class-string<Node>,RectorInterface[]>
+     */
+    private array $visitorsPerNodeClass = [];
+
     /**
      * @param RectorInterface[] $rectors
      */
@@ -42,10 +49,36 @@ public function refreshPhpRectors(array $rectors): void
     {
         $this->rectors = $rectors;
         $this->visitors = [];
+        $this->visitorsPerNodeClass = [];
 
         $this->areNodeVisitorsPrepared = false;
     }
 
+    /**
+     * We return the list of visitors (rector rules) that can be applied to each node class
+     * This list is cached so that we don't need to continually check if a rule can be applied to a node
+     *
+     * @return NodeVisitor[]
+     */
+    public function getVisitorsForNode(Node $node): array
+    {
+        $nodeClass = $node::class;
+        if (! isset($this->visitorsPerNodeClass[$nodeClass])) {
+            $this->visitorsPerNodeClass[$nodeClass] = [];
+            foreach ($this->visitors as $visitor) {
+                assert($visitor instanceof RectorInterface);
+                foreach ($visitor->getNodeTypes() as $nodeType) {
+                    if (is_a($nodeClass, $nodeType, true)) {
+                        $this->visitorsPerNodeClass[$nodeClass][] = $visitor;
+                        continue 2;
+                    }
+                }
+            }
+        }
+
+        return $this->visitorsPerNodeClass[$nodeClass];
+    }
+
     /**
      * This must happen after $this->configuration is set after ProcessCommand::execute() is run,
      * otherwise we get default false positives.

tests/PhpParser/NodeTraverser/ClassLike/RuleUsingClassLikeRector.php

@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\PhpParser\NodeTraverser\ClassLike;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\ClassLike;
+use Rector\Rector\AbstractRector;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+/**
+ * @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest
+ */
+final class RuleUsingClassLikeRector extends AbstractRector
+{
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('This rule applies to class like nodes', [new CodeSample('', '')]);
+    }
+
+    /**
+     * @return array<class-string<Node>>
+     */
+    public function getNodeTypes(): array
+    {
+        return [ClassLike::class];
+    }
+
+    public function refactor(Node $node): Node
+    {
+        return $node;
+    }
+}

tests/PhpParser/NodeTraverser/Class_/RuleUsingClassRector.php

@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\PhpParser\NodeTraverser\Class_;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Class_;
+use Rector\Rector\AbstractRector;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+/**
+ * @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest
+ */
+final class RuleUsingClassRector extends AbstractRector
+{
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('This rule applies to classes', [new CodeSample('', '')]);
+    }
+
+    /**
+     * @return array<class-string<Node>>
+     */
+    public function getNodeTypes(): array
+    {
+        return [Class_::class];
+    }
+
+    public function refactor(Node $node): Node
+    {
+        return $node;
+    }
+}

tests/PhpParser/NodeTraverser/Function_/RuleUsingFunctionRector.php

@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\PhpParser\NodeTraverser\Function_;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Function_;
+use Rector\Rector\AbstractRector;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+/**
+ * @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest
+ */
+final class RuleUsingFunctionRector extends AbstractRector
+{
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('This rule applies to functions', [new CodeSample('', '')]);
+    }
+
+    /**
+     * @return array<class-string<Node>>
+     */
+    public function getNodeTypes(): array
+    {
+        return [Function_::class];
+    }
+
+    public function refactor(Node $node): Node
+    {
+        return $node;
+    }
+}

tests/PhpParser/NodeTraverser/RectorNodeTraverserTest.php

@@ -0,0 +1,90 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\PhpParser\NodeTraverser;
+
+use PhpParser\Node\Stmt\Class_;
+use Rector\PhpParser\NodeTraverser\RectorNodeTraverser;
+use Rector\Testing\PHPUnit\AbstractLazyTestCase;
+use Rector\Tests\PhpParser\NodeTraverser\Class_\RuleUsingClassRector;
+use Rector\Tests\PhpParser\NodeTraverser\ClassLike\RuleUsingClassLikeRector;
+use Rector\Tests\PhpParser\NodeTraverser\Function_\RuleUsingFunctionRector;
+
+final class RectorNodeTraverserTest extends AbstractLazyTestCase
+{
+    private RectorNodeTraverser $rectorNodeTraverser;
+
+    private RuleUsingFunctionRector $ruleUsingFunctionRector;
+
+    private RuleUsingClassRector $ruleUsingClassRector;
+
+    private RuleUsingClassLikeRector $ruleUsingClassLikeRector;
+
+    protected function setUp(): void
+    {
+        $this->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->addVisitor($this->ruleUsingFunctionRector);
+
+        $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class);
+
+        $this->assertSame([], $visitors);
+    }
+
+    public function testGetVisitorsForNodeWhenSomeVisitorsMatch(): void
+    {
+        $class = new Class_('test');
+        $this->rectorNodeTraverser->addVisitor($this->ruleUsingFunctionRector);
+        $this->rectorNodeTraverser->addVisitor($this->ruleUsingClassRector);
+
+        $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class);
+
+        $this->assertEquals([$this->ruleUsingClassRector], $visitors);
+    }
+
+    public function testGetVisitorsForNodeWhenAllVisitorsMatch(): void
+    {
+        $class = new Class_('test');
+        $this->rectorNodeTraverser->addVisitor($this->ruleUsingClassRector);
+        $this->rectorNodeTraverser->addVisitor($this->ruleUsingClassLikeRector);
+
+        $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class);
+
+        $this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $visitors);
+    }
+
+    public function testGetVisitorsForNodeUsesCachedValue(): void
+    {
+        $class = new Class_('test');
+        $this->rectorNodeTraverser->addVisitor($this->ruleUsingClassRector);
+        $this->rectorNodeTraverser->addVisitor($this->ruleUsingClassLikeRector);
+
+        $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class);
+
+        $this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $visitors);
+
+        $this->rectorNodeTraverser->removeVisitor($this->ruleUsingClassRector);
+        $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class);
+
+        $this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $visitors);
+    }
+}