Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect multiple @coversDefaultClass uses in a class #167

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand Down
15 changes: 12 additions & 3 deletions moodle/Tests/PHPUnitTestCaseCoversTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
i<?php
<?php
defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures.

// Wrong @coversDefaultClass.
Expand Down