[DeadCode] Add empty() + count($values) > 0 checks to RemoveUnusedNonEmptyArrayBeforeForeachRector

rules/dead-code/src/NodeManipulator/CountManipulator.php

@@ -0,0 +1,120 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\DeadCode\NodeManipulator;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\BinaryOp\Greater;
+use PhpParser\Node\Expr\BinaryOp\GreaterOrEqual;
+use PhpParser\Node\Expr\BinaryOp\Smaller;
+use PhpParser\Node\Expr\BinaryOp\SmallerOrEqual;
+use PhpParser\Node\Expr\FuncCall;
+use PhpParser\Node\Scalar\LNumber;
+use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
+use Rector\NodeNameResolver\NodeNameResolver;
+
+final class CountManipulator
+{
+    /**
+     * @var BetterStandardPrinter
+     */
+    private $betterStandardPrinter;
+
+    /**
+     * @var NodeNameResolver
+     */
+    private $nodeNameResolver;
+
+    public function __construct(BetterStandardPrinter $betterStandardPrinter, NodeNameResolver $nodeNameResolver)
+    {
+        $this->betterStandardPrinter = $betterStandardPrinter;
+        $this->nodeNameResolver = $nodeNameResolver;
+    }
+
+    public function isCounterHigherThanOne(Node $node, Expr $expr): bool
+    {
+        // e.g. count($values) > 0
+        if ($node instanceof Greater) {
+            return $this->processGreater($node, $expr);
+        }
+
+        // e.g. count($values) >= 1
+        if ($node instanceof GreaterOrEqual) {
+            return $this->processGreaterOrEqual($node, $expr);
+        }
+
+        // e.g. 0 < count($values)
+        if ($node instanceof Smaller) {
+            return $this->processSmaller($node, $expr);
+        }
+
+        // e.g. 1 <= count($values)
+        if ($node instanceof SmallerOrEqual) {
+            return $this->processSmallerOrEqual($node, $expr);
+        }
+
+        return false;
+    }
+
+    private function isNumber(Node $node, int $value): bool
+    {
+        if (! $node instanceof LNumber) {
+            return false;
+        }
+
+        return $node->value === $value;
+    }
+
+    private function processGreater(Greater $greater, Expr $expr): bool
+    {
+        if (! $this->isNumber($greater->right, 0)) {
+            return false;
+        }
+
+        return $this->isCountWithExpression($greater->left, $expr);
+    }
+
+    private function processSmaller(Smaller $smaller, Expr $expr): bool
+    {
+        if (! $this->isNumber($smaller->left, 0)) {
+            return false;
+        }
+
+        return $this->isCountWithExpression($smaller->right, $expr);
+    }
+
+    private function processGreaterOrEqual(GreaterOrEqual $greaterOrEqual, Expr $expr)
+    {
+        if (! $this->isNumber($greaterOrEqual->right, 1)) {
+            return false;
+        }
+
+        return $this->isCountWithExpression($greaterOrEqual->left, $expr);
+    }
+
+    private function processSmallerOrEqual(SmallerOrEqual $smallerOrEqual, Expr $expr): bool
+    {
+        if (! $this->isNumber($smallerOrEqual->left, 1)) {
+            return false;
+        }
+
+        return $this->isCountWithExpression($smallerOrEqual->right, $expr);
+    }
+
+    private function isCountWithExpression(Node $node, Expr $expr): bool
+    {
+        if (! $node instanceof FuncCall) {
+            return false;
+        }
+
+        if (! $this->nodeNameResolver->isName($node, 'count')) {
+            return false;
+        }
+
+        $countedExpr = $node->args[0]->value;
+
+        return $this->betterStandardPrinter->areNodesWithoutCommentsEqual($countedExpr, $expr);
+    }
+}

rules/dead-code/src/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php

@@ -5,17 +5,14 @@
 namespace Rector\DeadCode\Rector\If_;
 
 use PhpParser\Node;
