Refactor PARENT_NODE away from ListEachRector (#3946)

rectorphp · May 24, 2023 · c050b66 · c050b66
1 parent 2791261
commit c050b66
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 26 deletions.
diff --git a/rules-tests/Php72/Rector/Assign/ListEachRector/Fixture/fixture2.php.inc b/rules-tests/Php72/Rector/Assign/ListEachRector/Fixture/fixture2.php.inc
@@ -22,9 +22,7 @@ function each2()
     $key = key($opt->option);
     $val = current($opt->option);
     next($opt->option);
-
     $tid = key($option->option);
-
     $curr = current($tree);
     next($tree);
 }

diff --git a/rules-tests/Php72/Rector/Assign/ListEachRector/Fixture/list_each_next.php.inc b/rules-tests/Php72/Rector/Assign/ListEachRector/Fixture/list_each_next.php.inc
@@ -27,11 +27,9 @@ final class ListEachNext
     public function run()
     {
         $parentArray = ['a' => 1, 'b' => 2];
-
         $key = key($parentArray);
         $value = current($parentArray);
         next($parentArray);
-
         $key2 = key($parentArray);
         $value2 = current($parentArray);
         next($parentArray);

diff --git a/rules-tests/Php72/Rector/Assign/ListEachRector/Fixture/skip_while_loop.php.inc b/rules-tests/Php72/Rector/Assign/ListEachRector/Fixture/skip_while_loop.php.inc
@@ -0,0 +1,15 @@
+<?php
+
+namespace Rector\Tests\Php72\Rector\Assign\ListEachRector\Fixture;
+
+final class SkipWhileLoop
+{
+    public function run($opt)
+    {
+        while (list($key, $val) = each($opt->option)) {
+            return false;
+        }
+
+        return true;
+    }
+}
diff --git a/rules/Php72/Rector/Assign/ListEachRector.php b/rules/Php72/Rector/Assign/ListEachRector.php
@@ -14,8 +14,6 @@
 use Rector\Core\NodeManipulator\AssignManipulator;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\ValueObject\PhpVersionFeature;
-use Rector\NodeTypeResolver\Node\AttributeKey;
-use Rector\PostRector\Collector\NodesToAddCollector;
 use Rector\VersionBonding\Contract\MinPhpVersionInterface;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -29,7 +27,6 @@ final class ListEachRector extends AbstractRector implements MinPhpVersionInterf
 {
     public function __construct(
         private readonly AssignManipulator $assignManipulator,
-        private readonly NodesToAddCollector $nodesToAddCollector,
     ) {
     }
 
@@ -63,23 +60,28 @@ public function getRuleDefinition(): RuleDefinition
      */
     public function getNodeTypes(): array
     {
-        return [Assign::class];
+        return [Expression::class];
     }
 
     /**
-     * @param Assign $node
+     * @param Expression $node
      */
-    public function refactor(Node $node): ?Node
+    public function refactor(Node $node)
     {
-        if ($this->shouldSkip($node)) {
+        if (! $node->expr instanceof Assign) {
+            return null;
+        }
+
+        $assign = $node->expr;
+        if ($this->shouldSkipAssign($assign)) {
             return null;
         }
 
         /** @var List_ $listNode */
-        $listNode = $node->var;
+        $listNode = $assign->var;
 
         /** @var FuncCall $eachFuncCall */
-        $eachFuncCall = $node->expr;
+        $eachFuncCall = $assign->expr;
 
         // only key: list($key, ) = each($values);
         if ($listNode->items[0] instanceof ArrayItem && ! $listNode->items[1] instanceof ArrayItem) {
@@ -90,12 +92,14 @@ public function refactor(Node $node): ?Node
         // only value: list(, $value) = each($values);
         if ($listNode->items[1] instanceof ArrayItem && ! $listNode->items[0] instanceof ArrayItem) {
             $nextFuncCall = $this->nodeFactory->createFuncCall('next', $eachFuncCall->args);
-            $this->nodesToAddCollector->addNodeAfterNode($nextFuncCall, $node);
+            //            $this->nodesToAddCollector->addNodeAfterNode($nextFuncCall, $assign);
 
             $currentFuncCall = $this->nodeFactory->createFuncCall('current', $eachFuncCall->args);
 
             $secondArrayItem = $listNode->items[1];
-            return new Assign($secondArrayItem->value, $currentFuncCall);
+            $currentAssign = new Assign($secondArrayItem->value, $currentFuncCall);
+
+            return [new Expression($currentAssign), new Expression($nextFuncCall)];
         }
 
         // both: list($key, $value) = each($values);
@@ -106,11 +110,11 @@ public function refactor(Node $node): ?Node
             throw new ShouldNotHappenException();
         }
 
-        $assign = new Assign($secondArrayItem->value, $currentFuncCall);
-        $this->nodesToAddCollector->addNodeAfterNode($assign, $node);
+        $currentAssign = new Assign($secondArrayItem->value, $currentFuncCall);
+        //        $this->nodesToAddCollector->addNodeAfterNode($assign, $assign);
 
         $nextFuncCall = $this->nodeFactory->createFuncCall('next', $eachFuncCall->args);
-        $this->nodesToAddCollector->addNodeAfterNode($nextFuncCall, $node);
+        //        $this->nodesToAddCollector->addNodeAfterNode($nextFuncCall, $node);
 
         $keyFuncCall = $this->nodeFactory->createFuncCall('key', $eachFuncCall->args);
 
@@ -119,21 +123,17 @@ public function refactor(Node $node): ?Node
             throw new ShouldNotHappenException();
         }
 
-        return new Assign($firstArrayItem->value, $keyFuncCall);
+        $keyAssign = new Assign($firstArrayItem->value, $keyFuncCall);
+
+        return [new Expression($keyAssign), new Expression($currentAssign), new Expression($nextFuncCall)];
     }
 
-    private function shouldSkip(Assign $assign): bool
+    private function shouldSkipAssign(Assign $assign): bool
     {
         if (! $this->assignManipulator->isListToEachAssign($assign)) {
             return true;
         }
 
-        // assign should be top level, e.g. not in a while loop
-        $parentNode = $assign->getAttribute(AttributeKey::PARENT_NODE);
-        if (! $parentNode instanceof Expression) {
-            return true;
-        }
-
         /** @var List_ $listNode */
         $listNode = $assign->var;