Repository: tighten/tlint (php)

Issue:
`File` class in current namespace mistaken for Facade alias
<img width="384" alt="image" src="https://user-images.githubusercontent.com/18192441/128521275-351bafde-ab2e-4a90-93d8-f0e21a1caa7b.png">


Pull request description:
Fix import facades
This PR aims to fix #251 and #252.

- Removes `Linters\ImportFacades::class` from both Tighten and Laravel presets
- Adds new `Tighten\TLint\Linters\FullyQualifiedFacades` linter
- Adds new `Tighten\TLint\Formatters\FullyQualifiedFacades` formatter

The `FullyQualifiedFacades` forces `use Illuminate/Support/Facades/Cache;` over `use Cache;`

**This only affects `use` statements.**

## Linter

- Triggers on facade alias use (`use Cache;`)
- Does not trigger when an "alias" is used without an import statement
  - This avoids false positives where you have a file in the same directory named the same as a facade
- Does not trigger when the facade is correctly imported (`use Illuminate/Support/Facades/Cache;`)

```php
<?php

use Illuminate\Support\Facades\Cache;

$abc = Cache::get('abc');
```

```php
<?php

namespace App\Utilities;

use Illuminate\Support\Facades\Cache;

class SuperCacher
{
    public function doThing()
    {
        return Cache::get('abc');
    }
}
```

## Formatter

- Updates aliases to fully qualified facades
  - `use DB;` => `use Illuminate\Support\Facades\DB;`


Patch:
diff --git a/readme.md b/readme.md
index fe5d7a2..48f5ba0 100644
--- a/readme.md
+++ b/readme.md
@@ -195,6 +195,7 @@ The default configuration is "tighten", but you may change this by adding a `tfo
 | `ClassThingsOrder` | Class "things" should follow the ordering presented in the [handbook](https://gist.github.com/mattstauffer/1178946cb585b17a3941dd0edcbce0c4). |
 | `ConcatenationNoSpacing` | There should be no space around `.` concatenations, and additional lines should always start with a `.` |
 | `ConcatenationSpacing` | There should be 1 space around `.` concatenations, and additional lines should always start with a `.` |
+| `FullyQualifiedFacades` | Import facades using their full namespace. |
 | `ImportFacades` | Import facades (don't use aliases). |
 | `MailableMethodsInBuild` | Mailable values (from and subject etc) should be set in build(). |
 | `ModelMethodOrder` | Model method order should be: booting > boot > booted > custom_static > relationships > scopes > accessors > mutators > custom |
@@ -203,7 +204,7 @@ The default configuration is "tighten", but you may change this by adding a `tfo
 | `NoDatesPropertyOnModels` | The `$dates` property was deprecated in Laravel 8. Use `$casts` instead. |
 | `NoDocBlocksForMigrationUpDown` | Remove doc blocks from the up and down method in migrations. |
 | `NoDump` | There should be no calls to `dd()`, `dump()`, `ray()`, or `var_dump()` |