-use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\Array_;
-use PhpParser\Node\Expr\BinaryOp;
-use PhpParser\Node\Expr\BinaryOp\NotEqual;
-use PhpParser\Node\Expr\BinaryOp\NotIdentical;
 use PhpParser\Node\Stmt\Foreach_;
 use PhpParser\Node\Stmt\If_;
 use Rector\Core\PhpParser\Node\Manipulator\IfManipulator;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\RectorDefinition\CodeSample;
 use Rector\Core\RectorDefinition\RectorDefinition;
+use Rector\DeadCode\NodeManipulator\CountManipulator;
+use Rector\DeadCode\UselessIfCondBeforeForeachDetector;
 
 /**
  * @see \Rector\DeadCode\Tests\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\RemoveUnusedNonEmptyArrayBeforeForeachRectorTest
@@ -27,9 +24,24 @@ final class RemoveUnusedNonEmptyArrayBeforeForeachRector extends AbstractRector
      */
     private $ifManipulator;
 
-    public function __construct(IfManipulator $ifManipulator)
-    {
+    /**
+     * @var UselessIfCondBeforeForeachDetector
+     */
+    private $uselessIfCondBeforeForeachDetector;
+
+    /**
+     * @var CountManipulator
+     */
+    private $countManipulator;
+
+    public function __construct(
+        IfManipulator $ifManipulator,
+        UselessIfCondBeforeForeachDetector $uselessIfCondBeforeForeachDetector,
+        CountManipulator $countManipulator
+    ) {
         $this->ifManipulator = $ifManipulator;
+        $this->uselessIfCondBeforeForeachDetector = $uselessIfCondBeforeForeachDetector;
+        $this->countManipulator = $countManipulator;
     }
 
     public function getDefinition(): RectorDefinition
@@ -81,55 +93,31 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
-        if (! $this->ifManipulator->isIfWithOnlyForeach($node)) {
-            return null;
-        }
-
-        /** @var Foreach_ $foreach */
-        $foreach = $node->stmts[0];
-        $foreachExpr = $foreach->expr;
-
-        if (! $node->cond instanceof NotIdentical && ! $node->cond instanceof NotEqual) {
-            return null;
-        }
-
-        /** @var NotIdentical|NotEqual $notIdentical */
-        $notIdentical = $node->cond;
-
-        if (! $this->isMatching($notIdentical, $foreachExpr)) {
+        if (! $this->isUselessBeforeForeachCheck($node)) {
             return null;
         }
 
-        return $foreach;
+        return $node->stmts[0];
     }
 
-    private function isEmptyArray(Expr $expr): bool
+    private function isUselessBeforeForeachCheck(If_ $if): bool
     {
-        if (! $expr instanceof Array_) {
+        if (! $this->ifManipulator->isIfWithOnlyForeach($if)) {
             return false;
         }
 
-        return $expr->items === [];
-    }
+        /** @var Foreach_ $foreach */
+        $foreach = $if->stmts[0];
+        $foreachExpr = $foreach->expr;
 
-    private function isEmptyArrayAndForeachedVariable(Expr $leftExpr, Expr $rightExpr, Expr $foreachExpr): bool
-    {
-        if (! $this->isEmptyArray($leftExpr)) {
-            return false;
+        if ($this->uselessIfCondBeforeForeachDetector->isMatchingNotIdenticalEmptyArray($if, $foreachExpr)) {
+            return true;
         }
 
-        return $this->areNodesWithoutCommentsEqual($foreachExpr, $rightExpr);
-    }
-
-    /**
-     * @param NotIdentical|NotEqual $binaryOp
-     */
-    private function isMatching(BinaryOp $binaryOp, Expr $foreachExpr): bool
-    {
-        if ($this->isEmptyArrayAndForeachedVariable($binaryOp->left, $binaryOp->right, $foreachExpr)) {
+        if ($this->uselessIfCondBeforeForeachDetector->isMatchingNotEmpty($if, $foreachExpr)) {
             return true;
         }
 
-        return $this->isEmptyArrayAndForeachedVariable($binaryOp->right, $binaryOp->left, $foreachExpr);
+        return $this->countManipulator->isCounterHigherThanOne($if->cond, $foreachExpr);
     }
 }

rules/dead-code/src/UselessIfCondBeforeForeachDetector.php

@@ -0,0 +1,98 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\DeadCode;
+
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\Array_;
+use PhpParser\Node\Expr\BinaryOp;
+use PhpParser\Node\Expr\BinaryOp\NotEqual;
+use PhpParser\Node\Expr\BinaryOp\NotIdentical;
+use PhpParser\Node\Expr\BooleanNot;
+use PhpParser\Node\Expr\Empty_;
+use PhpParser\Node\Stmt\If_;
+use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
+
+final class UselessIfCondBeforeForeachDetector
+{
+    /**
+     * @var BetterStandardPrinter
+     */
+    private $betterStandardPrinter;
+
+    public function __construct(BetterStandardPrinter $betterStandardPrinter)
+    {
+        $this->betterStandardPrinter = $betterStandardPrinter;
+    }
+
+    /**
+     * Matches:
+     * !empty($values)
+     */
+    public function isMatchingNotEmpty(If_ $if, Expr $foreachExpr): bool
+    {
+        $cond = $if->cond;
+        if (! $cond instanceof BooleanNot) {
+            return false;
+        }
+
+        if (! $cond->expr instanceof Empty_) {
+            return false;
+        }
+
+        /** @var Empty_ $empty */
+        $empty = $cond->expr;
+
+        return $this->betterStandardPrinter->areNodesWithoutCommentsEqual($empty->expr, $foreachExpr);
+    }
+
+    /**
+     * Matches:
+     * $values !== []
+     * $values != []
+     * [] !== $values
+     * [] != $values
+     */
+    public function isMatchingNotIdenticalEmptyArray(If_ $if, Expr $foreachExpr): bool
+    {
+        if (! $if->cond instanceof NotIdentical && ! $if->cond instanceof NotEqual) {
+            return false;
+        }
+
+        /** @var NotIdentical|NotEqual $notIdentical */
+        $notIdentical = $if->cond;
+
+        return $this->isMatchingNotBinaryOp($notIdentical, $foreachExpr);
+    }
+
+    /**
+     * @param NotIdentical|NotEqual $binaryOp
+     */
+    private function isMatchingNotBinaryOp(BinaryOp $binaryOp, Expr $foreachExpr): bool
+    {
+        if ($this->isEmptyArrayAndForeachedVariable($binaryOp->left, $binaryOp->right, $foreachExpr)) {
+            return true;
+        }
+
+        return $this->isEmptyArrayAndForeachedVariable($binaryOp->right, $binaryOp->left, $foreachExpr);
+    }
+
+    private function isEmptyArrayAndForeachedVariable(Expr $leftExpr, Expr $rightExpr, Expr $foreachExpr): bool
+    {
+        if (! $this->isEmptyArray($leftExpr)) {
+            return false;
+        }
+
+        return $this->betterStandardPrinter->areNodesWithoutCommentsEqual($foreachExpr, $rightExpr);
+    }
+
+    private function isEmptyArray(Expr $expr): bool
+    {
+        if (! $expr instanceof Array_) {
+            return false;
+        }
+
+        return $expr->items === [];
+    }
+}

rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/keep_if_else.php.inc

@@ -0,0 +1,18 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\Fixture;
+
+class KeepIfElse
+{
+    public function run()
+    {
+        $values = [];
+        if ($values !== []) {
+            foreach ($values as $value) {
+                echo $value;
+            }
+        } else {
+            return;
+        }
+    }
+}

rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/more_or_equal_than_one.php.inc

@@ -0,0 +1,35 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\Fixture;
+
+class MoreOrEqualThanOne
+{
+    public function run()
+    {
+        $values = [];
+        if (count($values) >= 1) {
+            foreach ($values as $value) {
+                echo $value;
+            }
+        }
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\Fixture;
+
+class MoreOrEqualThanOne
+{
+    public function run()
+    {
+        $values = [];
+        foreach ($values as $value) {
+            echo $value;
+        }
+    }
+}
+
+?>