Add REMOVE_NODE support to refactor() direct call (#4073)

Co-authored-by: GitHub Action <actions@github.com>

packages/PostRector/Rector/NodeRemovingPostRector.php

@@ -15,6 +15,9 @@
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
+/**
+ * @deprecated Remove node directly instead in refactor() method
+ */
 final class NodeRemovingPostRector extends AbstractPostRector
 {
     public function __construct(

phpstan.neon

@@ -240,9 +240,6 @@ parameters:
         # skipped on purpose, as ctor overrie
         - '#Rector\\StaticTypeMapper\\ValueObject\\Type\\SimpleStaticType\:\:__construct\(\) does not call parent constructor from PHPStan\\Type\\StaticType#'
 
-        # complex detection
-        - '#Cognitive complexity for "Rector\\Core\\DependencyInjection\\Collector\\ConfigureCallValuesCollector\:\:addConfigureCallValues\(\)" is \d+, keep it under 11#'
-
         # return bool on change
         - '#Method "(change|remove)(.*?)" returns bool type, so the name should start with is/has/was#'
 
@@ -737,3 +734,5 @@ parameters:
         - '#Cognitive complexity for "Rector\\CodeQuality\\Rector\\Class_\\InlineConstructorDefaultToPropertyRector\:\:refactor\(\)" is 20, keep it under 11#'
         - '#Cognitive complexity for "Rector\\DeadCode\\Rector\\For_\\RemoveDeadIfForeachForRector\:\:refactor\(\)" is 16, keep it under 11#'
         - '#Cognitive complexity for "Rector\\DeadCode\\Rector\\MethodCall\\RemoveEmptyMethodCallRector\:\:refactor\(\)" is 18, keep it under 11#'
+
+        - '#Return type (.*?) should be covariant with return type \(1\|2\|3\|4\|array<PhpParser\\Node>\|PhpParser\\Node\|null\) of method Rector\\Core\\Contract\\Rector\\PhpRectorInterface\:\:refactor\(\)#'

rules/DeadCode/Rector/If_/RemoveDeadInstanceOfRector.php

@@ -18,6 +18,7 @@
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\If_;
 use PhpParser\Node\Stmt\Property;
+use PhpParser\NodeTraverser;
 use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
 use Rector\Core\NodeManipulator\IfManipulator;
 use Rector\Core\Rector\AbstractRector;
@@ -83,9 +84,9 @@ public function getNodeTypes(): array
 
     /**
      * @param If_ $node
-     * @return Stmt[]|null|If_
+     * @return Stmt[]|null|int
      */
-    public function refactor(Node $node): If_|array|null
+    public function refactor(Node $node)
     {
         if (! $this->ifManipulator->isIfWithoutElseAndElseIfs($node)) {
             return null;
@@ -101,20 +102,20 @@ public function refactor(Node $node): If_|array|null
         }
 
         if ($node->cond instanceof BooleanNot && $node->cond->expr instanceof Instanceof_) {
-            return $this->processMayDeadInstanceOf($node, $node->cond->expr);
+            return $this->refactorStmtAndInstanceof($node, $node->cond->expr);
         }
 
         if ($node->cond instanceof Instanceof_) {
-            return $this->processMayDeadInstanceOf($node, $node->cond);
+            return $this->refactorStmtAndInstanceof($node, $node->cond);
         }
 
         return null;
     }
 
     /**
-     * @return null|Stmt[]|If_
+     * @return null|Stmt[]|int
      */
-    private function processMayDeadInstanceOf(If_ $if, Instanceof_ $instanceof): null|array|If_
+    private function refactorStmtAndInstanceof(If_ $if, Instanceof_ $instanceof): null|array|int
     {
         if (! $instanceof->class instanceof Name) {
             return null;
@@ -140,12 +141,15 @@ private function processMayDeadInstanceOf(If_ $if, Instanceof_ $instanceof): nul
             return null;
         }
 
-        if ($if->cond === $instanceof && $if->stmts !== []) {
-            return $if->stmts;
+        if ($if->cond !== $instanceof) {
+            return NodeTraverser::REMOVE_NODE;
         }
 
-        $this->removeNode($if);
-        return $if;
+        if ($if->stmts === []) {
+            return NodeTraverser::REMOVE_NODE;
+        }
+
+        return $if->stmts;
     }
 
     private function shouldSkipFromNotTypedParam(Instanceof_ $instanceof): bool

src/Rector/AbstractRector.php

@@ -11,6 +11,7 @@
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\InlineHTML;
 use PhpParser\Node\Stmt\Nop;
+use PhpParser\NodeTraverser;
 use PhpParser\NodeVisitorAbstract;
 use PHPStan\Analyser\MutatingScope;
 use PHPStan\Internal\BytesHelper;
@@ -118,6 +119,8 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
 
     private NodeConnectingTraverser $nodeConnectingTraverser;
 
+    private ?string $toBeRemovedNodeHash = null;
+
     #[Required]
     public function autowire(
         NodesToRemoveCollector $nodesToRemoveCollector,
@@ -232,7 +235,12 @@ final public function enterNode(Node $node)
             $this->printConsumptions($startTime, $previousMemory);
         }
 
-        // @see NodeTravser::* codes, e.g. removal of node of stopping the traversing
+        // @see NodeTraverser::* codes, e.g. removal of node of stopping the traversing
+        if ($refactoredNode === NodeTraverser::REMOVE_NODE && $originalNode instanceof Node) {
+            $this->toBeRemovedNodeHash = spl_object_hash($originalNode);
+            return $originalNode;
+        }
+
         if (is_int($refactoredNode)) {
             return $refactoredNode;
         }
@@ -256,6 +264,12 @@ final public function enterNode(Node $node)
      */
     public function leaveNode(Node $node)
     {
+        if ($this->toBeRemovedNodeHash !== null && $this->toBeRemovedNodeHash === spl_object_hash($node)) {
+            $this->toBeRemovedNodeHash = null;
+
+            return NodeConnectingTraverser::REMOVE_NODE;
+        }
+
         $objectHash = spl_object_hash($node);
 
         // update parents relations!!!
@@ -340,8 +354,13 @@ protected function removeNode(Node $node): void
     /**
      * @param Node|Node[] $refactoredNode
      */
-    private function postRefactorProcess(Node|null $originalNode, Node $node, Node|array $refactoredNode): Node
+    private function postRefactorProcess(Node|null $originalNode, Node $node, Node|array|int $refactoredNode): Node
     {
+        // node is removed, nothing to post process
+        if (is_int($refactoredNode)) {
+            return $originalNode ?? $node;
+        }
+
         $originalNode ??= $node;
 
         /** @var non-empty-array<Node>|Node $refactoredNode */