Merge pull request moodlehq#167 from stronk7/report_multiple_covers_d…

…efault_class Detect multiple @coversDefaultClass uses in a class
stronk7 · Jun 11, 2024 · 3961e3c · 3961e3c
2 parents 4f1bc63 + 9c1a12f
commit 3961e3c
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 The format of this change log follows the advice given at [Keep a CHANGELOG](https://keepachangelog.com).
 
 ## [Unreleased]
+### Added
+- The existing `moodle.PHPUnit.TestCaseCovers` sniff now detects multiple uses of the `@coversDefaultClass` annotation. Only one is allowed by class. 
 
 ## [v3.4.7] - 2024-05-31
 ### Added

diff --git a/moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php b/moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php
@@ -85,6 +85,7 @@ public function process(File $file, $pointer) {
             $class = $file->getDeclarationName($cStart);
             $classCovers = false; // To control when the class has a @covers tag.
             $classCoversNothing = false; // To control when the class has a @coversNothing tag.
+            $classCoversDefaultClass = []; // To annotate all the existing @coversDefaultClass tags.
 
             // Only if the class is extending something.
             // TODO: We could add a list of valid classes once we have a class-map available.
@@ -121,12 +122,30 @@ public function process(File $file, $pointer) {
                             case '@coversDefaultClass':
                                 // Validate basic syntax (FQCN).
                                 $this->checkCoversTagsSyntax($file, $docPointer, '@coversDefaultClass');
+                                $classCoversDefaultClass[] = $docPointer; // Annotated for later checks.
                                 break;
                         }
                     }
                 }
             }
 
+            // If we have found more than one @coversDefaultClass, that's an error.
+            if (count($classCoversDefaultClass) > 1) {
+                // We have to reverse the array to get them in correct order and then
+                // remove the 1st one that is correct/allowed.
+                $classCoversDefaultClass = array_reverse($classCoversDefaultClass);
+                array_shift($classCoversDefaultClass);
+                // Report the remaining ones.
+                foreach ($classCoversDefaultClass as $classCoversDefaultClassPointer) {
+                    $file->addError(
+                        'Class %s has more than one @coversDefaultClass tag, only one allowed',
+                        $classCoversDefaultClassPointer,
+                        'MultipleDefaultClass',
+                        [$class]
+                    );
+                }
+            }
+
             // Both @covers and @coversNothing, that's a mistake. 2 errors.
             if ($classCovers && $classCoversNothing) {
                 $file->addError(

diff --git a/moodle/Tests/PHPUnitTestCaseCoversTest.php b/moodle/Tests/PHPUnitTestCaseCoversTest.php
@@ -93,9 +93,18 @@ public function phpunitTestCaseCoversProvider() {
             'CoversDefaultClass' => [
                 'fixture' => 'fixtures/phpunit/testcasecovers_coversdefaultclass.php',
                 'errors' => [
-                    8 => 'Wrong @coversDefaultClass annotation, it must be FQCN (\\ prefixed)',
-                    9 => 'TestCaseCovers.WrongMethod',
-                    10 => '@coversDefaultClass annotation, it must contain some value',
+                    8 => [
+                        'Wrong @coversDefaultClass annotation, it must be FQCN (\\ prefixed)',
+                        'Class coversdefaultclass_test has more than one @coversDefaultClass tag',
+                    ],
+                    9 => [
+                        'TestCaseCovers.WrongMethod',
+                        'TestCaseCovers.MultipleDefaultClass',
+                    ],
+                    10 => [
+                        '@coversDefaultClass annotation, it must contain some value',
+                        'Class coversdefaultclass_test has more than one @coversDefaultClass tag',
+                    ],
                     14 => 'test_something() has @coversDefaultClass tag',
                     15 => 'TestCaseCovers.DefaultClassNotAllowed',
                     16 => 'TestCaseCovers.DefaultClassNotAllowed',

diff --git a/moodle/Tests/fixtures/phpunit/testcasecovers_coversdefaultclass.php b/moodle/Tests/fixtures/phpunit/testcasecovers_coversdefaultclass.php
@@ -1,4 +1,4 @@
-i<?php
+<?php
 defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures.
 
 // Wrong @coversDefaultClass.