From 1bc8e827fad430b0e905eed8bd3a69568e6d2408 Mon Sep 17 00:00:00 2001 From: Greg Korba Date: Fri, 15 Dec 2023 23:21:01 +0100 Subject: [PATCH] feat: Support more FQCNs cases in `fully_qualified_strict_types` (#7459) Co-authored-by: SpacePossum --- .../import/fully_qualified_strict_types.rst | 45 +++- doc/rules/index.rst | 2 +- .../Import/FullyQualifiedStrictTypesFixer.php | 188 ++++++++++++++- tests/Cache/NullCacheManagerTest.php | 4 +- tests/Cache/SignatureTest.php | 4 +- .../FullyQualifiedStrictTypesFixerTest.php | 220 +++++++++++++++++- tests/Runner/FileFilterIteratorTest.php | 4 +- tests/Runner/RunnerTest.php | 2 +- tests/Tokenizer/CTTest.php | 2 +- 9 files changed, 451 insertions(+), 20 deletions(-) diff --git a/doc/rules/import/fully_qualified_strict_types.rst b/doc/rules/import/fully_qualified_strict_types.rst index 56d2f80b6a0..09afc46e288 100644 --- a/doc/rules/import/fully_qualified_strict_types.rst +++ b/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 + `_ - 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. diff --git a/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php b/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php index 5589eea1a47..eeeceecc16d 100644 --- a/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php +++ b/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php @@ -34,24 +34,28 @@ * @author VeeWee * @author Tomas Jadrny * @author Greg Korba + * @author SpacePossum */ 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( 'baz; } + + public function doX(\Foo\Bar $foo, \Exception $e): \Foo\Bar\Baz + { + try {} + catch (\Foo\SomeException $e) {} + } } ' ), @@ -83,6 +93,23 @@ public function doY(Foo\NotImported $u, \Foo\NotImported $v) { } } +', + ['leading_backslash_in_global_namespace' => true] + ), + new CodeSample( + ' true] ), @@ -103,7 +130,14 @@ public function getPriority(): int 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 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void 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 @@ private function fixPhpDoc(Tokens $tokens, int $index, array $uses, string $name } } + /** + * @param array $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 $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 $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} $class + * @param array $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 $uses */ @@ -290,6 +454,24 @@ private function determineShortType(string $typeName, array $uses, string $names return null; } + /** + * @param array{content: string, tokens: array} $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 */ diff --git a/tests/Cache/NullCacheManagerTest.php b/tests/Cache/NullCacheManagerTest.php index 5ce26c5a154..87e221f93d8 100644 --- a/tests/Cache/NullCacheManagerTest.php +++ b/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)); } diff --git a/tests/Cache/SignatureTest.php b/tests/Cache/SignatureTest.php index 87359672624..06c66949176 100644 --- a/tests/Cache/SignatureTest.php +++ b/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)); } diff --git a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php index 33dbcc00b8e..efaccd94d49 100644 --- a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php +++ b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php @@ -28,9 +28,9 @@ final class FullyQualifiedStrictTypesFixerTest extends AbstractFixerTestCase /** * @param array $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 @@ public function testNewLogic(string $expected, ?string $input = null, array $con /** * @return iterable}> */ - public static function provideNewLogicCases(): iterable + public static function provideFixCases(): iterable { yield 'namespace === type name' => [ ' [ + ' [ + ' true], + ]; + + yield 'interface in global namespace with multiple extend' => [ + ' true], + ]; + + yield 'class implements' => [ + ' [ + ' [ + ' [ + ' [ + ' [ + ' [ + ' [ + ' [ + ' true], + ]; + + yield 'starts with but not full name extends' => [ + ' [ + ' [ + ' + ', + ' + ', + ]; + + yield [ + 'assertSame($names, \Foo\TestMyThing::zxy(1,2)); + ', + null, + ]; + + yield [ + '|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|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 @@ public static function provideFix80Cases(): iterable ' [ + 'getStatus()); } @@ -128,7 +128,7 @@ static function (FixerFileProcessedEvent $event) use (&$events): void { /** @var FixerFileProcessedEvent $event */ $event = reset($events); - self::assertInstanceOf(\PhpCsFixer\FixerFileProcessedEvent::class, $event); + self::assertInstanceOf(FixerFileProcessedEvent::class, $event); self::assertSame(FixerFileProcessedEvent::STATUS_SKIPPED, $event->getStatus()); } diff --git a/tests/Runner/RunnerTest.php b/tests/Runner/RunnerTest.php index 4a0ad1bfffc..13c9c57516f 100644 --- a/tests/Runner/RunnerTest.php +++ b/tests/Runner/RunnerTest.php @@ -127,7 +127,7 @@ public function testThatFixInvalidFileReportsToErrorManager(): void $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()); diff --git a/tests/Tokenizer/CTTest.php b/tests/Tokenizer/CTTest.php index aa4f7bebfbe..dd947fe4222 100644 --- a/tests/Tokenizer/CTTest.php +++ b/tests/Tokenizer/CTTest.php @@ -85,7 +85,7 @@ private static function getConstants(): array static $constants; if (null === $constants) { - $reflection = new \ReflectionClass(\PhpCsFixer\Tokenizer\CT::class); + $reflection = new \ReflectionClass(CT::class); $constants = $reflection->getConstants(); }