Remove removeNode() from RemoveDeadIfForeachForRector (#4072)

* Remove removeNode() from RemoveDeadIfForeachForRector * [ci-review] Rector Rectify --------- Co-authored-by: GitHub Action <actions@github.com>
rectorphp · Jun 4, 2023 · 5f2ddbf · 5f2ddbf
1 parent 8f68eb4
commit 5f2ddbf
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 67 deletions.
diff --git a/phpstan.neon b/phpstan.neon
@@ -735,3 +735,4 @@ parameters:
 
         # resolve later
         - '#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 18, keep it under 11#'
diff --git a/...ests/DeadCode/Rector/For_/RemoveDeadIfForeachForRector/Fixture/side_effect_checks.php.inc b/...ests/DeadCode/Rector/For_/RemoveDeadIfForeachForRector/Fixture/side_effect_checks.php.inc
@@ -27,8 +27,6 @@ class SideEffectChecks
 {
     public function run()
     {
-        if (5 + 10) {
-        }
     }
 }
 

diff --git a/rules/DeadCode/Rector/For_/RemoveDeadIfForeachForRector.php b/rules/DeadCode/Rector/For_/RemoveDeadIfForeachForRector.php
@@ -6,12 +6,13 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\Variable;
-use PhpParser\Node\Scalar;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\CallLike;
 use PhpParser\Node\Stmt\Else_;
 use PhpParser\Node\Stmt\For_;
 use PhpParser\Node\Stmt\Foreach_;
 use PhpParser\Node\Stmt\If_;
+use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Core\Rector\AbstractRector;
 use Rector\EarlyReturn\NodeTransformer\ConditionInverter;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -36,15 +37,11 @@ public function getRuleDefinition(): RuleDefinition
                     <<<'CODE_SAMPLE'
 class SomeClass
 {
-    public function run($someObject)
+    public function run($value)
     {
-        $value = 5;
         if ($value) {
         }
 
-        if ($someObject->run()) {
-        }
-
         foreach ($values as $value) {
         }
 
@@ -56,12 +53,8 @@ public function run($someObject)
                     <<<'CODE_SAMPLE'
 class SomeClass
 {
-    public function run($someObject)
+    public function run($value)
     {
-        $value = 5;
-        if ($someObject->run()) {
-        }
-
         return $value;
     }
 }
@@ -76,82 +69,67 @@ public function run($someObject)
      */
     public function getNodeTypes(): array
     {
-        return [For_::class, If_::class, Foreach_::class];
+        return [StmtsAwareInterface::class];
     }
 
     /**
-     * @param For_|If_|Foreach_ $node
+     * @param StmtsAwareInterface $node
      */
     public function refactor(Node $node): ?Node
     {
-        if ($node instanceof If_) {
-            $this->processIf($node);
-            return null;
-        }
-
-        if ($node instanceof Foreach_) {
-            $this->processForeach($node);
-            return null;
-        }
-
-        // For
-        if ($node->stmts !== []) {
+        if ($node->stmts === null) {
             return null;
         }
 
-        $this->removeNode($node);
+        $hasChanged = false;
 
-        return null;
-    }
+        foreach ($node->stmts as $key => $stmt) {
+            if ($stmt instanceof If_) {
+                $if = $stmt;
 
-    private function processIf(If_ $if): void
-    {
-        if ($if->stmts !== []) {
-            return;
-        }
+                if ($if->stmts !== []) {
+                    continue;
+                }
 
-        if ($if->elseifs !== []) {
-            return;
-        }
+                if ($if->elseifs !== []) {
+                    continue;
+                }
 
-        if ($if->else instanceof Else_) {
-            $if->cond = $this->conditionInverter->createInvertedCondition($if->cond);
-            $if->stmts = $if->else->stmts;
-            $if->else = null;
+                // useless if ()
+                if (! $if->else instanceof Else_) {
+                    if ($this->hasNodeSideEffect($if->cond)) {
+                        continue;
+                    }
 
-            return;
-        }
+                    unset($node->stmts[$key]);
+                    $hasChanged = true;
+                    continue;
+                }
 
-        if ($this->isNodeWithSideEffect($if->cond)) {
-            return;
-        }
+                $if->cond = $this->conditionInverter->createInvertedCondition($if->cond);
+                $if->stmts = $if->else->stmts;
+                $if->else = null;
+                $hasChanged = true;
 
-        $this->removeNode($if);
-    }
+                continue;
+            }
 
-    private function processForeach(Foreach_ $foreach): void
-    {
-        if ($foreach->stmts !== []) {
-            return;
+            // nothing to "for"
+            if (($stmt instanceof For_ || $stmt instanceof Foreach_) && $stmt->stmts === []) {
+                unset($node->stmts[$key]);
+                $hasChanged = true;
+            }
         }
 
-        if ($this->isNodeWithSideEffect($foreach->expr)) {
-            return;
+        if ($hasChanged) {
+            return $node;
         }
 
-        $this->removeNode($foreach);
+        return null;
     }
 
-    private function isNodeWithSideEffect(Expr $expr): bool
+    private function hasNodeSideEffect(Expr $expr): bool
     {
-        if ($expr instanceof Variable) {
-            return false;
-        }
-
-        if ($expr instanceof Scalar) {
-            return false;
-        }
-
-        return ! $this->valueResolver->isTrueOrFalse($expr);
+        return $this->betterNodeFinder->hasInstancesOf($expr, [CallLike::class, Assign::class]);
     }
 }