Browse Source

fix: `PhpUnitTestClassRequiresCoversFixer` - do not add annotation when there are attributes (#7880)

Kuba Werłos 11 months ago
parent
commit
8a115cd646

+ 61 - 3
src/Fixer/AbstractPhpUnitFixer.php

@@ -18,6 +18,8 @@ use PhpCsFixer\AbstractFixer;
 use PhpCsFixer\DocBlock\DocBlock;
 use PhpCsFixer\DocBlock\Line;
 use PhpCsFixer\Indicator\PhpUnitTestCaseIndicator;
+use PhpCsFixer\Tokenizer\Analyzer\AttributeAnalyzer;
+use PhpCsFixer\Tokenizer\Analyzer\NamespaceUsesAnalyzer;
 use PhpCsFixer\Tokenizer\Analyzer\WhitespacesAnalyzer;
 use PhpCsFixer\Tokenizer\CT;
 use PhpCsFixer\Tokenizer\Token;
@@ -68,16 +70,22 @@ abstract class AbstractPhpUnitFixer extends AbstractFixer
     }
 
     /**
-     * @param list<string> $preventingAnnotations
+     * @param list<string>       $preventingAnnotations
+     * @param list<class-string> $preventingAttributes
      */
-    final protected function ensureIsDockBlockWithAnnotation(
+    final protected function ensureIsDocBlockWithAnnotation(
         Tokens $tokens,
         int $index,
         string $annotation,
-        array $preventingAnnotations
+        array $preventingAnnotations,
+        array $preventingAttributes
     ): void {
         $docBlockIndex = $this->getDocBlockIndex($tokens, $index);
 
+        if (self::isPreventedByAttribute($tokens, $index, $preventingAttributes)) {
+            return;
+        }
+
         if ($this->isPHPDoc($tokens, $docBlockIndex)) {
             $this->updateDocBlockIfNeeded($tokens, $docBlockIndex, $annotation, $preventingAnnotations);
         } else {
@@ -136,6 +144,56 @@ abstract class AbstractPhpUnitFixer extends AbstractFixer
         $tokens[$docBlockIndex] = new Token([T_DOC_COMMENT, $lines]);
     }
 
+    /**
+     * @param list<class-string> $preventingAttributes
+     */
+    private static function isPreventedByAttribute(Tokens $tokens, int $index, array $preventingAttributes): bool
+    {
+        if ([] === $preventingAttributes) {
+            return false;
+        }
+
+        $attributeIndex = $tokens->getPrevMeaningfulToken($index);
+        if (!$tokens[$attributeIndex]->isGivenKind(CT::T_ATTRIBUTE_CLOSE)) {
+            return false;
+        }
+        $attributeIndex = $tokens->findBlockStart(Tokens::BLOCK_TYPE_ATTRIBUTE, $attributeIndex);
+
+        foreach (AttributeAnalyzer::collect($tokens, $attributeIndex) as $attributeAnalysis) {
+            foreach ($attributeAnalysis->getAttributes() as $attribute) {
+                if (\in_array(self::getFullyQualifiedName($tokens, $attribute['name']), $preventingAttributes, true)) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private static function getFullyQualifiedName(Tokens $tokens, string $name): string
+    {
+        $name = strtolower($name);
+
+        $names = [];
+        foreach ((new NamespaceUsesAnalyzer())->getDeclarationsFromTokens($tokens) as $namespaceUseAnalysis) {
+            $names[strtolower($namespaceUseAnalysis->getShortName())] = strtolower($namespaceUseAnalysis->getFullName());
+        }
+
+        foreach ($names as $shortName => $fullName) {
+            if ($name === $shortName) {
+                return $fullName;
+            }
+
+            if (!str_starts_with($name, $shortName.'\\')) {
+                continue;
+            }
+
+            return $fullName.substr($name, \strlen($shortName));
+        }
+
+        return $name;
+    }
+
     /**
      * @return list<Line>
      */

+ 3 - 2
src/Fixer/PhpUnit/PhpUnitInternalClassFixer.php

@@ -77,11 +77,12 @@ final class PhpUnitInternalClassFixer extends AbstractPhpUnitFixer implements Wh
             return;
         }
 
-        $this->ensureIsDockBlockWithAnnotation(
+        $this->ensureIsDocBlockWithAnnotation(
             $tokens,
             $classIndex,
             'internal',
-            ['internal']
+            ['internal'],
+            [],
         );
     }
 

+ 3 - 2
src/Fixer/PhpUnit/PhpUnitSizeClassFixer.php

@@ -72,11 +72,12 @@ final class PhpUnitSizeClassFixer extends AbstractPhpUnitFixer implements Whites
             return;
         }
 
-        $this->ensureIsDockBlockWithAnnotation(
+        $this->ensureIsDocBlockWithAnnotation(
             $tokens,
             $classIndex,
             $this->configuration['group'],
-            self::SIZES
+            self::SIZES,
+            [],
         );
     }
 

+ 6 - 2
src/Fixer/PhpUnit/PhpUnitTestClassRequiresCoversFixer.php

@@ -68,7 +68,7 @@ final class MyTest extends \PHPUnit_Framework_TestCase
             return; // don't add `@covers` annotation for abstract base classes
         }
 
-        $this->ensureIsDockBlockWithAnnotation(
+        $this->ensureIsDocBlockWithAnnotation(
             $tokens,
             $classIndex,
             'coversNothing',
@@ -76,7 +76,11 @@ final class MyTest extends \PHPUnit_Framework_TestCase
                 'covers',
                 'coversDefaultClass',
                 'coversNothing',
-            ]
+            ],
+            [
+                'phpunit\\framework\\attributes\\coversclass',
+                'phpunit\\framework\\attributes\\coversnothing',
+            ],
         );
     }
 }

