Merge pull request #347 from tighten/drift/fqcn-facades-duplicates

Prevent duplicate imports with fully qualified facades
tighten · Nov 17, 2023 · 4d126aa · 4d126aa
2 parents 8637bc3 + 95b1c17
commit 4d126aa
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 19 deletions.
diff --git a/src/Concerns/IdentifiesFacades.php b/src/Concerns/IdentifiesFacades.php
@@ -4,7 +4,7 @@
 
 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
@@ -5,11 +5,11 @@
 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 @@ public function format(Parser $parser, Lexer $lexer): string
         $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]))]);
+            }
+        };
     }
 }
diff --git a/tests/Formatting/Formatters/FullyQualifiedFacadesTest.php b/tests/Formatting/Formatters/FullyQualifiedFacadesTest.php
@@ -207,4 +207,35 @@ public function doStuff()
 
         $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);
+    }
 }