Browse Source

feat: Ability to remove unused imports from multi-use statements (#7815)

Greg Korba 1 year ago
parent
commit
8210c16064
2 changed files with 311 additions and 48 deletions
  1. 185 20
      src/Fixer/Import/NoUnusedImportsFixer.php
  2. 126 28
      tests/Fixer/Import/NoUnusedImportsFixerTest.php

+ 185 - 20
src/Fixer/Import/NoUnusedImportsFixer.php

@@ -59,7 +59,7 @@ final class NoUnusedImportsFixer extends AbstractFixer
 
     protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
     {
-        $useDeclarations = (new NamespaceUsesAnalyzer())->getDeclarationsFromTokens($tokens);
+        $useDeclarations = (new NamespaceUsesAnalyzer())->getDeclarationsFromTokens($tokens, true);
 
         if (0 === \count($useDeclarations)) {
             return;
@@ -186,15 +186,23 @@ final class NoUnusedImportsFixer extends AbstractFixer
         return false;
     }
 
-    private function removeUseDeclaration(Tokens $tokens, NamespaceUseAnalysis $useDeclaration): void
-    {
-        for ($index = $useDeclaration->getEndIndex() - 1; $index >= $useDeclaration->getStartIndex(); --$index) {
+    private function removeUseDeclaration(
+        Tokens $tokens,
+        NamespaceUseAnalysis $useDeclaration,
+        bool $forceCompleteRemoval = false
+    ): void {
+        [$start, $end] = ($useDeclaration->isInMulti() && !$forceCompleteRemoval)
+            ? [$useDeclaration->getChunkStartIndex(), $useDeclaration->getChunkEndIndex()]
+            : [$useDeclaration->getStartIndex(), $useDeclaration->getEndIndex()];
+        $loopStartIndex = $useDeclaration->isInMulti() || $forceCompleteRemoval ? $end : $end - 1;
+
+        for ($index = $loopStartIndex; $index >= $start; --$index) {
             if ($tokens[$index]->isComment()) {
                 continue;
             }
 
             if (!$tokens[$index]->isWhitespace() || !str_contains($tokens[$index]->getContent(), "\n")) {
-                $tokens->clearTokenAndMergeSurroundingWhitespace($index);
+                $tokens->clearAt($index);
 
                 continue;
             }
@@ -210,12 +218,109 @@ final class NoUnusedImportsFixer extends AbstractFixer
             }
         }
 
+        // For multi-use import statements the tokens containing FQN were already removed in the loop above.
+        // We need to clean up tokens around the ex-chunk to keep the correct syntax and achieve proper formatting.
+        if (!$forceCompleteRemoval && $useDeclaration->isInMulti()) {
+            $this->cleanUpAfterImportChunkRemoval($tokens, $useDeclaration);
+
+            return;
+        }
+
         if ($tokens[$useDeclaration->getEndIndex()]->equals(';')) { // do not remove `? >`
             $tokens->clearAt($useDeclaration->getEndIndex());
         }
 
-        // remove white space above and below where the `use` statement was
+        $this->cleanUpSurroundingNewLines($tokens, $useDeclaration);
+    }
+
+    /**
+     * @param list<NamespaceUseAnalysis> $useDeclarations
+     */
+    private function removeUsesInSameNamespace(Tokens $tokens, array $useDeclarations, NamespaceAnalysis $namespaceDeclaration): void
+    {
+        $namespace = $namespaceDeclaration->getFullName();
+        $nsLength = \strlen($namespace.'\\');
+
+        foreach ($useDeclarations as $useDeclaration) {
+            if ($useDeclaration->isAliased()) {
+                continue;
+            }
+
+            $useDeclarationFullName = ltrim($useDeclaration->getFullName(), '\\');
+
+            if (!str_starts_with($useDeclarationFullName, $namespace.'\\')) {
+                continue;
+            }
+
+            $partName = substr($useDeclarationFullName, $nsLength);
+
+            if (!str_contains($partName, '\\')) {
+                $this->removeUseDeclaration($tokens, $useDeclaration);
+            }
+        }
+    }
+
+    private function cleanUpAfterImportChunkRemoval(Tokens $tokens, NamespaceUseAnalysis $useDeclaration): void
+    {
+        $beforeChunkIndex = $tokens->getPrevMeaningfulToken($useDeclaration->getChunkStartIndex());
+        $afterChunkIndex = $tokens->getNextMeaningfulToken($useDeclaration->getChunkEndIndex());
+        $hasNonEmptyTokenBefore = $this->scanForNonEmptyTokensUntilNewLineFound(
+            $tokens,
+            $afterChunkIndex,
+            -1
+        );
+        $hasNonEmptyTokenAfter = $this->scanForNonEmptyTokensUntilNewLineFound(
+            $tokens,
+            $afterChunkIndex,
+            1
+        );
+
+        // We don't want to merge consequent new lines with indentation (leading to e.g. `\n    \n    `),
+        // so it's safe to merge whitespace only if there is any non-empty token before or after the chunk.
+        $mergingSurroundingWhitespaceIsSafe = $hasNonEmptyTokenBefore[1] || $hasNonEmptyTokenAfter[1];
+        $clearToken = static function (int $index) use ($tokens, $mergingSurroundingWhitespaceIsSafe): void {
+            if ($mergingSurroundingWhitespaceIsSafe) {
+                $tokens->clearTokenAndMergeSurroundingWhitespace($index);
+            } else {
+                $tokens->clearAt($index);
+            }
+        };
+
+        if ($tokens[$afterChunkIndex]->equals(',')) {
+            $clearToken($afterChunkIndex);
+        } elseif ($tokens[$beforeChunkIndex]->equals(',')) {
+            $clearToken($beforeChunkIndex);
+        }
+
+        // Ensure there's a single space where applicable, otherwise no space (before comma, before closing brace)
+        for ($index = $beforeChunkIndex; $index <= $afterChunkIndex; ++$index) {
+            if (null === $tokens[$index]->getId() || !$tokens[$index]->isWhitespace(' ')) {
+                continue;
+            }
+
+            $nextTokenIndex = $tokens->getNextMeaningfulToken($index);
+            if (
+                $tokens[$nextTokenIndex]->equals(',')
+                || $tokens[$nextTokenIndex]->equals(';')
+                || $tokens[$nextTokenIndex]->isGivenKind([CT::T_GROUP_IMPORT_BRACE_CLOSE])
+            ) {
+                $tokens->clearAt($index);
+            } else {
+                $tokens[$index] = new Token([T_WHITESPACE, ' ']);
+            }
+
+            $prevTokenIndex = $tokens->getPrevMeaningfulToken($index);
+            if ($tokens[$prevTokenIndex]->isGivenKind([CT::T_GROUP_IMPORT_BRACE_OPEN])) {
+                $tokens->clearAt($index);
+            }
+        }
+
+        $this->removeLineIfEmpty($tokens, $useDeclaration);
+        $this->removeImportStatementIfEmpty($tokens, $useDeclaration);
+    }
 
+    private function cleanUpSurroundingNewLines(Tokens $tokens, NamespaceUseAnalysis $useDeclaration): void
+    {
         $prevIndex = $useDeclaration->getStartIndex() - 1;
         $prevToken = $tokens[$prevIndex];
 
@@ -261,30 +366,90 @@ final class NoUnusedImportsFixer extends AbstractFixer
         }
     }
 
-    /**
-     * @param list<NamespaceUseAnalysis> $useDeclarations
-     */
-    private function removeUsesInSameNamespace(Tokens $tokens, array $useDeclarations, NamespaceAnalysis $namespaceDeclaration): void
+    private function removeImportStatementIfEmpty(Tokens $tokens, NamespaceUseAnalysis $useDeclaration): void
     {
-        $namespace = $namespaceDeclaration->getFullName();
-        $nsLength = \strlen($namespace.'\\');
+        // First we look for empty groups where all chunks were removed (`use Foo\{};`).
+        // We're only interested in ending brace if its index is between start and end of the import statement.
+        $endingBraceIndex = $tokens->getPrevTokenOfKind(
+            $useDeclaration->getEndIndex(),
+            [[CT::T_GROUP_IMPORT_BRACE_CLOSE]]
+        );
 
-        foreach ($useDeclarations as $useDeclaration) {
-            if ($useDeclaration->isAliased()) {
-                continue;
+        if ($endingBraceIndex > $useDeclaration->getStartIndex()) {
+            $openingBraceIndex = $tokens->getPrevMeaningfulToken($endingBraceIndex);
+
+            if ($tokens[$openingBraceIndex]->isGivenKind(CT::T_GROUP_IMPORT_BRACE_OPEN)) {
+                $this->removeUseDeclaration($tokens, $useDeclaration, true);
             }
+        }
 
-            $useDeclarationFullName = ltrim($useDeclaration->getFullName(), '\\');
+        // Second we look for empty groups where all comma-separated chunks were removed (`use;`).
+        $beforeSemicolonIndex = $tokens->getPrevMeaningfulToken($useDeclaration->getEndIndex());
+        if (
+            $tokens[$beforeSemicolonIndex]->isGivenKind([T_USE])
+            || \in_array($tokens[$beforeSemicolonIndex]->getContent(), ['function', 'const'], true)
+        ) {
+            $this->removeUseDeclaration($tokens, $useDeclaration, true);
+        }
+    }
 
-            if (!str_starts_with($useDeclarationFullName, $namespace.'\\')) {
+    private function removeLineIfEmpty(Tokens $tokens, NamespaceUseAnalysis $useAnalysis): void
+    {
+        if (!$useAnalysis->isInMulti()) {
+            return;
+        }
+
+        $hasNonEmptyTokenBefore = $this->scanForNonEmptyTokensUntilNewLineFound(
+            $tokens,
+            $useAnalysis->getChunkStartIndex(),
+            -1
+        );
+        $hasNonEmptyTokenAfter = $this->scanForNonEmptyTokensUntilNewLineFound(
+            $tokens,
+            $useAnalysis->getChunkEndIndex(),
+            1
+        );
+
+        if (
+            \is_int($hasNonEmptyTokenBefore[0])
+            && !$hasNonEmptyTokenBefore[1]
+            && \is_int($hasNonEmptyTokenAfter[0])
+            && !$hasNonEmptyTokenAfter[1]
+        ) {
+            $tokens->clearRange($hasNonEmptyTokenBefore[0], $hasNonEmptyTokenAfter[0] - 1);
+        }
+    }
+
+    /**
+     * Returns tuple with the index of first token with whitespace containing new line char
+     * and a flag if any non-empty token was found along the way.
+     *
+     * @param -1|1 $direction
+     *
+     * @return array{0: null|int, 1: bool}
+     */
+    private function scanForNonEmptyTokensUntilNewLineFound(Tokens $tokens, int $index, int $direction): array
+    {
+        $hasNonEmptyToken = false;
+        $newLineTokenIndex = null;
+
+        // Iterate until we find new line OR we get out of $tokens bounds (next sibling index is `null`).
+        while (\is_int($index)) {
+            $index = $tokens->getNonEmptySibling($index, $direction);
+
+            if (null === $index || null === $tokens[$index]->getId()) {
                 continue;
             }
 
-            $partName = substr($useDeclarationFullName, $nsLength);
+            if (!$tokens[$index]->isWhitespace()) {
+                $hasNonEmptyToken = true;
+            } elseif (str_starts_with($tokens[$index]->getContent(), "\n")) {
+                $newLineTokenIndex = $index;
 
-            if (!str_contains($partName, '\\')) {
-                $this->removeUseDeclaration($tokens, $useDeclaration);
+                break;
             }
         }
+
+        return [$newLineTokenIndex, $hasNonEmptyToken];
     }
 }

+ 126 - 28
tests/Fixer/Import/NoUnusedImportsFixerTest.php

@@ -169,7 +169,6 @@ final class NoUnusedImportsFixerTest extends AbstractFixerTestCase
             <<<'EOF'
                 <?php namespace App\Http\Controllers;
 
-
                 EOF,
             <<<'EOF'
                 <?php namespace App\Http\Controllers;
@@ -314,7 +313,6 @@ final class NoUnusedImportsFixerTest extends AbstractFixerTestCase
 
                 namespace Foo;
 
-                use BarB, BarC as C, BarD;
                 use BarE;
 
                 $c = new D();
@@ -332,6 +330,8 @@ final class NoUnusedImportsFixerTest extends AbstractFixerTestCase
                 use BarB2;
                 use BarB\B2;
                 use BarE;
+                use function fun_a, fun_b, fun_c;
+                use const CONST_A, CONST_B, CONST_C;
 
                 $c = new D();
                 $e = new BarE();
@@ -670,7 +670,6 @@ final class NoUnusedImportsFixerTest extends AbstractFixerTestCase
                 <?php
                 namespace Aaa;
 
-
                 class Ddd
                 {
                 }
@@ -1319,7 +1318,16 @@ Bar3:
         ];
 
         yield [
-            $expected = <<<'EOF'
+            <<<'EOF'
+                <?php
+                use some\a\{ClassD};
+                use function some\c\{fn_a};
+                use const some\d\{ConstB};
+
+                new CLassD();
+                echo fn_a(ConstB);
+                EOF,
+            <<<'EOF'
                 <?php
                 use some\a\{ClassD};
                 use some\b\{ClassA, ClassB, ClassC as C};
@@ -1327,34 +1335,124 @@ Bar3:
                 use const some\d\{ConstA, ConstB, ConstC};
 
                 new CLassD();
-                echo fn_a();
+                echo fn_a(ConstB);
                 EOF
         ];
 
-        yield [ // TODO test shows lot of cases where imports are not removed while could be
-            '<?php use A\{B,};
-use some\y\{ClassA, ClassB, ClassC as C,};
-use function some\a\{fn_a, fn_b, fn_c,};
-use const some\Z\{ConstAA,ConstBB,ConstCC,};
-use const some\X\{ConstA,ConstB,ConstC,ConstF};
-use C\{D,E,};
+        yield 'grouped imports' => [
+            <<<'EOF'
+                <?php
+                use some\y\{ClassA, ClassC as C,};
+                use function some\a\{
+                    fn_b,
+                };
+                use const some\Z\{ConstA,ConstC,};
 
-    echo ConstA.ConstB.ConstC,ConstF;
-    echo ConstBB.ConstCC;
-    fn_a(ClassA::test, new C());
-',
-            '<?php use A\{B,};
-use some\y\{ClassA, ClassB, ClassC as C,};
-use function some\a\{fn_a, fn_b, fn_c,};
-use const some\Z\{ConstAA,ConstBB,ConstCC,};
-use const some\X\{ConstA,ConstB,ConstC,ConstF};
-use C\{D,E,};
-use Z;
-
-    echo ConstA.ConstB.ConstC,ConstF;
-    echo ConstBB.ConstCC;
-    fn_a(ClassA::test, new C());
-',
+                echo ConstA.ConstC;
+                fn_b(ClassA::test, new C());
+                EOF,
+            <<<'EOF'
+                <?php
+                use A\{B,};
+                use some\y\{ClassA, ClassB, ClassC as C,};
+                use function some\a\{
+                    fn_a,
+                    fn_b,
+                    fn_c,
+                };
+                use function some\b\{fn_x, fn_y, fn_z,};
+                use const some\Z\{ConstA,ConstB,ConstC,};
+                use const some\X\{ConstX,ConstY,ConstZ};
+                use C\{D,E,};
+                use Z;
+
+                echo ConstA.ConstC;
+                fn_b(ClassA::test, new C());
+                EOF,
+        ];
+
+        yield 'multiline grouped imports with comments' => [
+            <<<'EOF'
+                <?php
+                use function some\a\{
+                     // Foo
+                    fn_b,
+                    /* Bar *//** Baz */
+                     # Buzz
+                };
+
+                fn_b();
+                EOF,
+            <<<'EOF'
+                <?php
+                use function some\a\{
+                    fn_a, // Foo
+                    fn_b,
+                    /* Bar */ fn_c, /** Baz */
+                    fn_d, # Buzz
+                };
+
+                fn_b();
+                EOF,
+        ];
+
+        yield 'comma-separated imports' => [
+            <<<'EOF'
+                <?php
+                use A;
+                use function fn_b;
+                use const ConstC;
+
+                fn_b(new A(), ConstC);
+                EOF,
+            <<<'EOF'
+                <?php
+                use A, B, C;
+                use function fn_a, fn_b, fn_c;
+                use const ConstA, ConstB, ConstC;
+
+                fn_b(new A(), ConstC);
+                EOF,
+        ];
+
+        yield 'only unused comma-separated imports in single line' => [
+            '<?php ',
+            '<?php use A, B, C;',
+        ];
+
+        yield 'only unused grouped imports in single line' => [
+            '<?php ',
+            '<?php use A\{B, C};',
+        ];
+
+        yield 'unused comma-separated imports right after open tag, with consecutive lines' => [
+            "<?php \n# Comment",
+            "<?php use A, B, C;\n\n# Comment",
+        ];
+
+        yield 'unused grouped imports right after open tag, with consecutive lines' => [
+            "<?php \n# Comment",
+            "<?php use A\\{B, C};\n\n# Comment",
+        ];
+
+        yield 'unused comma-separated imports right after open tag with a non-empty token after it, and with consecutive lines' => [
+            "<?php # Comment\n\n# Another comment",
+            "<?php use A, B, C; # Comment\n\n# Another comment",
+        ];
+
+        yield 'unused grouped imports right after open tag with a non-empty token after it, and with consecutive lines' => [
+            "<?php # Comment\n\n# Another comment",
+            "<?php use A\\{B, C}; # Comment\n\n# Another comment",
+        ];
+
+        yield 'only unused comma-separated imports in the last line, with whitespace after' => [
+            "<?php \n",
+            "<?php \nuse A, B, C;     \t\t",
+        ];
+
+        yield 'only unused grouped imports in the last line, with whitespace after' => [
+            "<?php \n",
+            "<?php \nuse A\\{B, C};     \t\t",
         ];
     }