add CLOSURE_NODE to attributes, move closure use name conflicting to BreakingVariableRenameGuard

.phpstorm.meta.php

@@ -45,6 +45,7 @@ expectedArguments(
     \Rector\NodeTypeResolver\Node\AttributeKey::ORIGINAL_NAME,
     \Rector\NodeTypeResolver\Node\AttributeKey::COMMENTS,
     \Rector\NodeTypeResolver\Node\AttributeKey::VIRTUAL_NODE,
+    \Rector\NodeTypeResolver\Node\AttributeKey::CLOSURE_NODE,
 );
 
 expectedArguments(
@@ -75,6 +76,7 @@ expectedArguments(
     \Rector\NodeTypeResolver\Node\AttributeKey::ORIGINAL_NAME,
     \Rector\NodeTypeResolver\Node\AttributeKey::COMMENTS,
     \Rector\NodeTypeResolver\Node\AttributeKey::VIRTUAL_NODE,
+    \Rector\NodeTypeResolver\Node\AttributeKey::CLOSURE_NODE,
 );
 
 expectedArguments(

packages/node-type-resolver/src/Node/AttributeKey.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Rector\NodeTypeResolver\Node;
 
+use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Function_;
@@ -174,4 +175,9 @@ final class AttributeKey
      * @var string
      */
     public const DO_NOT_CHANGE = 'do_not_change';
+
+    /**
+     * @var string
+     */
+    public const CLOSURE_NODE = Closure::class;
 }

packages/node-type-resolver/src/NodeVisitor/FunctionMethodAndClassNodeVisitor.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Rector\NodeTypeResolver\NodeVisitor;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Name\FullyQualified;
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassLike;
@@ -56,6 +57,11 @@ final class FunctionMethodAndClassNodeVisitor extends NodeVisitorAbstract
      */
     private $function;
 
+    /**
+     * @var Closure|null
+     */
+    private $closure;
+
     /**
      * @var ClassNaming
      */
@@ -77,6 +83,7 @@ final class FunctionMethodAndClassNodeVisitor extends NodeVisitorAbstract
         $this->methodName = null;
         $this->classMethod = null;
         $this->function = null;
+        $this->closure = null;
 
         return null;
     }
@@ -89,6 +96,7 @@ final class FunctionMethodAndClassNodeVisitor extends NodeVisitorAbstract
         $this->processClass($node);
         $this->processMethod($node);
         $this->processFunction($node);
+        $this->processClosure($node);
 
         return $node;
     }
@@ -99,10 +107,12 @@ final class FunctionMethodAndClassNodeVisitor extends NodeVisitorAbstract
             $currentClass = array_pop($this->classStack);
             $this->setClassNodeAndName($currentClass);
         }
+
         if ($node instanceof ClassMethod) {
             $this->classMethod = array_pop($this->methodStack);
             $this->methodName = (string) $this->methodName;
         }
+
         return null;
     }
 
@@ -144,6 +154,15 @@ final class FunctionMethodAndClassNodeVisitor extends NodeVisitorAbstract
         $node->setAttribute(AttributeKey::FUNCTION_NODE, $this->function);
     }
 
+    private function processClosure(Node $node): void
+    {
+        if ($node instanceof Closure) {
+            $this->closure = $node;
+        }
+
+        $node->setAttribute(AttributeKey::CLOSURE_NODE, $this->closure);
+    }
+
     private function setClassNodeAndName(?ClassLike $classLike): void
     {
         $this->classLike = $classLike;

rules/naming/src/Guard/BreakingVariableRenameGuard.php

@@ -7,16 +7,20 @@ namespace Rector\Naming\Guard;
 use DateTimeInterface;
 use Nette\Utils\Strings;
 use PhpParser\Node;
+use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\If_;
 use PhpParser\Node\Stmt\Property;
+use PhpParser\NodeTraverser;
 use PHPStan\Analyser\Scope;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeWithClassName;
 use PHPStan\Type\UnionType;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
+use Rector\Core\PhpParser\NodeTraverser\CallableNodeTraverser;
 use Rector\Naming\Naming\ConflictingNameResolver;
 use Rector\Naming\Naming\OverridenExistingNamesResolver;
 use Rector\NodeNameResolver\NodeNameResolver;
@@ -53,11 +57,17 @@ final class BreakingVariableRenameGuard
      */
     private $nodeTypeResolver;
 
+    /**
+     * @var CallableNodeTraverser
+     */
+    private $callableNodeTraverser;
+
     public function __construct(
         BetterNodeFinder $betterNodeFinder,
         ConflictingNameResolver $conflictingNameResolver,
         OverridenExistingNamesResolver $overridenExistingNamesResolver,
         NodeNameResolver $nodeNameResolver,
+        CallableNodeTraverser $callableNodeTraverser,
         NodeTypeResolver $nodeTypeResolver
     ) {
         $this->betterNodeFinder = $betterNodeFinder;
@@ -65,12 +75,13 @@ final class BreakingVariableRenameGuard
         $this->overridenExistingNamesResolver = $overridenExistingNamesResolver;
         $this->nodeNameResolver = $nodeNameResolver;
         $this->nodeTypeResolver = $nodeTypeResolver;
+        $this->callableNodeTraverser = $callableNodeTraverser;
     }
 
     public function shouldSkipVariable(
         string $currentName,
         string $expectedName,
-        Node\FunctionLike $functionLike,
+        FunctionLike $functionLike,
         Variable $variable
     ): bool {
         // is the suffix? → also accepted
@@ -90,6 +101,10 @@ final class BreakingVariableRenameGuard
             return true;
         }
 
+        if ($this->isUsedInClosureUsesName($expectedName, $functionLike)) {
+            return true;
+        }
+
         return $this->isUsedInIfAndOtherBranches($variable, $currentName);
     }
 
