[DX] Improve ForToForeachRector

rules/cakephp-to-symfony/src/Rector/Echo_/CakePHPTemplateLinkToTwigRector.php

@@ -7,7 +7,6 @@
 use Nette\Utils\Html;
 use PhpParser\Node;
 use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Stmt\Echo_;
@@ -81,7 +80,7 @@ public function refactor(Node $node): ?Node
 
         # e.g. |trans https://symfony.com/doc/current/translation/templates.html#using-twig-filters
         $labelFilters = [];
-        if ($label instanceof FuncCall && $this->isName($label, '__')) {
+        if ($this->isFuncCallName($label, '__')) {
             $labelFilters[] = 'trans';
             $label = $label->args[0]->value;
         }

rules/code-quality/src/Rector/BooleanAnd/SimplifyEmptyArrayCheckRector.php

@@ -9,7 +9,6 @@
 use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
 use PhpParser\Node\Expr\BinaryOp\Identical;
 use PhpParser\Node\Expr\Empty_;
-use PhpParser\Node\Expr\FuncCall;
 use Rector\Core\PhpParser\Node\Manipulator\BinaryOpManipulator;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\RectorDefinition\CodeSample;
@@ -57,7 +56,7 @@ public function refactor(Node $node): ?Node
             $node,
             // is_array(...)
             function (Node $node): bool {
-                return $node instanceof FuncCall && $this->isName($node, 'is_array');
+                return $this->isFuncCallName($node, 'is_array');
             },
             Empty_::class
         );

rules/code-quality/src/Rector/For_/ForToForeachRector.php

@@ -16,10 +16,10 @@
 use PhpParser\Node\Expr\PreInc;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Stmt;
-use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\For_;
 use PhpParser\Node\Stmt\Foreach_;
 use Rector\Core\Exception\ShouldNotHappenException;
+use Rector\Core\PhpParser\Node\Manipulator\AssignManipulator;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\RectorDefinition\CodeSample;
 use Rector\Core\RectorDefinition\RectorDefinition;
@@ -45,6 +45,16 @@ final class ForToForeachRector extends AbstractRector
      */
     private $iteratedExpr;
 
