[CodingStyle] ImportsInClassCollection fixes

packages/CodingStyle/config/config.yaml

@@ -0,0 +1,8 @@
+services:
+    _defaults:
+        autowire: true
+        public: true
+
+    Rector\CodingStyle\:
+        resource: '../src'
+        exclude: '../src/{Rector/**/*Rector.php}'

packages/CodingStyle/src/Imports/ImportsInClassCollection.php

@@ -0,0 +1,60 @@
+<?php declare(strict_types=1);
+
+namespace Rector\CodingStyle\Imports;
+
+use Rector\CodingStyle\Naming\ClassNaming;
+
+final class ImportsInClassCollection
+{
+    /**
+     * @var string[]
+     */
+    private $importsInClass = [];
+
+    /**
+     * @var ClassNaming
+     */
+    private $classNaming;
+
+    public function __construct(ClassNaming $classNaming)
+    {
+        $this->classNaming = $classNaming;
+    }
+
+    public function addImport(string $import): void
+    {
+        $this->importsInClass[] = $import;
+    }
+
+    public function hasImport(string $import): bool
+    {
+        return in_array($import, $this->importsInClass, true);
+    }
+
+    public function canImportBeAdded(string $import): bool
+    {
+        $shortImport = $this->classNaming->getShortName($import);
+
+        foreach ($this->importsInClass as $importsInClass) {
+            $shortImportInClass = $this->classNaming->getShortName($importsInClass);
+            if ($importsInClass !== $import && $shortImportInClass === $shortImport) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    public function reset(): void
+    {
+        $this->importsInClass = [];
+    }
+
+    /**
+     * @return string[]
+     */
+    public function get(): array
+    {
+        return $this->importsInClass;
+    }
+}

packages/CodingStyle/src/Naming/ClassNaming.php

@@ -0,0 +1,13 @@
+<?php declare(strict_types=1);
+
+namespace Rector\CodingStyle\Naming;
+
+use Nette\Utils\Strings;
+
+final class ClassNaming
+{
+    public function getShortName(string $fullyQualifiedName): string
+    {
+        return Strings::after($fullyQualifiedName, '\\', -1) ?: $fullyQualifiedName;
+    }
+}

packages/CodingStyle/src/Rector/Namespace_/ImportFullyQualifiedNamesRector.php

@@ -9,6 +9,8 @@
 use PhpParser\Node\Stmt\Namespace_;
 use PhpParser\Node\Stmt\Use_;
 use PhpParser\Node\Stmt\UseUse;
+use Rector\CodingStyle\Imports\ImportsInClassCollection;
+use Rector\CodingStyle\Naming\ClassNaming;
 use Rector\Exception\ShouldNotHappenException;
 use Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer\DocBlockManipulator;
 use Rector\PhpParser\NodeTraverser\CallableNodeTraverser;
@@ -28,11 +30,6 @@ final class ImportFullyQualifiedNamesRector extends AbstractRector
      */
     private $newUseStatements = [];
 
-    /**
-     * @var string[]
-     */
-    private $alreadyImportedUses = [];
-
     /**
      * @var string[]
      */
@@ -43,10 +40,26 @@ final class ImportFullyQualifiedNamesRector extends AbstractRector
      */
     private $docBlockManipulator;
 
-    public function __construct(CallableNodeTraverser $callableNodeTraverser, DocBlockManipulator $docBlockManipulator)
-    {
+    /**
+     * @var ImportsInClassCollection
+     */
+    private $importsInClassCollection;
+
+    /**
+     * @var ClassNaming
+     */
+    private $classNaming;
+
+    public function __construct(
+        CallableNodeTraverser $callableNodeTraverser,
+        DocBlockManipulator $docBlockManipulator,
+        ImportsInClassCollection $importsInClassCollection,
+        ClassNaming $classNaming
+    ) {
         $this->callableNodeTraverser = $callableNodeTraverser;
         $this->docBlockManipulator = $docBlockManipulator;
+        $this->importsInClassCollection = $importsInClassCollection;
+        $this->classNaming = $classNaming;
     }
 
     public function getDefinition(): RectorDefinition
@@ -91,8 +104,9 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
-        $this->alreadyImportedUses = [];
         $this->newUseStatements = [];
+        $this->importsInClassCollection->reset();
+        $this->docBlockManipulator->resetImportedNames();
 
         /** @var Class_|null $class */
         $class = $this->betterNodeFinder->findFirstInstanceOf($node, Class_::class);
@@ -110,6 +124,17 @@ public function refactor(Node $node): ?Node
 
     private function resolveAlreadyImportedUses(Namespace_ $namespace): void
     {
+        /** @var Class_ $class */
+        $class = $this->betterNodeFinder->findFirstInstanceOf($namespace->stmts, Class_::class);
+
+        // add class itself
+        $className = $this->getName($class);
+        if ($className === null) {
+            return;
+        }
+
+        $this->importsInClassCollection->addImport($className);
+
         /** @var Use_[] $uses */
         $uses = $this->betterNodeFinder->find($namespace->stmts, function (Node $node) {
             if (! $node instanceof Use_) {
@@ -132,7 +157,7 @@ private function resolveAlreadyImportedUses(Namespace_ $namespace): void
                     $this->aliasedUses[] = $name;
                 }
 
-                $this->alreadyImportedUses[] = $name;
+                $this->importsInClassCollection->addImport($name);
             }
         }
 
@@ -145,7 +170,7 @@ private function resolveAlreadyImportedUses(Namespace_ $namespace): void
             return;
         }
 
-        $this->alreadyImportedUses[] = $className;
+        $this->importsInClassCollection->addImport($className);
     }
 
     /**
@@ -161,14 +186,14 @@ private function addNewUseStatements(Namespace_ $namespace, array $newUseStateme
 
         foreach ($newUseStatements as $newUseStatement) {
             // already imported in previous cycle
-            if (in_array($newUseStatement, $this->alreadyImportedUses, true)) {
+            if ($this->importsInClassCollection->hasImport($newUseStatement)) {
                 continue;
             }
 
             $useUse = new UseUse(new Name($newUseStatement));
             $newUses[] = new Use_([$useUse]);
 
-            $this->alreadyImportedUses[] = $newUseStatement;
+            $this->importsInClassCollection->addImport($newUseStatement);
         }
 
         $namespace->stmts = array_merge($newUses, $namespace->stmts);
@@ -179,6 +204,7 @@ private function addNewUseStatements(Namespace_ $namespace, array $newUseStateme
      */
     private function importNamesAndCollectNewUseStatements(Class_ $class): array
     {
+        // probably anonymous class
         if ($class->name === null) {
             return [];
         }
@@ -213,7 +239,7 @@ private function importNamesAndCollectNewUseStatements(Class_ $class): array
                     return null;
                 }
 
-                $shortName = $this->getShortName($fullyQualifiedName);
+                $shortName = $this->classNaming->getShortName($fullyQualifiedName);
                 if (isset($this->newUseStatements[$shortName])) {
                     if ($fullyQualifiedName === $this->newUseStatements[$shortName]) {
                         return new Name($shortName);
@@ -222,7 +248,7 @@ private function importNamesAndCollectNewUseStatements(Class_ $class): array
                     return null;
                 }
 
-                if (! in_array($fullyQualifiedName, $this->alreadyImportedUses, true)) {
+                if (! $this->importsInClassCollection->hasImport($fullyQualifiedName)) {
                     $this->newUseStatements[$shortName] = $fullyQualifiedName;
                 }
 
@@ -237,18 +263,13 @@ private function importNamesAndCollectNewUseStatements(Class_ $class): array
 
         // for doc blocks
         $this->callableNodeTraverser->traverseNodesWithCallable([$class], function (Node $node): void {
-            $importedDocUseStatements = $this->docBlockManipulator->importNames($node, $this->alreadyImportedUses);
+            $importedDocUseStatements = $this->docBlockManipulator->importNames($node);
             $this->newUseStatements = array_merge($this->newUseStatements, $importedDocUseStatements);
         });
 
         return $this->newUseStatements;
     }
 
-    private function getShortName(string $fullyQualifiedName): string
-    {
-        return Strings::after($fullyQualifiedName, '\\', -1) ?: $fullyQualifiedName;
-    }
-
     // 1. name is fully qualified → import it
     private function shouldSkipName(string $fullyQualifiedName): bool
     {
@@ -257,20 +278,13 @@ private function shouldSkipName(string $fullyQualifiedName): bool
             return true;
         }
 
-        $shortName = $this->getShortName($fullyQualifiedName);
+        $shortName = $this->classNaming->getShortName($fullyQualifiedName);
 
         // nothing to change
         if ($shortName === $fullyQualifiedName) {
             return true;
         }
 
-        foreach ($this->alreadyImportedUses as $alreadyImportedUse) {
-            $shortAlreadyImportedUsed = $this->getShortName($alreadyImportedUse);
-            if ($alreadyImportedUse !== $fullyQualifiedName && $shortAlreadyImportedUsed === $shortName) {
-                return true;
-            }
-        }
-
-        return false;
+        return $this->importsInClassCollection->canImportBeAdded($fullyQualifiedName);
     }
 }

packages/CodingStyle/tests/Rector/Namespace_/ImportFullyQualifiedNamesRector/Fixture/keep_same_end.php.inc

@@ -7,9 +7,33 @@ use Some\Trait_;
 final class SameEnd
 {
     /**
-     * @param Another\Trait_ $phpStanScopeFactory
+     * @param \Some\Trait_ $firstTrait
+     * @param Another\Trait_ $secondTrait
+     * @param \Some\Trait_ $thirdTrait
      */
-    public function __construct(Another\Trait_ $phpStanScopeFactory)
+    public function __construct(\Some\Trait_ $firstTrait, Another\Trait_ $secondTrait, \Some\Trait_ $thirdTrait)
     {
     }
 }
+
+?>
+-----
+<?php
+
+namespace Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\Fixture;
+
+use Some\Trait_;
+
+final class SameEnd
+{
+    /**
+     * @param Trait_ $firstTrait
+     * @param Another\Trait_ $secondTrait
+     * @param Trait_ $thirdTrait
+     */
+    public function __construct(Trait_ $firstTrait, Another\Trait_ $secondTrait, Trait_ $thirdTrait)
+    {
+    }
+}
+
+?>

packages/CodingStyle/tests/Rector/Namespace_/ImportFullyQualifiedNamesRector/Fixture/many_imports.php.inc

@@ -0,0 +1,38 @@
+<?php
+
+namespace Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source;
+
+final class StockRepository
+{
+    public function filter(Stock\Query $query)
+    {
+        /** @var Stock\Query $query */
+        $query = 5;
+
+        /** @var Querying\Query $query */
+        $builder = $this->createBuilder();
+        $query->build($builder);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source;
+
+use Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source\Stock\Query;
+final class StockRepository
+{
+    public function filter(Query $query)
+    {
+        /** @var Query $query */
+        $query = 5;
+
+        /** @var Querying\Query $query */
+        $builder = $this->createBuilder();
+        $query->build($builder);
+    }
+}
+
+?>

packages/CodingStyle/tests/Rector/Namespace_/ImportFullyQualifiedNamesRector/ImportFullyQualifiedNamesRectorTest.php

@@ -25,6 +25,9 @@ public function test(): void
             __DIR__ . '/Fixture/doc_combined.php.inc',
             __DIR__ . '/Fixture/conflicting_endings.php.inc',
             __DIR__ . '/Fixture/already_class_name_in_param_doc.php.inc',
+
+            // buggy
+            __DIR__ . '/Fixture/many_imports.php.inc',
         ]);
     }
 

packages/CodingStyle/tests/Rector/Namespace_/ImportFullyQualifiedNamesRector/Source/Querying/Query.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source\Querying;
+
+final class Query
+{
+
+}

packages/CodingStyle/tests/Rector/Namespace_/ImportFullyQualifiedNamesRector/Source/Stock/Query.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\CodingStyle\Tests\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source\Stock;
+
+final class Query
+{
+
+}