Optimise post rectors (#6240)

* Optimise NameImportingPostRector * Optimise UseAddingPostRector * Optimise ClassRenamingPostRector * Optimise UnusedImportRemovingPostRector * Add comment explaining the use of STOP_TRAVERSAL * Updates after PR review
rectorphp · Aug 22, 2024 · e3ad355 · e3ad355
1 parent be50454
commit e3ad355
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 31 deletions.
diff --git a/src/PostRector/Rector/ClassRenamingPostRector.php b/src/PostRector/Rector/ClassRenamingPostRector.php
@@ -23,6 +23,11 @@ final class ClassRenamingPostRector extends AbstractPostRector
 {
     private FileWithoutNamespace|Namespace_|null $rootNode = null;
 
+    /**
+     * @var array<string, string>
+     */
+    private array $oldToNewClasses = [];
+
     public function __construct(
         private readonly ClassRenamer $classRenamer,
         private readonly RenamedClassesDataCollector $renamedClassesDataCollector,
@@ -50,18 +55,13 @@ public function enterNode(Node $node): ?Node
             return null;
         }
 
-        $oldToNewClasses = $this->renamedClassesDataCollector->getOldToNewClasses();
-        if ($oldToNewClasses === []) {
-            return null;
-        }
-
         /** @var Scope|null $scope */
         $scope = $node->getAttribute(AttributeKey::SCOPE);
 
         if ($node instanceof FullyQualified) {
-            $result = $this->classRenamer->renameNode($node, $oldToNewClasses, $scope);
+            $result = $this->classRenamer->renameNode($node, $this->oldToNewClasses, $scope);
         } else {
-            $result = $this->resolveResultWithPhpAttributeName($node, $oldToNewClasses, $scope);
+            $result = $this->resolveResultWithPhpAttributeName($node, $scope);
         }
 
         if (! SimpleParameterProvider::provideBoolParameter(Option::AUTO_IMPORT_NAMES)) {
@@ -88,19 +88,20 @@ public function afterTraverse(array $nodes): array
         return $nodes;
     }
 
-    /**
-     * @param array<string, string> $oldToNewClasses
-     */
-    private function resolveResultWithPhpAttributeName(
-        Name $name,
-        array $oldToNewClasses,
-        ?Scope $scope
-    ): ?FullyQualified {
+    public function shouldTraverse(array $stmts): bool
+    {
+        $this->oldToNewClasses = $this->renamedClassesDataCollector->getOldToNewClasses();
+
+        return $this->oldToNewClasses !== [];
+    }
+
+    private function resolveResultWithPhpAttributeName(Name $name, ?Scope $scope): ?FullyQualified
+    {
         $phpAttributeName = $name->getAttribute(AttributeKey::PHP_ATTRIBUTE_NAME);
         if (is_string($phpAttributeName)) {
             return $this->classRenamer->renameNode(
                 new FullyQualified($phpAttributeName, $name->getAttributes()),
-                $oldToNewClasses,
+                $this->oldToNewClasses,
                 $scope
             );
         }

diff --git a/src/PostRector/Rector/NameImportingPostRector.php b/src/PostRector/Rector/NameImportingPostRector.php
@@ -20,6 +20,11 @@
 
 final class NameImportingPostRector extends AbstractPostRector
 {
+    /**
+     * @var array<Use_|GroupUse>
+     */
+    private array $currentUses = [];
+
     public function __construct(
         private readonly NameImporter $nameImporter,
         private readonly ClassNameImportSkipper $classNameImportSkipper,
@@ -29,6 +34,12 @@ public function __construct(
     ) {
     }
 
+    public function beforeTraverse(array $nodes)
+    {
+        $this->currentUses = $this->useImportsResolver->resolve();
+        return $nodes;
+    }
+
     public function enterNode(Node $node): Node|int|null
     {
         if (! $node instanceof FullyQualified) {
@@ -39,13 +50,12 @@ public function enterNode(Node $node): Node|int|null
             return null;
         }
 
-        $currentUses = $this->useImportsResolver->resolve();
-        if ($this->classNameImportSkipper->shouldSkipName($node, $currentUses)) {
+        if ($this->classNameImportSkipper->shouldSkipName($node, $this->currentUses)) {
             return null;
         }
 
         // make use of existing use import
-        $nameInUse = $this->resolveNameInUse($node, $currentUses);
+        $nameInUse = $this->resolveNameInUse($node);
         if ($nameInUse instanceof Name) {
             $nameInUse->setAttribute(AttributeKey::NAMESPACED_NAME, $node->toString());
             return $nameInUse;
@@ -62,12 +72,9 @@ public function shouldTraverse(array $stmts): bool
         return $this->addUseStatementGuard->shouldTraverse($stmts, $this->getFile()->getFilePath());
     }
 
-    /**
-     * @param array<Use_|GroupUse> $currentUses
-     */
-    private function resolveNameInUse(FullyQualified $fullyQualified, array $currentUses): null|Name
+    private function resolveNameInUse(FullyQualified $fullyQualified): null|Name
     {
-        $aliasName = $this->aliasNameResolver->resolveByName($fullyQualified, $currentUses);
+        $aliasName = $this->aliasNameResolver->resolveByName($fullyQualified, $this->currentUses);
         if (is_string($aliasName)) {
             return new Name($aliasName);
         }
@@ -77,7 +84,7 @@ private function resolveNameInUse(FullyQualified $fullyQualified, array $current
         }
 
         $lastName = $fullyQualified->getLast();
-        foreach ($currentUses as $currentUse) {
+        foreach ($this->currentUses as $currentUse) {
             foreach ($currentUse->uses as $useUse) {
                 if ($useUse->name->getLast() !== $lastName) {
                     continue;

diff --git a/src/PostRector/Rector/UnusedImportRemovingPostRector.php b/src/PostRector/Rector/UnusedImportRemovingPostRector.php
@@ -16,8 +16,6 @@
 use PhpParser\Node\Stmt\UseUse;
 use PhpParser\NodeTraverser;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
-use Rector\Configuration\Option;
-use Rector\Configuration\Parameter\SimpleParameterProvider;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
 use Rector\PhpParser\Node\CustomNode\FileWithoutNamespace;
@@ -36,10 +34,6 @@ public function enterNode(Node $node): ?Node
             return null;
         }
 
-        if (! SimpleParameterProvider::provideBoolParameter(Option::REMOVE_UNUSED_IMPORTS)) {
-            return null;
-        }
-
         $hasChanged = false;
 
         $namespaceName = $node instanceof Namespace_ && $node->name instanceof Name

diff --git a/src/PostRector/Rector/UseAddingPostRector.php b/src/PostRector/Rector/UseAddingPostRector.php
@@ -4,8 +4,10 @@
 
 namespace Rector\PostRector\Rector;
 
+use PhpParser\Node;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Namespace_;
+use PhpParser\NodeTraverser;
 use Rector\CodingStyle\Application\UseImportsAdder;
 use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory;
 use Rector\PhpParser\Node\CustomNode\FileWithoutNamespace;
@@ -68,6 +70,19 @@ public function beforeTraverse(array $nodes): array
         );
     }
 
+    public function enterNode(Node $node): int
+    {
+        /**
+         * We stop the traversal because all the work has already been done in the beforeTraverse() function
+         *
+         * Using STOP_TRAVERSAL is usually dangerous as it will stop the processing of all your nodes for all visitors
+         * but since the PostFileProcessor is using direct new NodeTraverser() and traverse() for only a single
+         * visitor per execution, using stop traversal here is safe,
+         * ref https://github.com/rectorphp/rector-src/blob/fc1e742fa4d9861ccdc5933f3b53613b8223438d/src/PostRector/Application/PostFileProcessor.php#L59-L61
+         */
+        return NodeTraverser::STOP_TRAVERSAL;
+    }
+
     /**
      * @param Stmt[] $nodes
      * @param FullyQualifiedObjectType[] $useImportTypes