[Naming] Remove parent lookup on ForeachMatcher (#4293)

* [Naming] Remove parent lookup on ForeachMatcher * [ci-review] Rector Rectify * fix phpstan --------- Co-authored-by: GitHub Action <actions@github.com>
rectorphp · Jun 19, 2023 · 70287f1 · 70287f1
1 parent 253b4f6
commit 70287f1
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 38 deletions.
diff --git a/rules/Naming/Matcher/ForeachMatcher.php b/rules/Naming/Matcher/ForeachMatcher.php
@@ -9,7 +9,6 @@
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Foreach_;
 use PhpParser\Node\Stmt\Function_;
-use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\Naming\ValueObject\VariableAndCallForeach;
 use Rector\NodeNameResolver\NodeNameResolver;
 
@@ -18,11 +17,10 @@ final class ForeachMatcher
     public function __construct(
         private readonly NodeNameResolver $nodeNameResolver,
         private readonly CallMatcher $callMatcher,
-        private readonly BetterNodeFinder $betterNodeFinder,
     ) {
     }
 
-    public function match(Foreach_ $foreach): ?VariableAndCallForeach
+    public function match(Foreach_ $foreach, ClassMethod|Closure|Function_ $functionLike): ?VariableAndCallForeach
     {
         $call = $this->callMatcher->matchCall($foreach);
         if ($call === null) {
@@ -33,24 +31,11 @@ public function match(Foreach_ $foreach): ?VariableAndCallForeach
             return null;
         }
 
-        $functionLike = $this->getFunctionLike($foreach);
-        if ($functionLike === null) {
-            return null;
-        }
-
         $variableName = $this->nodeNameResolver->getName($foreach->valueVar);
         if ($variableName === null) {
             return null;
         }
 
         return new VariableAndCallForeach($foreach->valueVar, $call, $variableName, $functionLike);
     }
-
-    private function getFunctionLike(Foreach_ $foreach): ClassMethod | Function_ | Closure | null
-    {
-        return $this->betterNodeFinder->findParentByTypes(
-            $foreach,
-            [Closure::class, ClassMethod::class, Function_::class]
-        );
-    }
 }
diff --git a/rules/Naming/Rector/Foreach_/RenameForeachValueVariableToMatchMethodCallReturnTypeRector.php b/rules/Naming/Rector/Foreach_/RenameForeachValueVariableToMatchMethodCallReturnTypeRector.php
@@ -5,7 +5,12 @@
 namespace Rector\Naming\Rector\Foreach_;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\Closure;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Foreach_;
+use PhpParser\Node\Stmt\Function_;
+use PhpParser\NodeTraverser;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Naming\Guard\BreakingVariableRenameGuard;
 use Rector\Naming\Matcher\ForeachMatcher;
@@ -72,37 +77,63 @@ public function run()
      */
     public function getNodeTypes(): array
     {
-        return [Foreach_::class];
+        return [ClassMethod::class, Closure::class, Function_::class];
     }
 
     /**
-     * @param Foreach_ $node
+     * @param ClassMethod|Closure|Function_ $node
      */
     public function refactor(Node $node): ?Node
     {
-        $variableAndCallForeach = $this->foreachMatcher->match($node);
-        if (! $variableAndCallForeach instanceof VariableAndCallForeach) {
+        if ($node->stmts === null) {
             return null;
         }
 
-        $expectedName = $this->expectedNameResolver->resolveForForeach($variableAndCallForeach);
-        if ($expectedName === null) {
-            return null;
-        }
-
-        if ($this->isName($variableAndCallForeach->getVariable(), $expectedName)) {
-            return null;
-        }
-
-        if ($this->shouldSkip($variableAndCallForeach, $expectedName)) {
-            return null;
-        }
-
-        $hasRenamed = $this->variableRenamer->renameVariableInFunctionLike(
-            $variableAndCallForeach->getFunctionLike(),
-            $variableAndCallForeach->getVariableName(),
-            $expectedName,
-            null
+        $hasRenamed = false;
+        $this->traverseNodesWithCallable(
+            $node->stmts,
+            function (Node $subNode) use ($node, &$hasRenamed): ?int {
+                if ($subNode instanceof Class_ || $subNode instanceof Closure || $subNode instanceof Function_) {
+                    return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
+                }
+
+                if (! $subNode instanceof Foreach_) {
+                    return null;
+                }
+
+                $variableAndCallForeach = $this->foreachMatcher->match($subNode, $node);
+                if (! $variableAndCallForeach instanceof VariableAndCallForeach) {
+                    return null;
+                }
+
+                $expectedName = $this->expectedNameResolver->resolveForForeach($variableAndCallForeach);
+                if ($expectedName === null) {
+                    return null;
+                }
+
+                if ($this->isName($variableAndCallForeach->getVariable(), $expectedName)) {
+                    return null;
+                }
+
+                if ($this->shouldSkip($variableAndCallForeach, $expectedName)) {
+                    return null;
+                }
+
+                $hasChanged = $this->variableRenamer->renameVariableInFunctionLike(
+                    $variableAndCallForeach->getFunctionLike(),
+                    $variableAndCallForeach->getVariableName(),
+                    $expectedName,
+                    null
+                );
+
+                // use different variable on purpose to avoid variable re-assign back to false
+                // after go to other method
+                if ($hasChanged) {
+                    $hasRenamed = true;
+                }
+
+                return null;
+            }
         );
 
         if ($hasRenamed) {