From 11d83044907dc82d1571452ec52a05e3c04b7f9c Mon Sep 17 00:00:00 2001 From: JADRNT01 Date: Mon, 12 Apr 2021 09:52:26 +0200 Subject: [PATCH 1/7] Add PHPDoc support for `fully_qualified_strict_types fixer` Initial work was done in https://github.com/Jadro007/PHP-CS-Fixer/commit/7acec70a16141832a2eef2b29a5cb7446f460766 but since the branch became outdated, it couldn't be just rebased - it required significant changes to make it work again. It wasn't even possible to commit not working version and to provide changes afterwards, because there were huge conflicts and keeping the old code would end up with a mess. I decided to fix this up during rebase, so it's working as it was working in proposed PR (still needs improvements, but these will be done separately). --- .../import/fully_qualified_strict_types.rst | 9 +- .../Import/FullyQualifiedStrictTypesFixer.php | 141 +++++++++---- .../FullyQualifiedStrictTypesFixerTest.php | 198 ++++++++++++++++++ ...urn_type,fully_qualified_strict_types.test | 2 +- 4 files changed, 302 insertions(+), 48 deletions(-) diff --git a/doc/rules/import/fully_qualified_strict_types.rst b/doc/rules/import/fully_qualified_strict_types.rst index 12ab3d80ee4..f8980b728e6 100644 --- a/doc/rules/import/fully_qualified_strict_types.rst +++ b/doc/rules/import/fully_qualified_strict_types.rst @@ -34,7 +34,14 @@ Example #1 use Foo\Bar; use Foo\Bar\Baz; - + use Foo\Bar\Bam; + + /** + - * @see \Foo\Bar\Baz + - * @see \Foo\Bar\Bam + + * @see Baz + + * @see Bam + */ class SomeClass { - public function doX(\Foo\Bar $foo): \Foo\Bar\Baz diff --git a/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php b/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php index e9f04f3c976..c01e4fd5912 100644 --- a/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php +++ b/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php @@ -22,6 +22,7 @@ use PhpCsFixer\FixerDefinition\CodeSample; use PhpCsFixer\FixerDefinition\FixerDefinition; use PhpCsFixer\FixerDefinition\FixerDefinitionInterface; +use PhpCsFixer\Preg; use PhpCsFixer\Tokenizer\Analyzer\Analysis\TypeAnalysis; use PhpCsFixer\Tokenizer\Analyzer\FunctionsAnalyzer; use PhpCsFixer\Tokenizer\Analyzer\NamespaceUsesAnalyzer; @@ -31,6 +32,8 @@ /** * @author VeeWee + * @author Tomas Jadrny + * @author Greg Korba */ final class FullyQualifiedStrictTypesFixer extends AbstractFixer implements ConfigurableFixerInterface { @@ -44,7 +47,12 @@ public function getDefinition(): FixerDefinitionInterface use Foo\Bar; use Foo\Bar\Baz; +use Foo\Bar\Bam; +/** + * @see \Foo\Bar\Baz + * @see \Foo\Bar\Bam + */ class SomeClass { public function doX(\Foo\Bar $foo): \Foo\Bar\Baz @@ -86,7 +94,7 @@ public function getPriority(): int public function isCandidate(Tokens $tokens): bool { - return $tokens->isTokenKindFound(T_FUNCTION); + return $tokens->isAnyTokenKindsFound([T_FUNCTION, T_DOC_COMMENT]); } protected function createConfigurationDefinition(): FixerConfigurationResolverInterface @@ -119,6 +127,10 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void if ($tokens[$index]->isGivenKind(T_FUNCTION)) { $this->fixFunction($functionsAnalyzer, $tokens, $index, $uses, $namespaceName); } + + if ($tokens[$index]->isGivenKind(T_DOC_COMMENT)) { + $this->fixPhpDoc($tokens, $index, $uses, $namespaceName); + } } } } @@ -145,6 +157,28 @@ private function fixFunction(FunctionsAnalyzer $functionsAnalyzer, Tokens $token } } + /** + * @param array $uses + */ + private function fixPhpDoc(Tokens $tokens, int $index, array $uses, string $namespaceName): void + { + $phpDoc = $tokens[$index]; + $phpDocContent = $phpDoc->getContent(); + Preg::matchAll('#@([^\s]+)\s+([^\s]+)#', $phpDocContent, $matches); + + if ([] !== $matches) { + foreach ($matches[2] as $typeName) { + $shortTokens = $this->determineShortType($typeName, $uses, $namespaceName); + + if (null !== $shortTokens) { + $phpDocContent = str_replace($typeName, $shortTokens[0]->getContent(), $phpDocContent); + } + } + + $tokens[$index] = new Token([T_DOC_COMMENT, $phpDocContent]); + } + } + /** * @param array $uses */ @@ -156,7 +190,6 @@ private function replaceByShortType(Tokens $tokens, TypeAnalysis $type, array $u $typeStartIndex = $tokens->getNextMeaningfulToken($typeStartIndex); } - $namespaceNameLength = \strlen($namespaceName); $types = $this->getTypes($tokens, $typeStartIndex, $type->getEndIndex()); foreach ($types as $typeName => [$startIndex, $endIndex]) { @@ -164,60 +197,76 @@ private function replaceByShortType(Tokens $tokens, TypeAnalysis $type, array $u return; } - $withLeadingBackslash = str_starts_with($typeName, '\\'); - if ($withLeadingBackslash) { - $typeName = substr($typeName, 1); + $shortType = $this->determineShortType($typeName, $uses, $namespaceName); + + if (null !== $shortType) { + $tokens->overrideRange($startIndex, $endIndex, $shortType); } - $typeNameLower = strtolower($typeName); + } + } - if (isset($uses[$typeNameLower]) && ($withLeadingBackslash || '' === $namespaceName)) { - // if the type without leading "\" equals any of the full "uses" long names, it can be replaced with the short one - $tokens->overrideRange($startIndex, $endIndex, $this->namespacedStringToTokens($uses[$typeNameLower])); + /** + * Determines short type based on FQCN, current namespace and imports (`use` declarations). + * + * @param array $uses + * + * @return null|Token[] + */ + private function determineShortType(string $typeName, array $uses, string $namespaceName): ?array + { + $withLeadingBackslash = str_starts_with($typeName, '\\'); + if ($withLeadingBackslash) { + $typeName = substr($typeName, 1); + } + $typeNameLower = strtolower($typeName); + $namespaceNameLength = \strlen($namespaceName); - continue; - } + if (isset($uses[$typeNameLower]) && ($withLeadingBackslash || '' === $namespaceName)) { + // if the type without leading "\" equals any of the full "uses" long names, it can be replaced with the short one + return $this->namespacedStringToTokens($uses[$typeNameLower]); + } - if ('' === $namespaceName) { - foreach ($uses as $useShortName) { - if (strtolower($useShortName) === $typeNameLower) { - continue 2; - } + if ('' === $namespaceName) { + // if we are in the global namespace and the type is not imported the leading '\' can be removed (TODO nice config candidate) + foreach ($uses as $useShortName) { + if (strtolower($useShortName) === $typeNameLower) { + return null; } + } - // if we are in the global namespace and the type is not imported, - // we enforce/remove leading backslash (depending on the configuration) - if (true === $this->configuration['leading_backslash_in_global_namespace']) { - if (!$withLeadingBackslash && !isset($uses[$typeNameLower])) { - $tokens->overrideRange( - $startIndex, - $endIndex, - $this->namespacedStringToTokens($typeName, true) - ); - } - } else { - $tokens->overrideRange($startIndex, $endIndex, $this->namespacedStringToTokens($typeName)); + // if we are in the global namespace and the type is not imported, + // we enforce/remove leading backslash (depending on the configuration) + if (true === $this->configuration['leading_backslash_in_global_namespace']) { + if (!$withLeadingBackslash && !isset($uses[$typeNameLower])) { + return $this->namespacedStringToTokens($typeName, true); } - } elseif (!str_contains($typeName, '\\')) { - // If we're NOT in the global namespace, there's no related import, - // AND used type is from global namespace, then it can't be shortened. - continue; - } elseif ($typeNameLower !== $namespaceName && str_starts_with($typeNameLower, $namespaceName.'\\')) { - // if the type starts with namespace and the type is not the same as the namespace it can be shortened - $typeNameShort = substr($typeName, $namespaceNameLength + 1); - - // if short names are the same, but long one are different then it cannot be shortened - foreach ($uses as $useLongName => $useShortName) { - if ( - strtolower($typeNameShort) === strtolower($useShortName) - && strtolower($typeName) !== strtolower($useLongName) - ) { - continue 2; - } + } else { + return $this->namespacedStringToTokens($typeName); + } + } + if (!str_contains($typeName, '\\')) { + // If we're NOT in the global namespace, there's no related import, + // AND used type is from global namespace, then it can't be shortened. + return null; + } + if ($typeNameLower !== $namespaceName && str_starts_with($typeNameLower, $namespaceName.'\\')) { + // if the type starts with namespace and the type is not the same as the namespace it can be shortened + $typeNameShort = substr($typeName, $namespaceNameLength + 1); + + // if short names are the same, but long one are different then it cannot be shortened + foreach ($uses as $useLongName => $useShortName) { + if ( + strtolower($typeNameShort) === strtolower($useShortName) + && strtolower($typeName) !== strtolower($useLongName) + ) { + return null; } - - $tokens->overrideRange($startIndex, $endIndex, $this->namespacedStringToTokens($typeNameShort)); } + + return $this->namespacedStringToTokens($typeNameShort); } + + return null; } /** diff --git a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php index 82d497c6dc6..24542e5b911 100644 --- a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php +++ b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php @@ -687,6 +687,204 @@ public function three(Three\Four $four): ?Two\Three ]; } + /** + * @dataProvider provideCodeWithPhpDocCases + */ + public function testCodeWithPhpDoc(string $expected, ?string $input = null): void + { + $this->doTest($expected, $input); + } + + /** + * @return iterable + */ + public static function provideCodeWithPhpDocCases(): iterable + { + yield 'Test class PHPDoc fixes' => [ + ' [ + ' [ + ' [ + ' [ + ' [ + ' Date: Mon, 18 Sep 2023 07:20:16 +0200 Subject: [PATCH 2/7] Improve test cases - change `@var` to `@param` where applicable - introduce proper `@var` usages - add annotations with sub-namespace (shortened version contains `\`) - add test case for `@covers` which must be skipped - add empty lines for readability --- .../FullyQualifiedStrictTypesFixerTest.php | 101 ++++++++++++------ 1 file changed, 70 insertions(+), 31 deletions(-) diff --git a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php index 24542e5b911..cad2fe63c11 100644 --- a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php +++ b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php @@ -696,88 +696,112 @@ public function testCodeWithPhpDoc(string $expected, ?string $input = null): voi } /** - * @return iterable + * @return iterable */ public static function provideCodeWithPhpDocCases(): iterable { yield 'Test class PHPDoc fixes' => [ ' [ ' [ ' [ ' [ ' [ ' [ + ' Date: Mon, 18 Sep 2023 07:23:05 +0200 Subject: [PATCH 3/7] Improve implementation - Shorten types only in specific allowed annotations - support for sub-namespaces (re-glue tokens instead of using hardcoded first one) - fix tokenizing namespaced string (trailing `\` was added when initial array was filtered) --- .../Import/FullyQualifiedStrictTypesFixer.php | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php b/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php index c01e4fd5912..ba510c60d9c 100644 --- a/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php +++ b/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php @@ -167,11 +167,22 @@ private function fixPhpDoc(Tokens $tokens, int $index, array $uses, string $name Preg::matchAll('#@([^\s]+)\s+([^\s]+)#', $phpDocContent, $matches); if ([] !== $matches) { - foreach ($matches[2] as $typeName) { + foreach ($matches[2] as $i => $typeName) { + if (!\in_array($matches[1][$i], ['param', 'return', 'see', 'var'], true)) { + continue; + } + $shortTokens = $this->determineShortType($typeName, $uses, $namespaceName); if (null !== $shortTokens) { - $phpDocContent = str_replace($typeName, $shortTokens[0]->getContent(), $phpDocContent); + $phpDocContent = str_replace( + $typeName, + implode('', array_map( + static fn (Token $token) => $token->getContent(), + $shortTokens + )), + $phpDocContent + ); } } From f17209cb3bd1a7d530a2dc62dfa334b641460b6a Mon Sep 17 00:00:00 2001 From: Greg Korba Date: Mon, 18 Sep 2023 08:24:55 +0200 Subject: [PATCH 4/7] Add test case for shortening phpDoc types for aliased imports --- .../FullyQualifiedStrictTypesFixerTest.php | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php index cad2fe63c11..608da00f54b 100644 --- a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php +++ b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php @@ -922,6 +922,97 @@ public function doSomething(\Foo\Bar\SomeClass $foo, \Foo\Bar\Buz $buz): \Foo\Ba */ class SomeClassTest {}', ]; + + yield 'Imports with aliases' => [ + 'baz = $baz; + $this->bam = $bam; + } + + /** + * @return Buzz + */ + public function getBaz() { + return $this->baz; + } + + /** + * @return Boom + */ + public function getBam() { + return $this->bam; + } +}', + 'baz = $baz; + $this->bam = $bam; + } + + /** + * @return \Foo\Bar\Baz + */ + public function getBaz() { + return $this->baz; + } + + /** + * @return \Foo\Bar\Bam + */ + public function getBam() { + return $this->bam; + } +}', + ]; } /** From 5f393bf0e77ce132bfd52d7f0ffc7cf9671caed1 Mon Sep 17 00:00:00 2001 From: Greg Korba Date: Mon, 18 Sep 2023 09:02:17 +0200 Subject: [PATCH 5/7] Better code sample for docs --- .../import/fully_qualified_strict_types.rst | 27 ++++++++++++------- .../Import/FullyQualifiedStrictTypesFixer.php | 21 ++++++++++----- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/doc/rules/import/fully_qualified_strict_types.rst b/doc/rules/import/fully_qualified_strict_types.rst index f8980b728e6..56d2f80b6a0 100644 --- a/doc/rules/import/fully_qualified_strict_types.rst +++ b/doc/rules/import/fully_qualified_strict_types.rst @@ -34,24 +34,33 @@ Example #1 use Foo\Bar; use Foo\Bar\Baz; - use Foo\Bar\Bam; /** - * @see \Foo\Bar\Baz - - * @see \Foo\Bar\Bam + * @see Baz - + * @see Bam */ class SomeClass { - - public function doX(\Foo\Bar $foo): \Foo\Bar\Baz - + public function doX(Bar $foo): Baz - { + /** + - * @var \Foo\Bar\Baz + + * @var Baz + */ + public $baz; + + /** + - * @param \Foo\Bar\Baz $baz + + * @param Baz $baz + */ + public function __construct($baz) { + $this->baz = $baz; } - - public function doY(Foo\NotImported $u, \Foo\NotImported $v) - + public function doY(Foo\NotImported $u, Foo\NotImported $v) - { + /** + - * @return \Foo\Bar\Baz + + * @return Baz + */ + public function getBaz() { + return $this->baz; } } diff --git a/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php b/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php index ba510c60d9c..8542fdd1732 100644 --- a/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php +++ b/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php @@ -47,20 +47,29 @@ public function getDefinition(): FixerDefinitionInterface use Foo\Bar; use Foo\Bar\Baz; -use Foo\Bar\Bam; /** * @see \Foo\Bar\Baz - * @see \Foo\Bar\Bam */ class SomeClass { - public function doX(\Foo\Bar $foo): \Foo\Bar\Baz - { + /** + * @var \Foo\Bar\Baz + */ + public $baz; + + /** + * @param \Foo\Bar\Baz $baz + */ + public function __construct($baz) { + $this->baz = $baz; } - public function doY(Foo\NotImported $u, \Foo\NotImported $v) - { + /** + * @return \Foo\Bar\Baz + */ + public function getBaz() { + return $this->baz; } } ' From a7d303e28e39e325545818489b6612627f46ae15 Mon Sep 17 00:00:00 2001 From: Greg Korba Date: Tue, 12 Dec 2023 02:06:35 +0100 Subject: [PATCH 6/7] Add missing iterable types for data providers --- phpstan.dist.neon | 2 +- .../FullyQualifiedStrictTypesFixerTest.php | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/phpstan.dist.neon b/phpstan.dist.neon index bc0d0554113..15cd9324da3 100644 --- a/phpstan.dist.neon +++ b/phpstan.dist.neon @@ -21,7 +21,7 @@ parameters: - message: '#^Method PhpCsFixer\\Tests\\.+::provide.+Cases\(\) return type has no value type specified in iterable type iterable\.$#' path: tests - count: 1120 + count: 1113 - message: '#Call to static method .+ with .+ will always evaluate to true.$#' diff --git a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php index 608da00f54b..bb00372729c 100644 --- a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php +++ b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php @@ -36,6 +36,9 @@ public function testNewLogic(string $expected, ?string $input = null, array $con $this->doTest($expected, $input); } + /** + * @return iterable}> + */ public static function provideNewLogicCases(): iterable { yield 'namespace === type name' => [ @@ -164,6 +167,9 @@ public function testCodeWithReturnTypes(string $expected, ?string $input = null) $this->doTest($expected, $input); } + /** + * @return iterable + */ public static function provideCodeWithReturnTypesCases(): iterable { yield 'Import common strict types' => [ @@ -376,6 +382,9 @@ public function testCodeWithoutReturnTypes(string $expected, ?string $input = nu $this->doTest($expected, $input); } + /** + * @return iterable + */ public static function provideCodeWithoutReturnTypesCases(): iterable { yield 'import from namespace and global' => [ @@ -635,6 +644,9 @@ public function __construct( ]; } + /** + * @return iterable + */ public static function provideCodeWithReturnTypesCasesWithNullableCases(): iterable { yield 'Test namespace fixes with nullable types' => [ @@ -1025,6 +1037,9 @@ public function testFix80(string $expected, ?string $input = null): void $this->doTest($expected, $input); } + /** + * @return iterable + */ public static function provideFix80Cases(): iterable { yield [ @@ -1069,6 +1084,9 @@ public function testFix81(string $expected, ?string $input = null, array $config $this->doTest($expected, $input); } + /** + * @return iterable}> + */ public static function provideFix81Cases(): iterable { yield [ @@ -1123,6 +1141,9 @@ public function testFix82(string $expected, ?string $input = null, array $config $this->doTest($expected, $input); } + /** + * @return iterable}> + */ public static function provideFix82Cases(): iterable { yield 'simple param in global namespace without use' => [ From dbe0e27be1a36c6852944aa170d899faeac0933d Mon Sep 17 00:00:00 2001 From: Greg Korba Date: Tue, 12 Dec 2023 02:29:34 +0100 Subject: [PATCH 7/7] Support phpDoc types with leading backslash enforced --- .../Import/FullyQualifiedStrictTypesFixer.php | 7 +++-- .../FullyQualifiedStrictTypesFixerTest.php | 29 +++++++++++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php b/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php index 8542fdd1732..5589eea1a47 100644 --- a/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php +++ b/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php @@ -177,16 +177,17 @@ private function fixPhpDoc(Tokens $tokens, int $index, array $uses, string $name if ([] !== $matches) { foreach ($matches[2] as $i => $typeName) { - if (!\in_array($matches[1][$i], ['param', 'return', 'see', 'var'], true)) { + if (!\in_array($matches[1][$i], ['param', 'return', 'see', 'throws', 'var'], true)) { continue; } $shortTokens = $this->determineShortType($typeName, $uses, $namespaceName); if (null !== $shortTokens) { + // Replace tag+type in order to avoid replacing type multiple times (when same type is used in multiple places) $phpDocContent = str_replace( - $typeName, - implode('', array_map( + $matches[0][$i], + '@'.$matches[1][$i].' '.implode('', array_map( static fn (Token $token) => $token->getContent(), $shortTokens )), diff --git a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php index bb00372729c..33dbcc00b8e 100644 --- a/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php +++ b/tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php @@ -700,15 +700,18 @@ public function three(Three\Four $four): ?Two\Three } /** + * @param array $config + * * @dataProvider provideCodeWithPhpDocCases */ - public function testCodeWithPhpDoc(string $expected, ?string $input = null): void + public function testCodeWithPhpDoc(string $expected, ?string $input = null, array $config = []): void { + $this->fixer->configure($config); $this->doTest($expected, $input); } /** - * @return iterable + * @return iterable}> */ public static function provideCodeWithPhpDocCases(): iterable { @@ -1025,6 +1028,28 @@ public function getBam() { } }', ]; + + yield 'Leading backslash in global namespace' => [ + ' true], + ]; } /**