+ 89 - 0
tests/Fixer/PhpUnit/PhpUnitTestClassRequiresCoversFixerTest.php

@@ -283,6 +283,95 @@ class FooTest extends \PHPUnit_Framework_TestCase {}
         ];
     }
 
+    /**
+     * @dataProvider provideFix80Cases
+     *
+     * @requires PHP 8.0
+     */
+    public function testFix80(string $expected, ?string $input = null): void
+    {
+        $this->doTest($expected, $input);
+    }
+
+    /**
+     * @return iterable<array{0: string, 1?: string}>
+     */
+    public static function provideFix80Cases(): iterable
+    {
+        yield 'already with attribute CoversClass' => [
+            <<<'PHP'
+                <?php
+                #[PHPUnit\Framework\Attributes\CoversClass(Foo::class)]
+                class FooTest extends \PHPUnit_Framework_TestCase {}
+                PHP,
+        ];
+
+        yield 'already with attribute CoversNothing' => [
+            <<<'PHP'
+                <?php
+                #[PHPUnit\Framework\Attributes\CoversNothing]
+                class FooTest extends \PHPUnit_Framework_TestCase {}
+                PHP,
+        ];
+
+        yield 'already with imported attribute' => [
+            <<<'PHP'
+                <?php
+                use PHPUnit\Framework\TestCas;
+                use PHPUnit\Framework\Attributes\CoversClass;
+                #[CoversClass(Foo::class)]
+                class FooTest extends TestCas {}
+                PHP,
+        ];
+
+        yield 'already with partially imported attribute' => [
+            <<<'PHP'
+                <?php
+                use PHPUnit\Framework\Attributes;
+                #[Attributes\CoversClass(Foo::class)]
+                class FooTest extends \PHPUnit_Framework_TestCase {}
+                PHP,
+        ];
+
+        yield 'already with aliased attribute' => [
+            <<<'PHP'
+                <?php
+                use PHPUnit\Framework\Attributes\CoversClass as PHPUnitCoversClass;
+                #[PHPUnitCoversClass(Foo::class)]
+                class FooTest extends \PHPUnit_Framework_TestCase {}
+                PHP,
+        ];
+
+        yield 'already with partially aliased attribute' => [
+            <<<'PHP'
+                <?php
+                use PHPUnit\Framework\Attributes as PHPUnitAttributes;
+                #[PHPUnitAttributes\CoversClass(Foo::class)]
+                class FooTest extends \PHPUnit_Framework_TestCase {}
+                PHP,
+        ];
+
+        yield 'with attribute from different namespace' => [
+            <<<'PHP'
+                <?php
+                use Foo\CoversClass;
+                use PHPUnit\Framework\Attributes\CoversClass as PHPUnitCoversClass;
+                /**
+                 * @coversNothing
+                 */
+                #[CoversClass(Foo::class)]
+                class FooTest extends \PHPUnit_Framework_TestCase {}
+                PHP,
+            <<<'PHP'
+                <?php
+                use Foo\CoversClass;
+                use PHPUnit\Framework\Attributes\CoversClass as PHPUnitCoversClass;
+                #[CoversClass(Foo::class)]
+                class FooTest extends \PHPUnit_Framework_TestCase {}
+                PHP,
+        ];
+    }
+
     /**
      * @dataProvider provideFix82Cases
      *