From 9c1a12f395185d9c1e4493c8a87980362be3a813 Mon Sep 17 00:00:00 2001
From: "Eloy Lafuente (stronk7)" <stronk7@moodle.org>
Date: Mon, 10 Jun 2024 17:39:22 +0200
Subject: [PATCH] Detect multiple @coversDefaultClass uses in a class

---
 CHANGELOG.md                                  |  2 ++
 moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php | 19 +++++++++++++++++++
 moodle/Tests/PHPUnitTestCaseCoversTest.php    | 15 ++++++++++++---
 .../testcasecovers_coversdefaultclass.php     |  2 +-
 4 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 148fb981..a12d0e24 100644
--- 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
index de5dece5..0d77d5f4 100644
--- 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
index a5732a5a..f7024dfa 100644
--- 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
index 1cc6dacc..85cd8de3 100644
--- 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.