@@ -221,4 +236,35 @@ final class BreakingVariableRenameGuard
 
         return $type;
     }
+
+    private function isUsedInClosureUsesName(string $expectedName, FunctionLike $functionLike): bool
+    {
+        if (! $functionLike instanceof Closure) {
+            return false;
+        }
+
+        return (bool) $this->hasVariableOfName((array) $functionLike->uses, $expectedName);
+    }
+
+    /**
+     * @param Node[] $nodes
+     */
+    private function hasVariableOfName(array $nodes, string $name): bool
+    {
+        $contains = false;
+        $this->callableNodeTraverser->traverseNodesWithCallable($nodes, function (Node $node) use (&$contains, $name) {
+            if (! $node instanceof Variable) {
+                return null;
+            }
+
+            if (! $this->nodeNameResolver->isName($node, $name)) {
+                return null;
+            }
+
+            $contains = true;
+            return NodeTraverser::STOP_TRAVERSAL;
+        });
+
+        return $contains;
+    }
 }

rules/naming/src/Rector/Assign/RenameVariableToMatchGetMethodNameRector.php

@@ -8,7 +8,6 @@ use PhpParser\Node;
 use PhpParser\Node\Expr\ArrowFunction;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\Closure;
-use PhpParser\Node\Expr\ClosureUse;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Param;
@@ -107,23 +106,23 @@ PHP
             return null;
         }
 
-        $classMethodOrFunction = $this->getCurrentFunctionLike($node);
+        $functionLike = $this->getCurrentFunctionLike($node);
-        if ($classMethodOrFunction === null) {
+        if ($functionLike === null) {
             return null;
         }
 
         if ($this->breakingVariableRenameGuard->shouldSkipVariable(
             $currentName,
             $newName,
-            $classMethodOrFunction,
+            $functionLike,
             $node->var
         )) {
             return null;
         }
 
-        if ($this->shouldSkipForNameConflict($node, $newName)) {
+//        if ($this->shouldSkipForNameConflict($node, $newName)) {
-            return null;
+//            return null;
-        }
+//        }
 
         return $this->renameVariable($node, $newName);
     }
@@ -167,43 +166,26 @@ PHP
         return $parentNode->getParams() ?? [];
     }
 
-    /**
+//    /**
-     * @return ClosureUse[]
+//     * @todo decouple to standalone service
-     */
+//     */
-    private function getParentNodeUses(Node $node): array
+//    private function shouldSkipForNameConflict(Assign $assign, string $newName): bool
-    {
+//    {
-        /** @var FunctionLike|null $parentNode */
+//        if ($this->skipOnConflictParamName($assign, $newName)) {
-        $parentNode = $this->getCurrentFunctionLike($node);
+//            return true;
-
+//        }
-        if ($parentNode === null || ! $parentNode instanceof Closure) {
+//
-            return [];
+//        return $this->skipOnConflictOtherVariable($assign, $newName);
-        }
+//    }
-
-        return $parentNode->uses ?? [];
-    }
 
     /**
-     * @todo decouple to standalone service
+     * @return ClassMethod|Function_|Closure|null
-     */
-    private function shouldSkipForNameConflict(Assign $assign, string $newName): bool
-    {
-        if ($this->skipOnConflictParamName($assign, $newName)) {
-            return true;
-        }
-
-        if ($this->skipOnConflictClosureUsesName($assign, $newName)) {
-            return true;
-        }
-
-        return $this->skipOnConflictOtherVariable($assign, $newName);
-    }
-
-    /**
-     * @return ClassMethod|Function_|null
      */
     private function getCurrentFunctionLike(Node $node): ?FunctionLike
     {
-        return $node->getAttribute(AttributeKey::METHOD_NODE) ?? $node->getAttribute(AttributeKey::FUNCTION_NODE);
+        return $node->getAttribute(AttributeKey::CLOSURE_NODE) ??
+             $node->getAttribute(AttributeKey::METHOD_NODE) ??
+             $node->getAttribute(AttributeKey::FUNCTION_NODE);
     }
 
     private function skipOnConflictParamName(Assign $assign, string $newName): bool
@@ -231,23 +213,6 @@ PHP
         return $skip;
     }
 
-    private function skipOnConflictClosureUsesName(Assign $assign, string $newName): bool
-    {
-        $skip = false;
-
-        $parentNodeUses = $this->getParentNodeUses($assign);
-        $this->traverseNodesWithCallable($parentNodeUses, function (Node $node) use ($newName, &$skip) {
-            if ($this->isVariableName($node, $newName)) {
-                $skip = true;
-                return NodeTraverser::STOP_TRAVERSAL;
-            }
-
-            return null;
-        });
-
-        return $skip;
-    }
-
     private function skipOnConflictOtherVariable(Assign $assign, string $newName): bool
     {
         $skip = false;