From 1fc2bdafcf0b9e6cde197881b7ed5c97b1372e0d Mon Sep 17 00:00:00 2001
From: Julien Falque <julien.falque@gmail.com>
Date: Sun, 16 May 2021 14:24:58 +0200
Subject: [PATCH] NoUnusedImportsFixer - Fix undetected unused imports when
 type mismatch

---
 src/Fixer/Import/NoUnusedImportsFixer.php     |  68 ++++--
 .../Analysis/NamespaceUseAnalysis.php         |   2 +-
 src/Tokenizer/Analyzer/GotoLabelAnalyzer.php  |   2 +-
 .../Fixer/Import/NoUnusedImportsFixerTest.php | 209 +++++++++++++++++-
 tests/Fixtures/Integration/misc/PHP7_0.test   |   1 -
 tests/Fixtures/Integration/misc/PHP7_2.test   |   2 -
 .../Analyzer/GotoLabelAnalyzerTest.php        |  19 +-
 7 files changed, 270 insertions(+), 33 deletions(-)

diff --git a/src/Fixer/Import/NoUnusedImportsFixer.php b/src/Fixer/Import/NoUnusedImportsFixer.php
index 3f404710d71..9142123180d 100644
--- a/src/Fixer/Import/NoUnusedImportsFixer.php
+++ b/src/Fixer/Import/NoUnusedImportsFixer.php
@@ -21,10 +21,12 @@
 use PhpCsFixer\Preg;
 use PhpCsFixer\Tokenizer\Analyzer\Analysis\NamespaceAnalysis;
 use PhpCsFixer\Tokenizer\Analyzer\Analysis\NamespaceUseAnalysis;
+use PhpCsFixer\Tokenizer\Analyzer\GotoLabelAnalyzer;
 use PhpCsFixer\Tokenizer\Analyzer\NamespacesAnalyzer;
 use PhpCsFixer\Tokenizer\Analyzer\NamespaceUsesAnalyzer;
 use PhpCsFixer\Tokenizer\Token;
 use PhpCsFixer\Tokenizer\Tokens;
+use PhpCsFixer\Tokenizer\TokensAnalyzer;
 
 /**
  * @author Dariusz Rumiński <dariusz.ruminski@gmail.com>
@@ -73,24 +75,18 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
         }
 
         foreach ((new NamespacesAnalyzer())->getDeclarations($tokens) as $namespace) {
-            $currentNamespaceUseDeclarations = array_filter(
-                $useDeclarations,
-                static function (NamespaceUseAnalysis $useDeclaration) use ($namespace) {
-                    return
-                        $useDeclaration->getStartIndex() >= $namespace->getScopeStartIndex()
-                        && $useDeclaration->getEndIndex() <= $namespace->getScopeEndIndex()
-                    ;
-                }
-            );
-
-            $usagesSearchIgnoredIndexes = [];
+            $currentNamespaceUseDeclarations = [];
+            $currentNamespaceUseDeclarationIndexes = [];
 
-            foreach ($currentNamespaceUseDeclarations as $useDeclaration) {
-                $usagesSearchIgnoredIndexes[$useDeclaration->getStartIndex()] = $useDeclaration->getEndIndex();
+            foreach ($useDeclarations as $useDeclaration) {
+                if ($useDeclaration->getStartIndex() >= $namespace->getScopeStartIndex() && $useDeclaration->getEndIndex() <= $namespace->getScopeEndIndex()) {
+                    $currentNamespaceUseDeclarations[] = $useDeclaration;
+                    $currentNamespaceUseDeclarationIndexes[$useDeclaration->getStartIndex()] = $useDeclaration->getEndIndex();
+                }
             }
 
             foreach ($currentNamespaceUseDeclarations as $useDeclaration) {
-                if (!$this->isImportUsed($tokens, $namespace, $usagesSearchIgnoredIndexes, $useDeclaration->getShortName())) {
+                if (!$this->isImportUsed($tokens, $namespace, $useDeclaration, $currentNamespaceUseDeclarationIndexes)) {
                     $this->removeUseDeclaration($tokens, $useDeclaration);
                 }
             }
@@ -100,10 +96,19 @@ static function (NamespaceUseAnalysis $useDeclaration) use ($namespace) {
     }
 
     /**
-     * @param array<int, int> $ignoredIndexes
+     * @param array<int, int> $ignoredIndexes indexes of the use statements themselves that should not be checked as being "used"
      */
