[experiment] Remove leaveNode() method on AbstractRector and CallableNodeVisitor (#7767)

Author: samsonasik

src/PhpDocParser/NodeVisitor/CallableNodeVisitor.php

@@ -18,13 +18,6 @@ final class CallableNodeVisitor extends NodeVisitorAbstract
      */
     private $callable;
 
-    private ?int $nodeIdToRemove = null;
-
-    /**
-     * @var array<int, Node[]>
-     */
-    private array $nodesToReturn = [];
-
     /**
      * @param callable(Node $node): (int|Node|null|Node[]) $callable
      */
@@ -33,48 +26,22 @@ public function __construct(callable $callable)
         $this->callable = $callable;
     }
 
-    public function enterNode(Node $node): int|Node|null
+    /**
+     * @return NodeVisitor::*|Node|null|Node[]
+     */
+    public function enterNode(Node $node): int|Node|null|array
     {
         $originalNode = $node;
 
         $callable = $this->callable;
 
-        /** @var int|Node|null|Node[] $newNode */
+        /** @var NodeVisitor::*|Node|null|Node[] $newNode */
         $newNode = $callable($node);
 
-        if ($newNode === NodeVisitor::REMOVE_NODE) {
-            $this->nodeIdToRemove = spl_object_id($originalNode);
-            return $originalNode;
-        }
-
-        if (is_array($newNode)) {
-            $nodeId = spl_object_id($node);
-            $this->nodesToReturn[$nodeId] = $newNode;
-
-            return $node;
-        }
-
         if ($originalNode instanceof Stmt && $newNode instanceof Expr) {
             return new Expression($newNode);
         }
 
         return $newNode;
     }
-
-    /**
-     * @return int|Node|Node[]
-     */
-    public function leaveNode(Node $node): int|Node|array
-    {
-        if ($this->nodeIdToRemove !== null && $this->nodeIdToRemove === spl_object_id($node)) {
-            $this->nodeIdToRemove = null;
-            return NodeVisitor::REMOVE_NODE;
-        }
-
-        if ($this->nodesToReturn === []) {
-            return $node;
-        }
-
-        return $this->nodesToReturn[spl_object_id($node)] ?? $node;
-    }
 }

src/Rector/AbstractRector.php

@@ -12,7 +12,6 @@
 use PhpParser\Node\Stmt\Interface_;
 use PhpParser\Node\Stmt\Property;
 use PhpParser\Node\Stmt\Trait_;
-use PhpParser\NodeTraverser;
 use PhpParser\NodeVisitor;
 use PhpParser\NodeVisitorAbstract;
 use PHPStan\Analyser\MutatingScope;
@@ -72,15 +71,8 @@ abstract class AbstractRector extends NodeVisitorAbstract implements RectorInter
 
     private CommentsMerger $commentsMerger;
 
-    /**
-     * @var array<int, Node[]>
-     */
-    private array $nodesToReturn = [];
-
     private CreatedByRuleDecorator $createdByRuleDecorator;
 
-    private ?int $toBeRemovedNodeId = null;
-
     public function autowire(
         NodeNameResolver $nodeNameResolver,
         NodeTypeResolver $nodeTypeResolver,
@@ -128,9 +120,9 @@ public function beforeTraverse(array $nodes): ?array
     }
 
     /**
-     * @return NodeTraverser::REMOVE_NODE|Node|null
+     * @return NodeVisitor::REMOVE_NODE|Node|null|Node[]
      */
-    final public function enterNode(Node $node): int|Node|null
+    final public function enterNode(Node $node): int|Node|null|array
     {
         if (is_a($this, HTMLAverseRectorInterface::class, true) && $this->file->containsHTML()) {
             return null;
@@ -167,47 +159,16 @@ final public function enterNode(Node $node): int|Node|null
                 return null;
             }
 
-            // log here, so we can remove the node in leaveNode() method
-            $this->toBeRemovedNodeId = spl_object_id($originalNode);
-
             // notify this rule changed code
             $rectorWithLineChange = new RectorWithLineChange(static::class, $originalNode->getStartLine());
             $this->file->addRectorClassWithLine($rectorWithLineChange);
 
-            // keep original node as node will be removed in leaveNode()
-            return $originalNode;
+            return $refactoredNodeOrState;
         }
 
         return $this->postRefactorProcess($originalNode, $node, $refactoredNodeOrState, $filePath);
     }
 
-    /**
-     * Replacing nodes in leaveNode() method avoids infinite recursion
-     * see"infinite recursion" in https://github.com/nikic/PHP-Parser/blob/master/doc/component/Walking_the_AST.markdown
-     *
-     * @return Node|Node[]|NodeVisitor::REMOVE_NODE|null
-     */
-    final public function leaveNode(Node $node): array|int|Node|null
-    {
-        if ($node->hasAttribute(AttributeKey::ORIGINAL_NODE)) {
-            return null;
-        }
-
-        // nothing to change here
-        if ($this->toBeRemovedNodeId === null && $this->nodesToReturn === []) {
-            return null;
-        }
-
-        $objectId = spl_object_id($node);
-        if ($this->toBeRemovedNodeId === $objectId) {
-            $this->toBeRemovedNodeId = null;
-
-            return NodeVisitor::REMOVE_NODE;
-        }
-
-        return $this->nodesToReturn[$objectId] ?? $node;
-    }
-
     protected function isName(Node $node, string $name): bool
     {
         return $this->nodeNameResolver->isName($node, $name);
@@ -270,13 +231,14 @@ protected function mirrorComments(Node $newNode, Node $oldNode): void
 
     /**
      * @param Node|Node[] $refactoredNode
+     * @return Node|Node[]
      */
     private function postRefactorProcess(
         Node $originalNode,
         Node $node,
         Node|array $refactoredNode,
         string $filePath
-    ): Node {
+    ): Node|array {
         /** @var non-empty-array<Node>|Node $refactoredNode */
         $this->createdByRuleDecorator->decorate($refactoredNode, $originalNode, static::class);
 
@@ -285,20 +247,8 @@ private function postRefactorProcess(
 
         /** @var MutatingScope|null $currentScope */
         $currentScope = $node->getAttribute(AttributeKey::SCOPE);
-
-        if (is_array($refactoredNode)) {
-            $this->refreshScopeNodes($refactoredNode, $filePath, $currentScope);
-
-            // search "infinite recursion" in https://github.com/nikic/PHP-Parser/blob/master/doc/component/Walking_the_AST.markdown
-            $originalNodeId = spl_object_id($originalNode);
-
-            // will be replaced in leaveNode() the original node must be passed
-            $this->nodesToReturn[$originalNodeId] = $refactoredNode;
-
-            return $originalNode;
-        }
-
         $this->refreshScopeNodes($refactoredNode, $filePath, $currentScope);
+
         return $refactoredNode;
     }