Rollback ClassRenamePhpDocNodeVisitor to use clone $node to avoid unn…

…ecessary touch Identifier when no change (#3154) * Rollback ClassRenamePhpDocNodeVisitor to use clone $node to avoid unnecessary touch Identifier when no change * Fixed 🎉 * [ci-review] Rector Rectify * clean up * add auto import fqcn conflict same namespace * add deep namespace fixture Co-authored-by: GitHub Action <action@github.com>
rectorphp · Dec 4, 2022 · 25dec6b · 25dec6b
1 parent 16680c4
commit 25dec6b
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 13 deletions.
diff --git a/packages/NodeTypeResolver/PhpDocNodeVisitor/ClassRenamePhpDocNodeVisitor.php b/packages/NodeTypeResolver/PhpDocNodeVisitor/ClassRenamePhpDocNodeVisitor.php
@@ -16,6 +16,7 @@
 use PHPStan\Type\Type;
 use Rector\BetterPhpDocParser\ValueObject\PhpDocAttributeKey;
 use Rector\Core\Configuration\CurrentNodeProvider;
+use Rector\Core\Configuration\RectorConfigProvider;
 use Rector\Core\Exception\ShouldNotHappenException;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\Naming\Naming\UseImportsResolver;
@@ -39,7 +40,8 @@ public function __construct(
         private readonly CurrentNodeProvider $currentNodeProvider,
         private readonly UseImportsResolver $useImportsResolver,
         private readonly BetterNodeFinder $betterNodeFinder,
-        private readonly NodeNameResolver $nodeNameResolver
+        private readonly NodeNameResolver $nodeNameResolver,
+        private readonly RectorConfigProvider $rectorConfigProvider
     ) {
     }
 
@@ -66,12 +68,20 @@ public function enterNode(Node $node): ?Node
             return null;
         }
 
-        $node->name = $this->resolveNamespacedName($node, $phpParserNode, $node->name);
-        $staticType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType($node, $phpParserNode);
+        $identifier = clone $node;
+        $identifier->name = $this->resolveNamespacedName($identifier, $phpParserNode, $node->name);
+        $staticType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType($identifier, $phpParserNode);
+
+        $shouldImport = $this->rectorConfigProvider->shouldImportNames();
+        $isNoNamespacedName = ! str_starts_with($identifier->name, '\\') && substr_count($identifier->name, '\\') === 0;
+
+        // tweak overlapped import + rename
+        if ($shouldImport && $isNoNamespacedName) {
+            return null;
+        }
 
         // make sure to compare FQNs
         $objectType = $this->expandShortenedObjectType($staticType);
-
         foreach ($this->oldToNewTypes as $oldToNewType) {
             if (! $objectType->equals($oldToNewType->getOldType())) {
                 continue;
@@ -136,19 +146,14 @@ private function resolveNamespacedName(
         }
 
         $originalNode = $namespace->getAttribute(AttributeKey::ORIGINAL_NODE);
-
-        if (! $originalNode instanceof Namespace_) {
-            return $name;
-        }
-
         $namespaceName = (string) $this->nodeNameResolver->getName($namespace);
 
-        if (! $this->nodeNameResolver->isName($originalNode, $namespaceName)) {
+        if ($originalNode instanceof Namespace_ && ! $this->nodeNameResolver->isName($originalNode, $namespaceName)) {
             return $name;
         }
 
         if ($uses === []) {
-            return '\\' . ltrim($namespaceName . '\\' . $name, '\\');
+            return $namespaceName . '\\' . $name;
         }
 
         $nameFromUse = $this->resolveNamefromUse($uses, $name);
@@ -157,7 +162,7 @@ private function resolveNamespacedName(
             return $nameFromUse;
         }
 
-        return '\\' . ltrim($namespaceName . '\\' . $nameFromUse, '\\');
+        return $namespaceName . '\\' . $nameFromUse;
     }
 
     /**

diff --git a/...ssRector/FixtureAutoImportNames/auto_import_conflict_name_all_fqcn_same_namespace.php.inc b/...ssRector/FixtureAutoImportNames/auto_import_conflict_name_all_fqcn_same_namespace.php.inc
@@ -0,0 +1,32 @@
+<?php
+
+namespace Foo2;
+
+class AutoImportConflictNameAllFqcnSameNamespace extends FooBar
+{
+    /**
+     * @param \DateTime $foo
+     *
+     * @return \Foo2\Storage|\Storage
+     */
+    public function bar($foo){}
+}
+
+?>
+-----
+<?php
+
+namespace Foo2;
+
+use DateTime;
+class AutoImportConflictNameAllFqcnSameNamespace extends FooBar
+{
+    /**
+     * @param DateTime $foo
+     *
+     * @return Storage|\Illuminate\Support\Facades\Storage
+     */
+    public function bar($foo){}
+}
+
+?>
diff --git a/...r/FixtureAutoImportNames/auto_import_conflict_name_all_fqcn_same_namespace_inside.php.inc b/...r/FixtureAutoImportNames/auto_import_conflict_name_all_fqcn_same_namespace_inside.php.inc
@@ -0,0 +1,33 @@
+<?php
+
+namespace Foo2;
+
+class AutoImportConflictNameAllFqcnSameNamespaceInside extends FooBar
+{
+    /**
+     * @param \DateTime $foo
+     *
+     * @return \Foo2\Bar\Storage|\Storage
+     */
+    public function bar($foo){}
+}
+
+?>
+-----
+<?php
+
+namespace Foo2;
+
+use DateTime;
+use Foo2\Bar\Storage;
+class AutoImportConflictNameAllFqcnSameNamespaceInside extends FooBar
+{
+    /**
+     * @param DateTime $foo
+     *
+     * @return Storage|\Illuminate\Support\Facades\Storage
+     */
+    public function bar($foo){}
+}
+
+?>
diff --git a/...ferent_namespace_class_not_exists.php.inc → ...ferent_namespace_class_not_exists.php.inc b/...ferent_namespace_class_not_exists.php.inc → ...ferent_namespace_class_not_exists.php.inc
@@ -24,7 +24,7 @@ class Foo3 extends FooBar
     /**
      * @param DateTime $foo
      *
-     * @return \Foo3\Storage|\Illuminate\Support\Facades\Storage
+     * @return Storage|\Illuminate\Support\Facades\Storage
      */
     public function bar($foo){}
 }

diff --git a/rules-tests/Renaming/Rector/Name/RenameClassRector/Source/Foo2/Bar/Storage.php b/rules-tests/Renaming/Rector/Name/RenameClassRector/Source/Foo2/Bar/Storage.php
@@ -0,0 +1,8 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Foo2\Bar;
+
+class Storage
+{}