Remove PARENT_NODE from ForeachItemsAssignToEmptyArrayToAssignRector (#…

…3958)

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

packages/NodeNestingScope/ValueObject/ControlStructure.php

@@ -6,7 +6,6 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Expr\Match_;
-use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Case_;
 use PhpParser\Node\Stmt\Catch_;
@@ -21,22 +20,6 @@
 
 final class ControlStructure
 {
-    /**
-     * @var array<class-string<Node>>
-     */
-    public const BREAKING_SCOPE_NODE_TYPES = [
-        For_::class,
-        Foreach_::class,
-        If_::class,
-        While_::class,
-        Do_::class,
-        Else_::class,
-        ElseIf_::class,
-        Catch_::class,
-        Case_::class,
-        FunctionLike::class,
-    ];
-
     /**
      * These situations happens only if condition is met
      * @var array<class-string<Node>>

packages/ReadWrite/NodeFinder/NodeUsageFinder.php

@@ -5,22 +5,16 @@
 namespace Rector\ReadWrite\NodeFinder;
 
 use PhpParser\Node;
-use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Variable;
-use PhpParser\Node\Stmt\Foreach_;
-use Rector\Core\PhpParser\Comparing\NodeComparator;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\NodeNameResolver\NodeNameResolver;
-use Rector\NodeNestingScope\NodeFinder\ScopeAwareNodeFinder;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 
 final class NodeUsageFinder
 {
     public function __construct(
         private readonly NodeNameResolver $nodeNameResolver,
         private readonly BetterNodeFinder $betterNodeFinder,
-        private readonly ScopeAwareNodeFinder $scopeAwareNodeFinder,
-        private readonly NodeComparator $nodeComparator
     ) {
     }
 
@@ -52,16 +46,4 @@ public function findVariableUsages(array $nodes, Variable $variable): array
             return $assignedTo === null;
         });
     }
-
-    public function findPreviousForeachNodeUsage(Foreach_ $foreach, Expr $expr): ?Node
-    {
-        return $this->scopeAwareNodeFinder->findParent($foreach, function (Node $node) use ($expr): bool {
-            // skip itself
-            if ($node === $expr) {
-                return false;
-            }
-
-            return $this->nodeComparator->areNodesEqual($node, $expr);
-        }, [Foreach_::class]);
-    }
 }

phpstan.neon

@@ -99,7 +99,6 @@ parameters:
             message: '#If condition is always false#'
             paths:
                 - rules/CodeQuality/Rector/Concat/JoinStringConcatRector.php
-                - packages/NodeNestingScope/NodeFinder/ScopeAwareNodeFinder.php
 
         - '#Method (.*?) should return array<PhpParser\\Node\\(.*?)\> but returns array<PhpParser\\Node\>#'
 

rules/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector.php

@@ -4,16 +4,18 @@
 
 namespace Rector\CodeQuality\Rector\Foreach_;
 
+use PhpParser\Node\Stmt;
 use PhpParser\Node;
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Foreach_;
 use PHPStan\Analyser\Scope;
 use Rector\CodeQuality\NodeAnalyzer\ForeachAnalyzer;
+use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Core\Rector\AbstractScopeAwareRector;
 use Rector\NodeNestingScope\ValueObject\ControlStructure;
-use Rector\NodeTypeResolver\Node\AttributeKey;
-use Rector\ReadWrite\NodeFinder\NodeUsageFinder;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -23,7 +25,6 @@
 final class ForeachItemsAssignToEmptyArrayToAssignRector extends AbstractScopeAwareRector
 {
     public function __construct(
-        private readonly NodeUsageFinder $nodeUsageFinder,
         private readonly ForeachAnalyzer $foreachAnalyzer
     ) {
     }
@@ -69,61 +70,98 @@ public function run($items)
      */
     public function getNodeTypes(): array
     {
-        return [Foreach_::class];
+        return [StmtsAwareInterface::class];
     }
 
     /**
-     * @param Foreach_ $node
+     * @param StmtsAwareInterface $node
      */
     public function refactorWithScope(Node $node, Scope $scope): ?Node
     {
-        if ($this->shouldSkip($node, $scope)) {
+        if ($node->stmts === null) {
             return null;
         }
 
-        $assignVariable = $this->foreachAnalyzer->matchAssignItemsOnlyForeachArrayVariable($node);
-        if (! $assignVariable instanceof Expr) {
-            return null;
+        $emptyArrayVariables = [];
+
+        foreach ($node->stmts as $key => $stmt) {
+            $variableName = $this->matchEmptyArrayVariableAssign($stmt);
+            if (is_string($variableName)) {
+                $emptyArrayVariables[] = $variableName;
+            }
+
+            if (! $stmt instanceof Foreach_) {
+                continue;
+            }
+
+            if ($this->shouldSkip($stmt, $emptyArrayVariables)) {
+                continue;
+            }
+
+            $assignVariable = $this->foreachAnalyzer->matchAssignItemsOnlyForeachArrayVariable($stmt);
+            if (! $assignVariable instanceof Expr) {
+                continue;
+            }
+
+            $directAssign = new Assign($assignVariable, $stmt->expr);
+            $node->stmts[$key] = new Expression($directAssign);
+
+            return $node;
         }
 
-        return new Assign($assignVariable, $node->expr);
+        return null;
     }
 
-    private function shouldSkip(Foreach_ $foreach, Scope $scope): bool
+    /**
+     * @param string[] $emptyArrayVariables
+     */
+    private function shouldSkip(Foreach_ $foreach, array $emptyArrayVariables): bool
     {
         $assignVariable = $this->foreachAnalyzer->matchAssignItemsOnlyForeachArrayVariable($foreach);
         if (! $assignVariable instanceof Expr) {
             return true;
         }
 
-        if ($this->shouldSkipAsPartOfOtherLoop($foreach)) {
-            return true;
-        }
-
-        $node = $this->nodeUsageFinder->findPreviousForeachNodeUsage($foreach, $assignVariable);
-        if (! $node instanceof Node) {
-            return true;
-        }
+        $foreachedExprType = $this->getType($foreach->expr);
 
-        $previousDeclarationParentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
-        if (! $previousDeclarationParentNode instanceof Assign) {
+        // only arrays, not traversable/iterable
+        if (! $foreachedExprType->isArray()->yes()) {
             return true;
         }
 
-        // must be empty array, otherwise it will false override
-        $defaultValue = $this->valueResolver->getValue($previousDeclarationParentNode->expr);
-        if ($defaultValue !== []) {
+        if ($this->shouldSkipAsPartOfOtherLoop($foreach)) {
             return true;
         }
 
-        $type = $scope->getType($foreach->expr);
-        return ! $type->isArray()
-            ->yes();
+        return ! $this->isNames($assignVariable, $emptyArrayVariables);
     }
 
     private function shouldSkipAsPartOfOtherLoop(Foreach_ $foreach): bool
     {
         $foreachParent = $this->betterNodeFinder->findParentByTypes($foreach, ControlStructure::LOOP_NODES);
         return $foreachParent instanceof Node;
     }
+
+    private function matchEmptyArrayVariableAssign(Stmt $stmt): ?string
+    {
+        if (! $stmt instanceof Expression) {
+            return null;
+        }
+
+        if (! $stmt->expr instanceof Assign) {
+            return null;
+        }
+
+        $assign = $stmt->expr;
+        if (! $assign->var instanceof Variable) {
+            return null;
+        }
+
+        // must be assign of empty array
+        if (! $this->valueResolver->isValue($assign->expr, [])) {
+            return null;
+        }
+
+        return $this->getName($assign->var);
+    }
 }

src/Kernel/RectorKernel.php

@@ -17,7 +17,7 @@ final class RectorKernel
     /**
      * @var string
      */
-    private const CACHE_KEY = 'v30';
+    private const CACHE_KEY = 'v31';
 
     private ContainerInterface|null $container = null;