-    private function isImportUsed(Tokens $tokens, NamespaceAnalysis $namespace, array $ignoredIndexes, string $shortName): bool
+    private function isImportUsed(Tokens $tokens, NamespaceAnalysis $namespace, NamespaceUseAnalysis $import, array $ignoredIndexes): bool
     {
+        $analyzer = new TokensAnalyzer($tokens);
+        $gotoLabelAnalyzer = new GotoLabelAnalyzer();
+
+        $tokensNotBeforeFunctionCall = [T_NEW];
+        // @TODO: drop condition when PHP 8.0+ is required
+        if (\defined('T_ATTRIBUTE')) {
+            $tokensNotBeforeFunctionCall[] = T_ATTRIBUTE;
+        }
+
         $namespaceEndIndex = $namespace->getScopeEndIndex();
         for ($index = $namespace->getScopeStartIndex(); $index <= $namespaceEndIndex; ++$index) {
             if (isset($ignoredIndexes[$index])) {
@@ -115,6 +120,10 @@ private function isImportUsed(Tokens $tokens, NamespaceAnalysis $namespace, arra
             $token = $tokens[$index];
 
             if ($token->isGivenKind(T_STRING)) {
+                if (0 !== strcasecmp($import->getShortName(), $token->getContent())) {
+                    continue;
+                }
+
                 $prevMeaningfulToken = $tokens[$tokens->getPrevMeaningfulToken($index)];
 
                 if ($prevMeaningfulToken->isGivenKind(T_NAMESPACE)) {
@@ -124,10 +133,29 @@ private function isImportUsed(Tokens $tokens, NamespaceAnalysis $namespace, arra
                 }
 
                 if (
-                    0 === strcasecmp($shortName, $token->getContent())
-                    && !$prevMeaningfulToken->isGivenKind([T_NS_SEPARATOR, T_CONST, T_DOUBLE_COLON])
-                    && !$prevMeaningfulToken->isObjectOperator()
+                    $prevMeaningfulToken->isGivenKind([T_NS_SEPARATOR, T_FUNCTION, T_CONST, T_DOUBLE_COLON])
+                    || $prevMeaningfulToken->isObjectOperator()
                 ) {
+                    continue;
+                }
+
+                $nextMeaningfulIndex = $tokens->getNextMeaningfulToken($index);
+
+                if ($gotoLabelAnalyzer->belongsToGoToLabel($tokens, $nextMeaningfulIndex)) {
+                    continue;
+                }
+
+                $nextMeaningfulToken = $tokens[$nextMeaningfulIndex];
+
+                if ($analyzer->isConstantInvocation($index)) {
+                    $type = NamespaceUseAnalysis::TYPE_CONSTANT;
+                } elseif ($nextMeaningfulToken->equals('(') && !$prevMeaningfulToken->isGivenKind($tokensNotBeforeFunctionCall)) {
+                    $type = NamespaceUseAnalysis::TYPE_FUNCTION;
+                } else {
+                    $type = NamespaceUseAnalysis::TYPE_CLASS;
+                }
+
+                if ($import->getType() === $type) {
                     return true;
                 }
 
@@ -136,7 +164,7 @@ private function isImportUsed(Tokens $tokens, NamespaceAnalysis $namespace, arra
 
             if ($token->isComment()
                 && Preg::match(
-                    '/(?<![[:alnum:]\$])(?<!\\\\)'.$shortName.'(?![[:alnum:]])/i',
+                    '/(?<![[:alnum:]\$])(?<!\\\\)'.$import->getShortName().'(?![[:alnum:]])/i',
                     $token->getContent()
                 )
             ) {
diff --git a/src/Tokenizer/Analyzer/Analysis/NamespaceUseAnalysis.php b/src/Tokenizer/Analyzer/Analysis/NamespaceUseAnalysis.php
index adf57870a08..beb29d70edd 100644
--- a/src/Tokenizer/Analyzer/Analysis/NamespaceUseAnalysis.php
+++ b/src/Tokenizer/Analyzer/Analysis/NamespaceUseAnalysis.php
@@ -19,7 +19,7 @@
  */
 final class NamespaceUseAnalysis implements StartEndTokenAwareAnalysis
 {
-    public const TYPE_CLASS = 1;
+    public const TYPE_CLASS = 1; // "classy" could be class, interface or trait
     public const TYPE_FUNCTION = 2;
     public const TYPE_CONSTANT = 3;
 
diff --git a/src/Tokenizer/Analyzer/GotoLabelAnalyzer.php b/src/Tokenizer/Analyzer/GotoLabelAnalyzer.php
index fb851f8dcb5..15d5b0660f8 100644
--- a/src/Tokenizer/Analyzer/GotoLabelAnalyzer.php
+++ b/src/Tokenizer/Analyzer/GotoLabelAnalyzer.php
@@ -35,6 +35,6 @@ public function belongsToGoToLabel(Tokens $tokens, int $index): bool
 
         $prevMeaningfulTokenIndex = $tokens->getPrevMeaningfulToken($prevMeaningfulTokenIndex);
 
-        return $tokens[$prevMeaningfulTokenIndex]->equalsAny([';', '{', '}', [T_OPEN_TAG]]);
+        return $tokens[$prevMeaningfulTokenIndex]->equalsAny([':', ';', '{', '}', [T_OPEN_TAG]]);
     }
 }
diff --git a/tests/Fixer/Import/NoUnusedImportsFixerTest.php b/tests/Fixer/Import/NoUnusedImportsFixerTest.php
index 84407301abf..507d4deba32 100644
--- a/tests/Fixer/Import/NoUnusedImportsFixerTest.php
+++ b/tests/Fixer/Import/NoUnusedImportsFixerTest.php
@@ -408,6 +408,7 @@ public function doSomething($foo)
 <?php
 
 use Foo as Bar;
+use A\MyTrait1;
 
 class MyParent
 {
@@ -422,6 +423,7 @@ class MyParent
 
 use Foo;
 use Foo as Bar;
+use A\MyTrait1;
 
 class MyParent
 {
@@ -1085,6 +1087,192 @@ public function __invoke(StoreRequest $request)
     {}
 }',
             ],
+            'unused import matching function call' => [
+                '<?php
+namespace Foo;
+bar();',
+                '<?php
+namespace Foo;
+use Bar;
+bar();',
+            ],
+            'unused import matching function declaration' => [
+                '<?php
+namespace Foo;
+function bar () {}',
+                '<?php
+namespace Foo;
+use Bar;
+function bar () {}',
+            ],
+            'unused import matching method declaration' => [
+                '<?php
+namespace Foo;
+class Foo {
+    public function bar () {}
+}',
+                '<?php
+namespace Foo;
+use Bar;
+class Foo {
+    public function bar () {}
+}',
+            ],
+            'unused import matching constant usage' => [
+                '<?php
+namespace Foo;
+echo BAR;',
+                '<?php
+namespace Foo;
+use Bar;
+echo BAR;',
+            ],
+            'unused import matching class constant' => [
+                '<?php
+namespace Foo;
+class Foo {
+    const BAR = 1;
+}',
+                '<?php
+namespace Foo;
+use Bar;
+class Foo {
+    const BAR = 1;
+}',
+            ],
+            'unused function import matching class usage' => [
+                '<?php
+namespace Foo;
+new Bar();
+Baz::method();',
+                '<?php
+namespace Foo;
+use function bar;
+use function baz;
+new Bar();
+Baz::method();',
+            ],
+            'unused function import matching method call' => [
+                '<?php
+namespace Foo;
+Foo::bar();',
+                '<?php
+namespace Foo;
+use function bar;
+Foo::bar();',
+            ],
+            'unused function import matching method declaration' => [
+                '<?php
+namespace Foo;
+class Foo {
+    public function bar () {}
+}',
+                '<?php
+namespace Foo;
+use function bar;
+class Foo {
+    public function bar () {}
+}',
+            ],
+            'unused function import matching constant usage' => [
+                '<?php
+namespace Foo;
+echo BAR;',
+                '<?php
+namespace Foo;
+use function bar;
+echo BAR;',
+            ],
+            'unused function import matching class constant' => [
+                '<?php
+namespace Foo;
+class Foo {
+    const BAR = 1;
+}',
+                '<?php
+namespace Foo;
+use function bar;
+class Foo {
+    const BAR = 1;
+}',
+            ],
+            'unused constant import matching function call' => [
+                '<?php
+namespace Foo;
+bar();',
+                '<?php
+namespace Foo;
+use const BAR;
+bar();',
+            ],
+            'unused constant import matching function declaration' => [
+                '<?php
+namespace Foo;
+function bar () {}',
+                '<?php
+namespace Foo;
+use const BAR;
+function bar () {}',
+            ],
+            'unused constant import matching method declaration' => [
+                '<?php
+namespace Foo;
+class Foo {
+    public function bar () {}
+}',
+                '<?php
+namespace Foo;
+use const BAR;
+class Foo {
+    public function bar () {}
+}',
+            ],
+            'unused constant import matching class constant' => [
+                '<?php
+namespace Foo;
+class Foo {
+    const BAR = 1;
+}',
+                '<?php
+namespace Foo;
+use const BAR;
+class Foo {
+    const BAR = 1;
+}',
+            ],
+            'attribute without braces' => [
+                '<?php
+use Foo;
+class Controller
+{
+    #[Foo]
+    public function foo() {}
+}',
+            ],
+            'attribute with braces' => [
+                '<?php
+use Foo;
+class Controller
+{
+    #[Foo()]
+    public function foo() {}
+}',
+            ],
+            'go to' => [
+                '<?php
+Bar1:
+Bar2:
+Bar3:
+',
+                '<?php
+use Bar1;
+use const Bar2;
+use function Bar3;
+Bar1:
+Bar2:
+Bar3:
+',
+            ],
         ];
     }
 
@@ -1181,10 +1369,16 @@ public function testFixPrePHP80(): void
 
     /**
      * @requires PHP 8.0
+     * @dataProvider providePhp80Cases
      */
-    public function testFix80(): void
+    public function testFix80(string $expected, ?string $input = null): void
     {
-        $this->doTest(
+        $this->doTest($expected, $input);
+    }
+
+    public function providePhp80Cases()
+    {
+        yield [
             '<?php
 
 
@@ -1197,7 +1391,14 @@ public function testFix80(): void
 
 $x = $foo?->bar;
 $y = foo?->bar();
-'
-        );
+',
+        ];
+
+        yield 'with union type in non-capturing catch' => [
+            '<?php
+use Foo;
+use Bar;
+try {} catch (Foo | Bar) {}',
+        ];
     }
 }
diff --git a/tests/Fixtures/Integration/misc/PHP7_0.test b/tests/Fixtures/Integration/misc/PHP7_0.test
index afb3e9d9582..da6c47d60ea 100644
--- a/tests/Fixtures/Integration/misc/PHP7_0.test
+++ b/tests/Fixtures/Integration/misc/PHP7_0.test
@@ -25,7 +25,6 @@ use const some\a\ConstC;
 use function some\a\fn_a;
 use function some\a\fn_b;
 use function some\a\fn_c;
-use function some\x\CC as C;
 use some\x\ClassX;
 use function some\x\D;
 use const some\x\E;
diff --git a/tests/Fixtures/Integration/misc/PHP7_2.test b/tests/Fixtures/Integration/misc/PHP7_2.test
index 1e46b3346c7..a15f9d299d9 100644
--- a/tests/Fixtures/Integration/misc/PHP7_2.test
+++ b/tests/Fixtures/Integration/misc/PHP7_2.test
@@ -12,8 +12,6 @@ PHP 7.2 test.
 
 namespace Foo;
 
-use Bar\A;
-
 class C
 {
     public function A($a, object $b): object
diff --git a/tests/Tokenizer/Analyzer/GotoLabelAnalyzerTest.php b/tests/Tokenizer/Analyzer/GotoLabelAnalyzerTest.php
index ae8ea5bbaae..d657f492c0e 100644
--- a/tests/Tokenizer/Analyzer/GotoLabelAnalyzerTest.php
+++ b/tests/Tokenizer/Analyzer/GotoLabelAnalyzerTest.php
@@ -53,27 +53,27 @@ public function provideIsClassyInvocationCases()
                 ',
                 [],
             ],
-            'handle goto labels 1' => [
+            'after php tag' => [
                 '<?php
                     beginning:
                     echo $guard?1:2;',
                 [3],
             ],
-            'handle goto labels 2' => [
+            'after closing brace' => [
                 '<?php
                     function A(){}
                     beginning:
                     echo $guard?1:2;',
                 [11],
             ],
-            'handle goto labels 3' => [
+            'after statement' => [
                 '<?php
                     echo 1;
                     beginning:
                     echo $guard?1:2;',
                 [8],
             ],
-            'handle goto labels 4' => [
+            'after opening brace' => [
                 '<?php
                     echo 1;
                     {
@@ -83,6 +83,17 @@ function A(){}
                 ',
                 [10],
             ],
+            'after use statements' => [
+                '<?php
+use Bar1;
+use const Bar2;
+use function Bar3;
+Bar1:
+Bar2:
+Bar3:
+',
+                [21, 24, 27],
+            ],
         ];
 
         foreach ($tests as $index => $test) {