Browse Source

feat: Support more FQCNs cases in `fully_qualified_strict_types` (#7459)

Co-authored-by: SpacePossum <possumfromspace@gmail.com>
Greg Korba 1 year ago
parent
commit
1bc8e827fa

+ 42 - 3
doc/rules/import/fully_qualified_strict_types.rst

@@ -2,8 +2,10 @@
 Rule ``fully_qualified_strict_types``
 =====================================
 
-Transforms imported FQCN parameters and return types in function arguments to
-short version.
+Removes the leading part of fully qualified symbol references if a given symbol
+is imported or belongs to the current namespace. Fixes function arguments,
+exceptions in ``catch`` block, ``extend`` and ``implements`` of classes and
+interfaces.
 
 Configuration
 -------------
@@ -34,12 +36,16 @@ Example #1
 
     use Foo\Bar;
     use Foo\Bar\Baz;
+    use Foo\OtherClass;
+    use Foo\SomeContract;
+    use Foo\SomeException;
 
     /**
    - * @see \Foo\Bar\Baz
    + * @see Baz
      */
-    class SomeClass
+   -class SomeClass extends \Foo\OtherClass implements \Foo\SomeContract
+   +class SomeClass extends OtherClass implements SomeContract
     {
         /**
    -     * @var \Foo\Bar\Baz
@@ -62,6 +68,14 @@ Example #1
         public function getBaz() {
             return $this->baz;
         }
+
+   -    public function doX(\Foo\Bar $foo, \Exception $e): \Foo\Bar\Baz
+   +    public function doX(Bar $foo, Exception $e): Baz
+        {
+            try {}
+   -        catch (\Foo\SomeException $e) {}
+   +        catch (SomeException $e) {}
+        }
     }
 
 Example #2
@@ -83,6 +97,31 @@ With configuration: ``['leading_backslash_in_global_namespace' => true]``.
         }
     }
 
+Example #3
+~~~~~~~~~~
+
+With configuration: ``['leading_backslash_in_global_namespace' => true]``.
+
+.. code-block:: diff
+
+   --- Original
+   +++ New
+    <?php
+    namespace {
+        use Foo\A;
+        try {
+            foo();
+   -    } catch (\Exception|\Foo\A $e) {
+   +    } catch (Exception|A $e) {
+        }
+    }
+    namespace Foo\Bar {
+   -    class SomeClass implements \Foo\Bar\Baz
+   +    class SomeClass implements Baz
+        {
+        }
+    }
+
 Rule sets
 ---------
 

+ 1 - 1
doc/rules/index.rst

@@ -431,7 +431,7 @@ Import
 
 - `fully_qualified_strict_types <./import/fully_qualified_strict_types.rst>`_
 
-  Transforms imported FQCN parameters and return types in function arguments to short version.
+  Removes the leading part of fully qualified symbol references if a given symbol is imported or belongs to the current namespace. Fixes function arguments, exceptions in ``catch`` block, ``extend`` and ``implements`` of classes and interfaces.
 - `global_namespace_import <./import/global_namespace_import.rst>`_
 
   Imports or fully qualifies global classes/functions/constants.

+ 185 - 3
src/Fixer/Import/FullyQualifiedStrictTypesFixer.php

@@ -34,24 +34,28 @@ use PhpCsFixer\Tokenizer\Tokens;
  * @author VeeWee <toonverwerft@gmail.com>
  * @author Tomas Jadrny <developer@tomasjadrny.cz>
  * @author Greg Korba <greg@codito.dev>
+ * @author SpacePossum <possumfromspace@gmail.com>
  */
 final class FullyQualifiedStrictTypesFixer extends AbstractFixer implements ConfigurableFixerInterface
 {
     public function getDefinition(): FixerDefinitionInterface
     {
         return new FixerDefinition(
-            'Transforms imported FQCN parameters and return types in function arguments to short version.',
+            'Removes the leading part of fully qualified symbol references if a given symbol is imported or belongs to the current namespace. Fixes function arguments, exceptions in `catch` block, `extend` and `implements` of classes and interfaces.',
             [
                 new CodeSample(
                     '<?php
 
 use Foo\Bar;
 use Foo\Bar\Baz;
+use Foo\OtherClass;
+use Foo\SomeContract;
+use Foo\SomeException;
 
 /**
  * @see \Foo\Bar\Baz
  */
-class SomeClass
+class SomeClass extends \Foo\OtherClass implements \Foo\SomeContract
 {
     /**
      * @var \Foo\Bar\Baz
@@ -71,6 +75,12 @@ class SomeClass
     public function getBaz() {
         return $this->baz;
     }
+
+    public function doX(\Foo\Bar $foo, \Exception $e): \Foo\Bar\Baz
+    {
+        try {}
+        catch (\Foo\SomeException $e) {}
+    }
 }
 '
                 ),
@@ -83,6 +93,23 @@ class SomeClass
     {
     }
 }
+',
+                    ['leading_backslash_in_global_namespace' => true]
+                ),
+                new CodeSample(
+                    '<?php
+namespace {
+    use Foo\A;
+    try {
+        foo();
+    } catch (\Exception|\Foo\A $e) {
+    }
+}
+namespace Foo\Bar {
+    class SomeClass implements \Foo\Bar\Baz
+    {
+    }
+}
 ',
                     ['leading_backslash_in_global_namespace' => true]
                 ),
@@ -103,7 +130,14 @@ class SomeClass
 
     public function isCandidate(Tokens $tokens): bool
     {
-        return $tokens->isAnyTokenKindsFound([T_FUNCTION, T_DOC_COMMENT]);
+        return $tokens->isAnyTokenKindsFound([
+            T_FUNCTION,
+            T_DOC_COMMENT,
+            T_IMPLEMENTS,
+            T_EXTENDS,
+            T_CATCH,
+            T_DOUBLE_COLON,
+        ]);
     }
 
     protected function createConfigurationDefinition(): FixerConfigurationResolverInterface
@@ -135,6 +169,12 @@ class SomeClass
             for ($index = $namespace->getScopeStartIndex(); $index < $namespace->getScopeEndIndex(); ++$index) {
                 if ($tokens[$index]->isGivenKind(T_FUNCTION)) {
                     $this->fixFunction($functionsAnalyzer, $tokens, $index, $uses, $namespaceName);
+                } elseif ($tokens[$index]->isGivenKind([T_EXTENDS, T_IMPLEMENTS])) {
+                    $this->fixExtendsImplements($tokens, $index, $uses, $namespaceName);
+                } elseif ($tokens[$index]->isGivenKind(T_CATCH)) {
+                    $this->fixCatch($tokens, $index, $uses, $namespaceName);
+                } elseif ($tokens[$index]->isGivenKind(T_DOUBLE_COLON)) {
+                    $this->fixClassStaticAccess($tokens, $index, $uses, $namespaceName);
                 }
 
                 if ($tokens[$index]->isGivenKind(T_DOC_COMMENT)) {
@@ -200,6 +240,130 @@ class SomeClass
         }
     }
 
+    /**
+     * @param array<string, string> $uses
+     */
+    private function fixExtendsImplements(Tokens $tokens, int $index, array $uses, string $namespaceName): void
+    {
+        $index = $tokens->getNextMeaningfulToken($index);
+        $extend = ['content' => '', 'tokens' => []];
+
+        while (true) {
+            if ($tokens[$index]->equalsAny([',', '{', [T_IMPLEMENTS]])) {
+                $this->shortenClassIfPossible($tokens, $extend, $uses, $namespaceName);
+
+                if ($tokens[$index]->equals('{')) {
+                    break;
+                }
+
+                $extend = ['content' => '', 'tokens' => []];
+            } else {
+                $extend['tokens'][] = $index;
+                $extend['content'] .= $tokens[$index]->getContent();
+            }
+
+            $index = $tokens->getNextMeaningfulToken($index);
+        }
+    }
+
+    /**
+     * @param array<string, string> $uses
+     */
+    private function fixCatch(Tokens $tokens, int $index, array $uses, string $namespaceName): void
+    {
+        $index = $tokens->getNextMeaningfulToken($index); // '('
+        $index = $tokens->getNextMeaningfulToken($index); // first part of first exception class to be caught
+
+        $caughtExceptionClass = ['content' => '', 'tokens' => []];
+
+        while (true) {
+            if ($tokens[$index]->equalsAny([')', [T_VARIABLE], [CT::T_TYPE_ALTERNATION]])) {
+                if (0 === \count($caughtExceptionClass['tokens'])) {
+                    break;
+                }
+
+                $this->shortenClassIfPossible($tokens, $caughtExceptionClass, $uses, $namespaceName);
+
+                if ($tokens[$index]->equals(')')) {
+                    break;
+                }
+
+                $caughtExceptionClass = ['content' => '', 'tokens' => []];
+            } else {
+                $caughtExceptionClass['tokens'][] = $index;
+                $caughtExceptionClass['content'] .= $tokens[$index]->getContent();
+            }
+
+            $index = $tokens->getNextMeaningfulToken($index);
+        }
+    }
+
+    /**
+     * @param array<string, string> $uses
+     */
+    private function fixClassStaticAccess(Tokens $tokens, int $index, array $uses, string $namespaceName): void
+    {
+        $classConstantRef = ['content' => '', 'tokens' => []];
+
+        while (true) {
+            $index = $tokens->getPrevMeaningfulToken($index);
+
+            if ($tokens[$index]->equalsAny([[T_STRING], [T_NS_SEPARATOR]])) {
+                $classConstantRef['tokens'][] = $index;
+                $classConstantRef['content'] = $tokens[$index]->getContent().$classConstantRef['content'];
+            } else {
+                $classConstantRef['tokens'] = array_reverse($classConstantRef['tokens']);
+                $this->shortenClassIfPossible($tokens, $classConstantRef, $uses, $namespaceName);
+
+                break;
+            }
+        }
+    }
+
+    /**
+     * @param array{content: string, tokens: array<int>} $class
+     * @param array<string, string>                      $uses
+     */
+    private function shortenClassIfPossible(Tokens $tokens, array $class, array $uses, string $namespaceName): void
+    {
+        $longTypeContent = $class['content'];
+
+        if (str_starts_with($longTypeContent, '\\')) {
+            $typeName = substr($longTypeContent, 1);
+            $typeNameLower = strtolower($typeName);
+
+            if (isset($uses[$typeNameLower])) {
+                // if the type without leading "\" equals any of the full "uses" long names, it can be replaced with the short one
+                $this->replaceClassWithShort($tokens, $class, $uses[$typeNameLower]);
+            } elseif ('' === $namespaceName) {
+                if (true === $this->configuration['leading_backslash_in_global_namespace']) {
+                    // if we are in the global namespace and the type is not imported the leading '\' can be removed
+                    $inUses = false;
+
+                    foreach ($uses as $useShortName) {
+                        if (strtolower($useShortName) === $typeNameLower) {
+                            $inUses = true;
+
+                            break;
+                        }
+                    }
+
+                    if (!$inUses) {
+                        $this->replaceClassWithShort($tokens, $class, $typeName);
+                    }
+                }
+            } elseif (
+                $typeNameLower !== $namespaceName
+                && str_starts_with($typeNameLower, $namespaceName)
+                && '\\' === $typeNameLower[\strlen($namespaceName)]
+            ) {
+                // if the type starts with namespace and the type is not the same as the namespace it can be shortened
+                $typeNameShort = substr($typeName, \strlen($namespaceName) + 1);
+                $this->replaceClassWithShort($tokens, $class, $typeNameShort);
+            }
+        } // else: no shorter type possible
+    }
+
     /**
      * @param array<string, string> $uses
      */
@@ -290,6 +454,24 @@ class SomeClass
         return null;
     }
 
+    /**
+     * @param array{content: string, tokens: array<int>} $class
+     */
+    private function replaceClassWithShort(Tokens $tokens, array $class, string $short): void
+    {
+        $i = 0; // override the tokens
+
+        foreach ($this->namespacedStringToTokens($short) as $shortToken) {
+            $tokens[$class['tokens'][$i]] = $shortToken;
+            ++$i;
+        }
+
+        // clear the leftovers
+        for ($j = \count($class['tokens']) - 1; $j >= $i; --$j) {
+            $tokens->clearTokenAndMergeSurroundingWhitespace($class['tokens'][$j]);
+        }
+    }
+
     /**
      * @return iterable<string, array{int, int}>
      */

+ 2 - 2
tests/Cache/NullCacheManagerTest.php

@@ -28,14 +28,14 @@ final class NullCacheManagerTest extends TestCase
 {
     public function testIsFinal(): void
     {
-        $reflection = new \ReflectionClass(\PhpCsFixer\Cache\NullCacheManager::class);
+        $reflection = new \ReflectionClass(NullCacheManager::class);
 
         self::assertTrue($reflection->isFinal());
     }
 
     public function testImplementsCacheManagerInterface(): void
     {
-        $reflection = new \ReflectionClass(\PhpCsFixer\Cache\NullCacheManager::class);
+        $reflection = new \ReflectionClass(NullCacheManager::class);
 
         self::assertTrue($reflection->implementsInterface(\PhpCsFixer\Cache\CacheManagerInterface::class));
     }

+ 2 - 2
tests/Cache/SignatureTest.php

@@ -28,14 +28,14 @@ final class SignatureTest extends TestCase
 {
     public function testIsFinal(): void
     {
-        $reflection = new \ReflectionClass(\PhpCsFixer\Cache\Signature::class);
+        $reflection = new \ReflectionClass(Signature::class);
 
         self::assertTrue($reflection->isFinal());
     }
 
     public function testImplementsSignatureInterface(): void
     {
-        $reflection = new \ReflectionClass(\PhpCsFixer\Cache\Signature::class);
+        $reflection = new \ReflectionClass(Signature::class);
 
         self::assertTrue($reflection->implementsInterface(\PhpCsFixer\Cache\SignatureInterface::class));
     }

+ 215 - 5
tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php

@@ -28,9 +28,9 @@ final class FullyQualifiedStrictTypesFixerTest extends AbstractFixerTestCase
     /**
      * @param array<string, bool> $config
      *
-     * @dataProvider provideNewLogicCases
+     * @dataProvider provideFixCases
      */
-    public function testNewLogic(string $expected, ?string $input = null, array $config = []): void
+    public function testFix(string $expected, ?string $input = null, array $config = []): void
     {
         $this->fixer->configure($config);
         $this->doTest($expected, $input);
@@ -39,7 +39,7 @@ final class FullyQualifiedStrictTypesFixerTest extends AbstractFixerTestCase
     /**
      * @return iterable<array{0: string, 1?: null|string, 2?: array<string, mixed>}>
      */
-    public static function provideNewLogicCases(): iterable
+    public static function provideFixCases(): iterable
     {
         yield 'namespace === type name' => [
             '<?php
@@ -156,6 +156,205 @@ class A {
     }
 }',
         ];
+
+        yield 'interface multiple extends' => [
+            '<?php
+namespace Foo\Bar;
+use D\E;
+use IIII\G;
+use Foo\Bar\C;
+interface NakanoInterface extends IzumiInterface, A, E, \C, EZ
+{
+}',
+            '<?php
+namespace Foo\Bar;
+use D\E;
+use IIII\G;
+use Foo\Bar\C;
+interface NakanoInterface extends \Foo\Bar\IzumiInterface, \Foo\Bar\A, \D\E, \C, EZ
+{
+}',
+        ];
+
+        yield 'interface in global namespace with global extend' => [
+            '<?php interface Foo1 extends ArrayAccess2{}',
+            '<?php interface Foo1 extends \ArrayAccess2{}',
+            ['leading_backslash_in_global_namespace' => true],
+        ];
+
+        yield 'interface in global namespace with multiple extend' => [
+            '<?php use B\Exception; interface Foo extends ArrayAccess, \Exception, Exception {}',
+            '<?php use B\Exception; interface Foo extends \ArrayAccess, \Exception, \B\Exception {}',
+            ['leading_backslash_in_global_namespace' => true],
+        ];
+
+        yield 'class implements' => [
+            '<?php
+namespace Foo\Bar;
+class SomeClass implements Izumi
+{
+}',
+            '<?php
+namespace Foo\Bar;
+class SomeClass implements \Foo\Bar\Izumi
+{
+}',
+        ];
+
+        yield 'anonymous class implements, shorten to namespace' => [
+            '<?php
+namespace Foo\Bar;
+$a = new class implements Izumi {};',
+            '<?php
+namespace Foo\Bar;
+$a = new class implements \Foo\Bar\Izumi {};',
+        ];
+
+        yield 'anonymous class implements, shorten to imported name' => [
+            '<?php
+use Foo\Bar\Izumi;
+$a = new class implements Izumi {};',
+            '<?php
+use Foo\Bar\Izumi;
+$a = new class implements \Foo\Bar\Izumi {};',
+        ];
+
+        yield 'class extends and implements' => [
+            '<?php
+namespace Foo\Bar;
+class SomeClass extends A implements Izumi
+{
+}',
+            '<?php
+namespace Foo\Bar;
+class SomeClass extends \Foo\Bar\A implements \Foo\Bar\Izumi
+{
+}',
+        ];
+
+        yield 'class extends and implements multiple' => [
+            '<?php
+namespace Foo\Bar;
+class SomeClass extends A implements Izumi, A, \A\B, C
+{
+}',
+            '<?php
+namespace Foo\Bar;
+class SomeClass extends \Foo\Bar\A implements \Foo\Bar\Izumi, A, \A\B, \Foo\Bar\C
+{
+}',
+        ];
+
+        yield 'single caught exception' => [
+            '<?php use A\B; echo 1; try{ foo(999); } catch (B $z) {}',
+            '<?php use A\B; echo 1; try{ foo(999); } catch (\A\B $z) {}',
+        ];
+
+        yield 'single caught exception namespaced' => [
+            '<?php namespace B; try{ foo(999); } catch (A $z) {}',
+            '<?php namespace B; try{ foo(999); } catch (\B\A $z) {}',
+        ];
+
+        yield 'multiple caught exceptions' => [
+            '<?php namespace D; use A\B; try{ foo(); } catch (B |  \A\C  | /* 1 */  \A\D $z) {}',
+            '<?php namespace D; use A\B; try{ foo(); } catch (\A\B |  \A\C  | /* 1 */  \A\D $z) {}',
+        ];
+
+        yield 'catch in multiple namespaces' => [
+            '<?php
+namespace {
+    try{ foo(); } catch (Exception $z) {}
+    try{ foo(); } catch (A\X $z) {}
+    try{ foo(); } catch (B\Z $z) {}
+}
+namespace A {
+    try{ foo(); } catch (\Exception $z) {}
+    try{ foo(); } catch (X $z) {}
+    try{ foo(); } catch (\B\Z $z) {}
+}
+namespace B {
+    try{ foo(); } catch (\Exception $z) {}
+    try{ foo(); } catch (\A\X $z) {}
+    try{ foo(); } catch (Z $z) {}
+}
+',
+            '<?php
+namespace {
+    try{ foo(); } catch (\Exception $z) {}
+    try{ foo(); } catch (\A\X $z) {}
+    try{ foo(); } catch (\B\Z $z) {}
+}
+namespace A {
+    try{ foo(); } catch (\Exception $z) {}
+    try{ foo(); } catch (\A\X $z) {}
+    try{ foo(); } catch (\B\Z $z) {}
+}
+namespace B {
+    try{ foo(); } catch (\Exception $z) {}
+    try{ foo(); } catch (\A\X $z) {}
+    try{ foo(); } catch (\B\Z $z) {}
+}
+',
+            ['leading_backslash_in_global_namespace' => true],
+        ];
+
+        yield 'starts with but not full name extends' => [
+            '<?php namespace a\abcd;
+class Foo extends \a\abcdTest { }',
+            null,
+        ];
+
+        yield 'starts with but not full name function arg' => [
+            '<?php
+namespace Z\B\C\D
+{
+    function A(\Z\B\C\DE\Foo $fix) {}
+}
+',
+            null,
+        ];
+
+        yield 'static class reference' => [
+            '<?php
+            use ZXY\A;
+            echo A::class;
+            echo A::B();
+            echo A::class;
+            foo(A::B,A::C);
+            echo $a[A::class];
+            echo A::class?>
+            ',
+            '<?php
+            use ZXY\A;
+            echo \ZXY\A::class;
+            echo \ZXY\A::B();
+            echo \ZXY\A::class;
+            foo(\ZXY\A::B,\ZXY\A::C);
+            echo $a[\ZXY\A::class];
+            echo \ZXY\A::class?>
+            ',
+        ];
+
+        yield [
+            '<?php
+            namespace Foo\Test;
+            $this->assertSame($names, \Foo\TestMyThing::zxy(1,2));
+            ',
+            null,
+        ];
+
+        yield [
+            '<?php
+            use ZXY\A;
+            use D;
+            echo $D::CONST_VALUE;
+            echo parent::CONST_VALUE;
+            echo self::$abc;
+            echo Z::F;
+            echo X\Z::F;
+            ',
+            null,
+        ];
     }
 
     /**
@@ -375,10 +574,13 @@ class SomeClass
     }
 
     /**
+     * @param array<string, array<string, mixed>|bool> $config
+     *
      * @dataProvider provideCodeWithoutReturnTypesCases
      */
-    public function testCodeWithoutReturnTypes(string $expected, ?string $input = null): void
+    public function testCodeWithoutReturnTypes(string $expected, ?string $input = null, array $config = []): void
     {
+        $this->fixer->configure($config);
         $this->doTest($expected, $input);
     }
 
@@ -1053,12 +1255,15 @@ function foo($dateTime) {}',
     }
 
     /**
+     * @param array<string, array<string, mixed>|bool> $config
+     *
      * @requires PHP 8.0
      *
      * @dataProvider provideFix80Cases
      */
-    public function testFix80(string $expected, ?string $input = null): void
+    public function testFix80(string $expected, ?string $input = null, array $config = []): void
     {
+        $this->fixer->configure($config);
         $this->doTest($expected, $input);
     }
 
@@ -1094,6 +1299,11 @@ function foo($dateTime) {}',
             '<?php function f(): Foo|Bar|A\B\C {}',
             '<?php function f(): Foo|\Bar|\A\B\C {}',
         ];
+
+        yield 'caught exception without var' => [
+            '<?php use A\B; try{ foo(0); } catch (B) {}',
+            '<?php use A\B; try{ foo(0); } catch (\A\B) {}',
+        ];
     }
 
     /**

+ 2 - 2
tests/Runner/FileFilterIteratorTest.php

@@ -98,7 +98,7 @@ final class FileFilterIteratorTest extends TestCase
         /** @var FixerFileProcessedEvent $event */
         $event = reset($events);
 
-        self::assertInstanceOf(\PhpCsFixer\FixerFileProcessedEvent::class, $event);
+        self::assertInstanceOf(FixerFileProcessedEvent::class, $event);
         self::assertSame(FixerFileProcessedEvent::STATUS_SKIPPED, $event->getStatus());
     }
 
@@ -128,7 +128,7 @@ final class FileFilterIteratorTest extends TestCase
         /** @var FixerFileProcessedEvent $event */
         $event = reset($events);
 
-        self::assertInstanceOf(\PhpCsFixer\FixerFileProcessedEvent::class, $event);
+        self::assertInstanceOf(FixerFileProcessedEvent::class, $event);
         self::assertSame(FixerFileProcessedEvent::STATUS_SKIPPED, $event->getStatus());
     }
 

+ 1 - 1
tests/Runner/RunnerTest.php

@@ -127,7 +127,7 @@ final class RunnerTest extends TestCase
 
         $error = $errors[0];
 
-        self::assertInstanceOf(\PhpCsFixer\Error\Error::class, $error);
+        self::assertInstanceOf(Error::class, $error);
 
         self::assertSame(Error::TYPE_INVALID, $error->getType());
         self::assertSame($pathToInvalidFile, $error->getFilePath());

+ 1 - 1
tests/Tokenizer/CTTest.php

@@ -85,7 +85,7 @@ final class CTTest extends TestCase
         static $constants;
 
         if (null === $constants) {
-            $reflection = new \ReflectionClass(\PhpCsFixer\Tokenizer\CT::class);
+            $reflection = new \ReflectionClass(CT::class);
             $constants = $reflection->getConstants();
         }