-| `NoInlineVarDocs` | No /\*_ @var ClassName $var _/ inline docs. [ref](https://github.com/tighten/tlint/issues/108) |
+| `NoInlineVarDocs` | No /** @var ClassName $var */ inline docs. [ref](https://github.com/tighten/tlint/issues/108) |
 | `NoJsonDirective` | Use blade `{{ $model }}` auto escaping for models, and double quotes via json_encode over @json blade directive: `<vue-comp :values='@json($var)'>` -> `<vue-comp :values="{{ $model }}">` OR `<vue-comp :values="{{ json_encode($var) }}">` |
 | `NoLeadingSlashesOnRoutePaths` | No leading slashes on route paths. |
 | `NoMethodVisibilityInTests` | There should be no method visibility in test methods. [ref](https://github.com/tighten/tlint/issues/106#issuecomment-537952774) |
@@ -253,6 +254,7 @@ The default configuration is "tighten", but you may change this by adding a `tfo
 
 - `ApplyMiddlewareInRoutes`
 - `ArrayParametersOverViewWith`
+- `FullyQualifiedFacades`
 - `ImportFacades`
 - `MailableMethodsInBuild`
 - `NoLeadingSlashesOnRoutePaths`
@@ -282,7 +284,7 @@ The default configuration is "tighten", but you may change this by adding a `tfo
 | --- | --- |
 | `AlphabeticalImports` | Alphabetizes import statements. |
 | `ExcessSpaceBetweenAndAfterImports` | Removes excess newlines around use statements. |
-| `ImportFacades` | Import facades using their full namespace. |
+| `FullyQualifiedFacades` | Import facades using their full namespace. |
 | `NewLineAtEndOfFile` | Applies a newline at the end of files. |
 | `NoDatesPropertyOnModels` | Use `$casts` instead of `$dates` on Eloquent models. |
 | `NoDocBlocksForMigrationUpDown` | Removes doc blocks from the up and down method in migrations. |
@@ -298,7 +300,7 @@ The default configuration is "tighten", but you may change this by adding a `tfo
 
 ### Laravel
 
-- `ImportFacades`
+- `FullyQualifiedFacades`
 - `NoDatesPropertyOnModels`
 - `NoDocBlocksForMigrationUpDown`
 - `UseAnonymousMigrations`
diff --git a/src/Formatters/ImportFacades.php b/src/Formatters/FullyQualifiedFacades.php
similarity index 56%
rename from src/Formatters/ImportFacades.php
rename to src/Formatters/FullyQualifiedFacades.php
index 0990203..1d48576 100644
--- a/src/Formatters/ImportFacades.php
+++ b/src/Formatters/FullyQualifiedFacades.php
@@ -5,6 +5,7 @@ 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;
@@ -14,7 +15,7 @@ use PhpParser\PrettyPrinter\Standard;
 use Tighten\TLint\BaseFormatter;
 use Tighten\TLint\Concerns\IdentifiesFacades;
 
-class ImportFacades extends BaseFormatter
+class FullyQualifiedFacades extends BaseFormatter
 {
     use IdentifiesFacades;
 
@@ -25,10 +26,23 @@ class ImportFacades extends BaseFormatter
         $traverser = new NodeTraverser;
         $traverser->addVisitor(new CloningVisitor);
 
-        $originalStatements = $parser->parse($this->code);
-        $tokens = $lexer->getTokens();
+        $oldStmts = $parser->parse($this->code);
+        $oldTokens = $lexer->getTokens();
 
-        $statements = array_map(function (Node $node) {
+        $newStmts = $traverser->traverse($oldStmts);
+
+        if (count($newStmts) && $newStmts[0] instanceof Namespace_) {
+            $newStmts[0]->stmts = $this->transformFacadesToFullyQualified($newStmts[0]->stmts);
+        } else {
+            $newStmts = $this->transformFacadesToFullyQualified($newStmts);
+        }
+
+        return (new Standard)->printFormatPreserving($newStmts, $oldStmts, $oldTokens);
+    }
+
+    private function transformFacadesToFullyQualified(array $stmts)
+    {
+        return array_map(function (Node $node) {
             if (
                 $node instanceof Use_
                 && array_key_exists((string) $node->uses[0]->name, static::$aliases)
@@ -37,8 +51,6 @@ class ImportFacades extends BaseFormatter
             }
 
             return $node;
-        }, $traverser->traverse($originalStatements));
-
-        return (new Standard)->printFormatPreserving($statements, $originalStatements, $tokens);
+        }, $stmts);
     }
 }
diff --git a/src/Linters/FullyQualifiedFacades.php b/src/Linters/FullyQualifiedFacades.php
new file mode 100644
index 0000000..1b5093f
--- /dev/null
+++ b/src/Linters/FullyQualifiedFacades.php
@@ -0,0 +1,52 @@
+<?php
+
+namespace Tighten\TLint\Linters;
+
+use PhpParser\Node;
+use PhpParser\NodeTraverser;
+use PhpParser\NodeVisitor\FindingVisitor;
+use PhpParser\Parser;
+use Tighten\TLint\BaseLinter;
+use Tighten\TLint\Concerns\IdentifiesFacades;
+
+class FullyQualifiedFacades extends BaseLinter
+{
+    use IdentifiesFacades;
+
+    public const description = "Import facades using their full namespace.";
+
+    public function lint(Parser $parser)
+    {
+        $traverser = new NodeTraverser;
+
+        $visitor = new FindingVisitor(function (Node $node) {
+            static $groupUse = [];
+
+            /**
+             * Build an array of group use statement classes (groups use FQNs)
+             * Illuminate\Support\Facades\{Config, Hash} => ['Config', 'Hash']
+             */
+            if ($node instanceof Node\Stmt\GroupUse) {
+                foreach ($node->uses as $use) {
+                    $groupUse[] = $use->name->toString();
+                }
+            }
+
+            /**
+             * Check if the node is a use statement and not a group use statement
+             * Return if use statement is a facade or not
+             */
+            if ($node instanceof Node\Stmt\UseUse && ! in_array($node->name->toString(), $groupUse)) {
+                return in_array($node->name->toString(), array_keys(static::$aliases));
+            }
+
+            return false;
+        });
+
+        $traverser->addVisitor($visitor);
+
+        $traverser->traverse($parser->parse($this->code));
+
+        return $visitor->getFoundNodes();
+    }
+}
diff --git a/src/Presets/LaravelPreset.php b/src/Presets/LaravelPreset.php
index 2b37638..729286d 100644
--- a/src/Presets/LaravelPreset.php
+++ b/src/Presets/LaravelPreset.php
@@ -15,7 +15,6 @@ class LaravelPreset implements PresetInterface
             Linters\ArrayParametersOverViewWith::class,
             Linters\ClassThingsOrder::class,
             Linters\ConcatenationNoSpacing::class,
-            Linters\ImportFacades::class,
             Linters\MailableMethodsInBuild::class,
             Linters\NewLineAtEndOfFile::class,
             Linters\NoCompact::class,
diff --git a/src/Presets/TightenPreset.php b/src/Presets/TightenPreset.php
index 0a47585..c140ab9 100644
--- a/src/Presets/TightenPreset.php
+++ b/src/Presets/TightenPreset.php
@@ -15,7 +15,6 @@ class TightenPreset implements PresetInterface
             Linters\ArrayParametersOverViewWith::class,
             Linters\ClassThingsOrder::class,
             Linters\ConcatenationSpacing::class,
-            Linters\ImportFacades::class,
             Linters\MailableMethodsInBuild::class,
             Linters\NewLineAtEndOfFile::class,
             Linters\NoCompact::class,


Test patch:
diff --git a/tests/Formatting/Formatters/FullyQualifiedFacadesTest.php b/tests/Formatting/Formatters/FullyQualifiedFacadesTest.php
new file mode 100644
index 0000000..4292c22
--- /dev/null
+++ b/tests/Formatting/Formatters/FullyQualifiedFacadesTest.php
@@ -0,0 +1,210 @@
+<?php
+
+namespace Tests\Formatting\Formatters;
+
+use PHPUnit\Framework\TestCase;
+use Tighten\TLint\Formatters\FullyQualifiedFacades;
+use Tighten\TLint\TFormat;
+
+class FullyQualifiedFacadesTest extends TestCase
+{
+    /** @test */
+    public function fixes_facade_aliases()
+    {
+        $file = <<<'file'
+<?php
+
+namespace Test;
+
+use DB;
+use Storage;
+
+file;
+
+        $formatted = (new TFormat)->format(new FullyQualifiedFacades($file));
+
+        $expected = <<<'file'
+<?php
+
+namespace Test;
+
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Storage;
+
+file;
+
+        $this->assertSame($expected, $formatted);
+    }
+
+    /** @test */
+    public function fixes_facade_aliases_when_file_not_namespaced()
+    {
+        $file = <<<'file'
+<?php
+
+use DB;
+use Storage;
+
+file;
+
+        $formatted = (new TFormat)->format(new FullyQualifiedFacades($file));
+
+        $expected = <<<'file'
+<?php
+
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Storage;
+
+file;
+
+        $this->assertSame($expected, $formatted);
+    }
+
+    /** @test */
+    public function fixes_facade_aliases_mixed_with_valid_use_statements()
+    {
+        $file = <<<'file'
+<?php
+
+namespace Test;
+
+use DB;
+use Illuminate\Http\Request;
+use Illuminate\Support\Facades\Schema;
+use Storage;
+use Tests\TestCase;
+
+file;
+
+        $formatted = (new TFormat)->format(new FullyQualifiedFacades($file));
+
+        $expected = <<<'file'
+<?php
+
+namespace Test;
+
+use Illuminate\Support\Facades\DB;
+use Illuminate\Http\Request;
+use Illuminate\Support\Facades\Schema;
+use Illuminate\Support\Facades\Storage;
+use Tests\TestCase;
+
+file;
+
+        $this->assertSame($expected, $formatted);
+    }
+
+    /** @test */
+    public function fixes_facade_aliases_after_group_use_when_file_not_namespaced()
+    {
+        $file = <<<'file'
+<?php
+
+use Illuminate\Support\Facades\{Config, Hash};
+use Illuminate\Http\Request;
+use Illuminate\Support\Facades\Schema;
+use Storage;
+use Tests\TestCase;
+
+file;
+
+        $formatted = (new TFormat)->format(new FullyQualifiedFacades($file));
+
+        $expected = <<<'file'
+<?php
+
+use Illuminate\Support\Facades\{Config, Hash};
+use Illuminate\Http\Request;
+use Illuminate\Support\Facades\Schema;
+use Illuminate\Support\Facades\Storage;
+use Tests\TestCase;
+
+file;
+
+        $this->assertSame($expected, $formatted);
+    }
+
+    /** @test */
+    public function fixes_str_and_arr_helpers()
+    {
+        $file = <<<'file'
+<?php
+
+namespace Test;
+
+use Arr;
+use Str;
+
+file;
+
+        $formatted = (new TFormat)->format(new FullyQualifiedFacades($file));
+
+        $expected = <<<'file'
+<?php
+
+namespace Test;
+
+use Illuminate\Support\Arr;
+use Illuminate\Support\Str;
+
+file;
+
+        $this->assertSame($expected, $formatted);
+    }
+
+    /** @test */
+    public function ignores_grouped_imports()
+    {
+        $file = <<<'file'
+<?php
+
+namespace Test;
+
+use App\{ User, Post };
+
+file;
+
+        $formatted = (new TFormat)->format(new FullyQualifiedFacades($file));
+
+        $this->assertSame($file, $formatted);
+    }
+
+    /** @test */
+    public function ignores_unknown_aliases()
+    {
+        $file = <<<'file'
+<?php
+
+namespace Test;
+
+use Shortcut;
+
+file;
+
+        $formatted = (new TFormat)->format(new FullyQualifiedFacades($file));
+
+        $this->assertSame($file, $formatted);
+    }
+
+    /** @test */
+    public function ignores_files_in_same_directory_with_same_name_as_facade_alias()
+    {
+        $file = <<<'file'
+<?php
+
+namespace App\Utilities;
+
+class Stuff
+{
+    public function doStuff()
+    {
+        return File::otherStuff();
+    }
+}
+file;
+
+        $formatted = (new TFormat)->format(new FullyQualifiedFacades($file));
+
+        $this->assertSame($file, $formatted);
+    }
+}
diff --git a/tests/Formatting/Formatters/ImportFacadesTest.php b/tests/Formatting/Formatters/ImportFacadesTest.php
deleted file mode 100644
index 96443b6..0000000
--- a/tests/Formatting/Formatters/ImportFacadesTest.php
+++ /dev/null
@@ -1,126 +0,0 @@
-<?php
-
-namespace Tests\Formatting\Formatters;
-
-use PHPUnit\Framework\TestCase;
-use Tighten\TLint\Formatters\ImportFacades;
-use Tighten\TLint\TFormat;
-
-class ImportFacadesTest extends TestCase
-{
-    /** @test */
-    function fixes_facade_aliases()
-    {
-        $file = <<<'file'
-<?php
-
-use DB;
-use Storage;
-
-return true;
-file;
-
-        $formatted = (new TFormat)->format(new ImportFacades($file));
-
-        $expected = <<<'file'
-<?php
-
-use Illuminate\Support\Facades\DB;
-use Illuminate\Support\Facades\Storage;
-
-return true;
-file;
-
-        $this->assertSame($expected, $formatted);
-    }
-
-    /** @test */
-    function fixes_facade_aliases_mixed_with_valid_use_statements()
-    {
-        $file = <<<'file'
-<?php
-
-use DB;
-use Illuminate\Http\Request;
-use Illuminate\Support\Facades\Schema;
-use Storage;
-use Tests\TestCase;
-
-return true;
-file;
-
-        $formatted = (new TFormat)->format(new ImportFacades($file));
-
-        $expected = <<<'file'
-<?php
-
-use Illuminate\Support\Facades\DB;
-use Illuminate\Http\Request;
-use Illuminate\Support\Facades\Schema;
-use Illuminate\Support\Facades\Storage;
-use Tests\TestCase;
-
-return true;
-file;
-
-        $this->assertSame($expected, $formatted);
-    }
-
-    /** @test */
-    function fixes_str_and_arr_helpers()
-    {
-        $file = <<<'file'
-<?php
-
-use Arr;
-use Str;
-
-return true;
-file;
-
-        $formatted = (new TFormat)->format(new ImportFacades($file));
-
-        $expected = <<<'file'
-<?php
-
-use Illuminate\Support\Arr;
-use Illuminate\Support\Str;
-
-return true;
-file;
-
-        $this->assertSame($expected, $formatted);
-    }
-
-    /** @test */
-    function ignores_grouped_imports()
-    {
-        $file = <<<'file'
-<?php
-
-use App\{ User, Post };
-
-return true;
-file;
-
-        $formatted = (new TFormat)->format(new ImportFacades($file));
-
-        $this->assertSame($file, $formatted);
-    }
-
-    /** @test */
-    function ignores_unknown_aliases()
-    {
-        $file = <<<'file'
-<?php
-
-use Shortcut;
-
-return true;
-file;
-
-        $formatted = (new TFormat)->format(new ImportFacades($file));
-
-        $this->assertSame($file, $formatted);
-    }
-}
diff --git a/tests/Linting/Linters/FullyQualifiedFacadesTest.php b/tests/Linting/Linters/FullyQualifiedFacadesTest.php
new file mode 100644
index 0000000..4c8bd8b
--- /dev/null
+++ b/tests/Linting/Linters/FullyQualifiedFacadesTest.php
@@ -0,0 +1,201 @@
+<?php
+
+namespace Tests\Linting\Linters;
+
+use PHPUnit\Framework\TestCase;
+use Tighten\TLint\Linters\FullyQualifiedFacades;
+use Tighten\TLint\TLint;
+
+class FullyQualifiedFacadesTest extends TestCase
+{
+    /** @test */
+    public function it_triggers_on_namespaced_file()
+    {
+        $file = <<<file
+<?php
+
+namespace Test;
+
+use Cache;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new FullyQualifiedFacades($file)
+        );
+
+        $this->assertEquals(5, $lints[0]->getNode()->getLine());
+    }
+
+    /** @test */
+    public function it_triggers_on_non_namespaced_file()
+    {
+        $file = <<<file
+<?php
+
+use Cache;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new FullyQualifiedFacades($file)
+        );
+
+        $this->assertEquals(3, $lints[0]->getNode()->getLine());
+    }
+
+    /** @test */
+    public function it_triggers_on_aliased_facade()
+    {
+        $file = <<<file
+<?php
+
+use Cache as Store;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new FullyQualifiedFacades($file)
+        );
+
+        $this->assertEquals(3, $lints[0]->getNode()->getLine());
+    }
+
+    /** @test */
+    public function does_not_trigger_on_a_fully_qualified_facade()
+    {
+        $file = <<<file
+<?php
+
+namespace Test;
+
+use Illuminate\Support\Facades\Hash;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new FullyQualifiedFacades($file)
+        );
+
+        $this->assertEmpty($lints);
+    }
+
+    /** @test */
+    public function does_not_trigger_on_custom_class_aliased_to_facade_name()
+    {
+        $file = <<<file
+<?php
+
+namespace Test;
+
+use MyNamespace\MyClass as Config;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new FullyQualifiedFacades($file)
+        );
+
+        $this->assertEmpty($lints);
+    }
+
+    /** @test */
+    public function does_not_trigger_on_facade_usage_with_grouped_import()
+    {
+        $file = <<<file
+<?php
+
+namespace Test;
+
+use Illuminate\Support\Facades\{Config, Hash};
+
+file;
+
+        $lints = (new TLint)->lint(
+            new FullyQualifiedFacades($file)
+        );
+
+        $this->assertEmpty($lints);
+    }
+
+    /** @test */
+    public function does_not_trigger_on_facade_usage_with_grouped_import_and_renamed_imports()
+    {
+        $file = <<<file
+<?php
+
+namespace Test;
+
+use Illuminate\Support\Facades\{Config, Hash};
+use Some\Othere\Namespace\{Config as OtherConfig};
+use Some\Other\Namespace\File;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new FullyQualifiedFacades($file)
+        );
+
+        $this->assertEmpty($lints);
+    }
+
+    /** @test */
+    public function ignores_unknown_aliases_in_namespaced_file()
+    {
+        $file = <<<'file'
+<?php
+
+namespace Test;
+
+use Shortcut;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new FullyQualifiedFacades($file)
+        );
+
+        $this->assertEmpty($lints);
+    }
+
+    /** @test */
+    public function ignores_unknown_aliases_in_non_namespaced_file()
+    {
+        $file = <<<'file'
+<?php
+
+use Shortcut;
+
+file;
+
+        $lints = (new TLint)->lint(
+            new FullyQualifiedFacades($file)
+        );
+
+        $this->assertEmpty($lints);
+    }
+
+    /** @test */
+    public function ignores_files_in_same_directory_with_same_name_as_facade_alias()
+    {
+        $file = <<<'file'
+<?php
+
+namespace App\Utilities;
+
+class Stuff
+{
+    public function doStuff()
+    {
+        return File::otherStuff();
+    }
+}
+file;
+
+        $lints = (new TLint)->lint(
+            new FullyQualifiedFacades($file)
+        );
+
+        $this->assertEmpty($lints);
+    }
+}