Repository: tighten/tlint (php)

Issue:
`FullyQualifiedFacades` formatter creates duplicate Model imports
Hi,  

When a file `use`s the `Eloquent` facade and `Illuminate\Database\Eloquent\Model`, the `FullyQualifiedFacades` formatter fixes the `Eloquent` facade import by importing `Illuminate\Database\Eloquent\Model` another time, resulting in a duplicate `use` statement.  

Example:  
Before fix:  
```php
use Eloquent;
use Exception;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Model;
```

Run `./vendor/bin/duster fix --using=tlint`  

After fix:  
```php
use Illuminate\Database\Eloquent\Model;
use Exception;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Model;
```  

I don't know if subsequent tools will fix this, but I suppose it's better if Tlint doesn't create invalid PHP code when formatting.  

Maybe just checking if a FQN import of the same class exists in the file would be enough.  

Let me know if you want me to give a short at a PR handling this.  

Cheers and thanks for your work on this project and Duster <3 

Pull request:
Prevent duplicate imports with fully qualified facades
This PR prevents the `FullyQualifiedFacades` formatter from adding duplicate import statements.


Closes #345

Patch:
diff --git a/src/Concerns/IdentifiesFacades.php b/src/Concerns/IdentifiesFacades.php
index e3e4af5..09796ef 100644
--- a/src/Concerns/IdentifiesFacades.php
+++ b/src/Concerns/IdentifiesFacades.php
@@ -4,7 +4,7 @@ namespace Tighten\TLint\Concerns;
 
 trait IdentifiesFacades
 {
-    private static $aliases = [
+    public static $aliases = [
         'App' => 'Illuminate\Support\Facades\App',
         'Arr' => 'Illuminate\Support\Arr',
         'Artisan' => 'Illuminate\Support\Facades\Artisan',
diff --git a/src/Formatters/FullyQualifiedFacades.php b/src/Formatters/FullyQualifiedFacades.php
index 142f662..1b3e020 100644
--- a/src/Formatters/FullyQualifiedFacades.php
+++ b/src/Formatters/FullyQualifiedFacades.php
@@ -5,11 +5,11 @@ namespace Tighten\TLint\Formatters;
 use PhpParser\Lexer;
 use PhpParser\Node;
 use PhpParser\Node\Name;
-use PhpParser\Node\Stmt\Namespace_;
 use PhpParser\Node\Stmt\Use_;
 use PhpParser\Node\Stmt\UseUse;
 use PhpParser\NodeTraverser;
 use PhpParser\NodeVisitor\CloningVisitor;
+use PhpParser\NodeVisitorAbstract;
 use PhpParser\Parser;
 use PhpParser\PrettyPrinter\Standard;
 use Tighten\TLint\BaseFormatter;
@@ -27,30 +27,92 @@ class FullyQualifiedFacades extends BaseFormatter
         $traverser->addVisitor(new CloningVisitor);
 
         $oldStmts = $parser->parse($this->code);
-        $oldTokens = $lexer->getTokens();
-
         $newStmts = $traverser->traverse($oldStmts);
 
-        if (count($newStmts) && $newStmts[0] instanceof Namespace_) {
-            $newStmts[0]->stmts = $this->transformFacadesToFullyQualified($newStmts[0]->stmts);
-        } else {
-            $newStmts = $this->transformFacadesToFullyQualified($newStmts);
-        }
+        $traverser = new NodeTraverser;
+        $traverser->addVisitor($currentFullyQualifiedFacadesVisitor = $this->currentFullyQualifiedFacadesVisitor());
+        $traverser->traverse($newStmts);
+
+        $traverser = new NodeTraverser;
+        $traverser->addVisitor($this->removeDuplicatesVisitor(
+            $currentFullyQualifiedFacadesVisitor->getCurrentFullyQualifiedFacades()
+        ));
+        $newStmts = $traverser->traverse($newStmts);
+
+        $traverser = new NodeTraverser;
+        $traverser->addVisitor($this->transformFacadesVisitor());
+        $newStmts = $traverser->traverse($newStmts);
+
+        return preg_replace('/\r?\n/', PHP_EOL, (new Standard)->printFormatPreserving($newStmts, $oldStmts, $lexer->getTokens()));
+    }
+
+    private function currentFullyQualifiedFacadesVisitor(): NodeVisitorAbstract
+    {
+        return new class extends NodeVisitorAbstract
+        {
+            public $currentFullyQualifiedFacades = [];
+
+            public function getCurrentFullyQualifiedFacades()
+            {
+                return $this->currentFullyQualifiedFacades;
+            }
 
-        return (new Standard)->printFormatPreserving($newStmts, $oldStmts, $oldTokens);
+            public function leaveNode(Node $node)
+            {
+                if ($node instanceof Use_
+                    && in_array((string) $node->uses[0]->name, FullyQualifiedFacades::$aliases)
+                ) {
+                    $this->currentFullyQualifiedFacades[] = (string) $node->uses[0]->name;
+                }
+            }
+        };
     }
 
-    private function transformFacadesToFullyQualified(array $stmts)
+    private function removeDuplicatesVisitor(array $currentFullyQualifiedFacades): NodeVisitorAbstract
     {
-        return array_map(function (Node $node) {
-            if (
-                $node instanceof Use_
-                && array_key_exists((string) $node->uses[0]->name, static::$aliases)
-            ) {
-                return new Use_([new UseUse(new Name(static::$aliases[(string) $node->uses[0]->name]))]);
+        return new class($currentFullyQualifiedFacades) extends NodeVisitorAbstract
+        {
+            private $currentFullyQualifiedFacades;
+
+            public function __construct(array $currentFullyQualifiedFacades)
+            {
+                $this->currentFullyQualifiedFacades = $currentFullyQualifiedFacades;
+            }
+
+            public function leaveNode(Node $node)
+            {
+                if (! $node instanceof Use_) {
+                    return null;
+                }
+
+                $qualifiedUseStatement = FullyQualifiedFacades::$aliases[(string) $node->uses[0]->name] ?? null;
+
+                if (in_array($qualifiedUseStatement, $this->currentFullyQualifiedFacades)) {
+                    return NodeTraverser::REMOVE_NODE;
+                }
             }
+        };
+    }
+
+    private function transformFacadesVisitor(): NodeVisitorAbstract
+    {
+        return new class extends NodeVisitorAbstract
+        {
+            public function enterNode(Node $node): Node|int|null
+            {
+                if (! $node instanceof Use_) {
+                    return null;
+                }
 
-            return $node;
-        }, $stmts);
+                $facades = FullyQualifiedFacades::$aliases;
+                $useStatement = (string) $node->uses[0]->name;
+
+                if (! array_key_exists($useStatement, $facades)) {
+                    return null;
+                }
+
+                return new Use_([new UseUse(new Name($facades[$useStatement]))]);
+            }
+        };
     }
 }


Test patch:
diff --git a/tests/Formatting/Formatters/FullyQualifiedFacadesTest.php b/tests/Formatting/Formatters/FullyQualifiedFacadesTest.php
index 4292c22..1a67e38 100644
--- a/tests/Formatting/Formatters/FullyQualifiedFacadesTest.php
+++ b/tests/Formatting/Formatters/FullyQualifiedFacadesTest.php
@@ -207,4 +207,35 @@ file;
 
         $this->assertSame($file, $formatted);
     }
+
+    /** @test */
+    public function it_does_not_cause_duplicate_import_statements()
+    {
+        $file = <<<'file'
+<?php
+
+namespace Test;
+
+use Eloquent;
+use Exception;
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+
+file;
+
+        $formatted = (new TFormat)->format(new FullyQualifiedFacades($file));
+
+        $expected = <<<'file'
+<?php
+
+namespace Test;
+
+use Exception;
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+
+file;
+
+        $this->assertSame($expected, $formatted);
+    }
 }