+    /**
+     * @var AssignManipulator
+     */
+    private $assignManipulator;
+
+    public function __construct(AssignManipulator $assignManipulator)
+    {
+        $this->assignManipulator = $assignManipulator;
+    }
+
     public function getDefinition(): RectorDefinition
     {
         return new RectorDefinition('Change for() to foreach() where useful', [
@@ -123,15 +133,7 @@ public function refactor(Node $node): ?Node
         }
 
         $iteratedVariableSingle = Inflector::singularize($iteratedVariable);
-
-        $foreach = new Foreach_($this->iteratedExpr, new Variable($iteratedVariableSingle));
-        $foreach->stmts = $node->stmts;
-
-        if ($this->keyValueName === null) {
-            return null;
-        }
-
-        $foreach->keyVar = new Variable($this->keyValueName);
+        $foreach = $this->createForeach($node, $iteratedVariableSingle);
 
         $this->useForeachVariableInStmts($foreach->valueVar, $foreach->stmts);
 
@@ -151,15 +153,17 @@ private function reset(): void
     private function matchInit(array $initExprs): void
     {
         foreach ($initExprs as $initExpr) {
-            if ($initExpr instanceof Assign) {
-                if ($this->isValue($initExpr->expr, 0)) {
-                    $this->keyValueName = $this->getName($initExpr->var);
-                }
+            if (! $initExpr instanceof Assign) {
+                continue;
+            }
 
-                if ($initExpr->expr instanceof FuncCall && $this->isName($initExpr->expr, 'count')) {
-                    $this->countValueName = $this->getName($initExpr->var);
-                    $this->iteratedExpr = $initExpr->expr->args[0]->value;
-                }
+            if ($this->isValue($initExpr->expr, 0)) {
+                $this->keyValueName = $this->getName($initExpr->var);
+            }
+
+            if ($this->isFuncCallName($initExpr->expr, 'count')) {
+                $this->countValueName = $this->getName($initExpr->var);
+                $this->iteratedExpr = $initExpr->expr->args[0]->value;
             }
         }
     }
@@ -182,7 +186,7 @@ private function isConditionMatch(array $condExprs): bool
         }
 
         // count($values)
-        if ($condExprs[0]->right instanceof FuncCall && $this->isName($condExprs[0]->right, 'count')) {
+        if ($this->isFuncCallName($condExprs[0]->right, 'count')) {
             /** @var FuncCall $countFuncCall */
             $countFuncCall = $condExprs[0]->right;
             $this->iteratedExpr = $countFuncCall->args[0]->value;
@@ -232,8 +236,7 @@ private function useForeachVariableInStmts(Expr $expr, array $stmts): void
             }
 
             $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
-
-            if ($this->isPartOfAssign($parentNode)) {
+            if ($this->assignManipulator->isNodePartOfAssign($parentNode)) {
                 return null;
             }
 
@@ -275,24 +278,20 @@ private function isSmallerOrGreater(array $condExprs, string $keyValueName, stri
         return false;
     }
 
-    private function isPartOfAssign(?Node $node): bool
+    private function createForeach(For_ $for, string $iteratedVariableName): Foreach_
     {
-        if ($node === null) {
-            return false;
+        if ($this->iteratedExpr === null) {
+            throw new ShouldNotHappenException();
         }
 
-        $previousNode = $node;
-        $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
-
-        while ($parentNode !== null && ! $parentNode instanceof Expression) {
-            if ($parentNode instanceof Assign && $this->areNodesEqual($parentNode->var, $previousNode)) {
-                return true;
-            }
-
-            $previousNode = $parentNode;
-            $parentNode = $parentNode->getAttribute(AttributeKey::PARENT_NODE);
+        if ($this->keyValueName === null) {
+            throw new ShouldNotHappenException();
         }
 
-        return false;
+        $foreach = new Foreach_($this->iteratedExpr, new Variable($iteratedVariableName));
+        $foreach->stmts = $for->stmts;
+        $foreach->keyVar = new Variable($this->keyValueName);
+
+        return $foreach;
     }
 }

rules/code-quality/src/Rector/Identical/SimplifyArraySearchRector.php

@@ -64,7 +64,7 @@ public function refactor(Node $node): ?Node
         $matchedNodes = $this->binaryOpManipulator->matchFirstAndSecondConditionNode(
             $node,
             function (Node $node): bool {
-                return $node instanceof FuncCall && $this->isName($node, 'array_search');
+                return $this->isFuncCallName($node, 'array_search');
             },
             function (Node $node): bool {
                 return $this->isBool($node);

rules/code-quality/src/Rector/If_/ExplicitBoolCompareRector.php

@@ -13,7 +13,6 @@
 use PhpParser\Node\Expr\BinaryOp\NotIdentical;
 use PhpParser\Node\Expr\BooleanNot;
 use PhpParser\Node\Expr\Cast\Bool_;
-use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\Ternary;
 use PhpParser\Node\Scalar\DNumber;
 use PhpParser\Node\Scalar\LNumber;
@@ -113,7 +112,7 @@ public function refactor(Node $node): ?Node
     private function resolveNewConditionNode(Expr $expr, bool $isNegated): ?BinaryOp
     {
         // various cases
-        if ($expr instanceof FuncCall && $this->isName($expr, 'count')) {
+        if ($this->isFuncCallName($expr, 'count')) {
             return $this->resolveCount($isNegated, $expr);
         }
 

rules/php-72/src/Rector/FuncCall/ParseStrWithResultArgumentRector.php

@@ -74,7 +74,7 @@ public function refactor(Node $node): ?Node
         }
 
         $this->traverseNodesWithCallable($nextExpression, function (Node $node) use ($resultVariable): ?Variable {
-            if ($node instanceof FuncCall && $this->isName($node, 'get_defined_vars')) {
+            if ($this->isFuncCallName($node, 'get_defined_vars')) {
                 return $resultVariable;
             }
 

rules/php-74/src/Rector/FuncCall/ArraySpreadInsteadOfArrayMergeRector.php

@@ -173,7 +173,7 @@ private function createUnpackedArrayItem(Expr $expr): ArrayItem
 
     private function isIteratorToArrayFuncCall(Expr $expr): bool
     {
-        return $expr instanceof FuncCall && $this->isName($expr, 'iterator_to_array');
+        return $this->isFuncCallName($expr, 'iterator_to_array');
     }
 
     private function isConstantArrayTypeWithStringKeyType(Type $type): bool

rules/symfony/src/Rector/HttpKernel/GetRequestRector.php

@@ -139,7 +139,7 @@ private function isGetRequestInAction(Node $node): bool
         }
 
         // must be $this->getRequest() in controller
-        if (! $this->isName($node->var, 'this')) {
+        if (! $this->isThisVariable($node->var)) {
             return false;
         }
 
@@ -220,15 +220,20 @@ private function containsGetRequestMethod(Node $node): bool
                 return false;
             }
 
-            if (! $node->var instanceof Variable) {
-                return false;
-            }
-
-            if (! $this->isName($node->var, 'this')) {
+            if (! $this->isThisVariable($node->var)) {
                 return false;
             }
 
             return $this->isName($node->name, 'getRequest');
         });
     }
+
+    private function isThisVariable(Node $node): bool
+    {
+        if (! $node instanceof Variable) {
+            return false;
+        }
+
+        return $this->isName($node, 'this');
+    }
 }

src/PhpParser/Node/Manipulator/AssignManipulator.php

@@ -15,6 +15,8 @@
 use PhpParser\Node\Expr\PostInc;
 use PhpParser\Node\Expr\PreDec;
 use PhpParser\Node\Expr\PreInc;
+use PhpParser\Node\Stmt\Expression;
+use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
 use Rector\NodeNameResolver\NodeNameResolver;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 
@@ -41,10 +43,19 @@ final class AssignManipulator
      */
     private $propertyFetchManipulator;
 
-    public function __construct(NodeNameResolver $nodeNameResolver, PropertyFetchManipulator $propertyFetchManipulator)
-    {
+    /**
+     * @var BetterStandardPrinter
+     */
+    private $betterStandardPrinter;
+
+    public function __construct(
+        NodeNameResolver $nodeNameResolver,
+        PropertyFetchManipulator $propertyFetchManipulator,
+        BetterStandardPrinter $betterStandardPrinter
+    ) {
         $this->nodeNameResolver = $nodeNameResolver;
         $this->propertyFetchManipulator = $propertyFetchManipulator;
+        $this->betterStandardPrinter = $betterStandardPrinter;
     }
 
     /**
@@ -141,6 +152,30 @@ public function isNodeLeftPartOfAssign(Node $node): bool
         return false;
     }
 
+    public function isNodePartOfAssign(?Node $node): bool
+    {
+        if ($node === null) {
+            return false;
+        }
+
+        $previousNode = $node;
+        $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
+
+        while ($parentNode !== null && ! $parentNode instanceof Expression) {
+            if ($parentNode instanceof Assign && $this->betterStandardPrinter->areNodesEqual(
+                $parentNode->var,
+                $previousNode
+            )) {
+                return true;
+            }
+
+            $previousNode = $parentNode;
+            $parentNode = $parentNode->getAttribute(AttributeKey::PARENT_NODE);
+        }
+
+        return false;
+    }
+
     private function isValueModifyingNode(Node $node): bool
     {
         foreach (self::MODIFYING_NODES as $modifyingNode) {

src/Rector/AbstractRector/NameResolverTrait.php

@@ -5,6 +5,7 @@
 namespace Rector\Core\Rector\AbstractRector;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Identifier;
 use PhpParser\Node\Name;
 use Rector\CodingStyle\Naming\ClassNaming;
@@ -65,4 +66,13 @@ protected function getShortName($name): string
     {
         return $this->classNaming->getShortName($name);
     }
+
+    protected function isFuncCallName(Node $node, string $name): bool
+    {
+        if (! $node instanceof FuncCall) {
+            return false;
+        }
+
+        return $this->isName($node, $name);
+    }
 }