Skip to content

Commit

Permalink
Merge pull request #330 from Yoast/JRF/yoastcs-coverstag-various-impr…
Browse files Browse the repository at this point in the history 
…ovements

Commenting/CoversTag: flag deprecated syntaxes, bug fix and various other improvements
  • Loading branch information
@jrfnl
jrfnl authored Nov 4, 2023
2 parents ae39773 + cd94f23 commit 9d00928
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 57 deletions.
48 changes: 42 additions & 6 deletions Yoast/Docs/Commenting/CoversTagStandard.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
>
<standard>
<![CDATA[
The @covers tag used to annotate which code is coverage by a test should follow the specifications of PHPUnit with regards to the supported annotations.
See:
* https://phpunit.readthedocs.io/en/7.5/code-coverage-analysis.html#specifying-covered-code-parts
* https://phpunit.readthedocs.io/en/7.5/annotations.html#covers
The @covers tag used to annotate which code is covered by a test should follow the specifications of PHPUnit with regards to the supported annotations.
See:
* https://docs.phpunit.de/en/9.6/code-coverage-analysis.html#specifying-covered-code-parts
* https://docs.phpunit.de/en/9.6/annotations.html#covers
]]>
</standard>
<code_comparison>
Expand All @@ -24,7 +25,7 @@ class Some_Test extends TestCase {
}
]]>
</code>
<code title="Invalid: Incorrect covers tag annotation.">
<code title="Invalid: Incorrect covers tag annotation (superfluous parentheses).">
<![CDATA[
class Some_Test extends TestCase {
/**
Expand Down Expand Up @@ -73,7 +74,7 @@ class Some_Test extends TestCase {
</code_comparison>
<standard>
<![CDATA[
There should be not be both a @covers tag as well as a @coversNothing tag for the same test.
The @covers tag and the @coversNothing tag are mutually exclusive and should never both occur in the same docblock.
]]>
</standard>
<code_comparison>
Expand Down Expand Up @@ -106,6 +107,41 @@ class Some_Test extends TestCase {
* @covers ::globalFunction</em>
*/
function test_something() {}
}
]]>
</code>
</code_comparison>
<standard>
<![CDATA[
The use of @covers ClassName::<[!](public|protected|private)> tags is deprecated since PHPUnit 9.0 and support has been removed in PHPUnit 10.0.
These type of annotations should not be used.
]]>
</standard>
<code_comparison>
<code title="Valid: A @covers tag referencing a function or a method.">
<![CDATA[
class Some_Test extends TestCase {
/**
* Testing...
*
* @covers <em>::globalFunction</em>
* @covers <em>\ClassName::methodName</em>
*/
function test_something() {}
}
]]>
</code>
<code title="Invalid: A @covers tag using one of the deprecated/removed function group formats.">
<![CDATA[
class Some_Test extends TestCase {
/**
* Testing...
*
* @covers <em>\ClassName::<public></em>
* @covers <em>\ClassName::<!protected></em>
* @covers <em>\ClassName<extended></em>
*/
function test_something() {}
}
]]>
</code>
Expand Down
61 changes: 52 additions & 9 deletions Yoast/Sniffs/Commenting/CoversTagSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
* - each @covers tag has an annotation;
* - there are no duplicate @covers tags;
* - there are no duplicate @coversNothing tags;
* - a method does not have both a @covers as well as a @coversNothing tag.
* - a method does not have both a @covers as well as a @coversNothing tag;
* - deprecated @covers tag formats are flagged. (since 3.0.0)
*
* @since 1.3.0
*/
Expand All @@ -24,7 +25,27 @@ final class CoversTagSniff implements Sniff {
*
* @var string
*/
private const VALID_CONTENT_REGEX = '(?:\\\\?(?:(?<OOName>[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\\\\)*(?P>OOName)(?:<extended>|::<[!]?(?:public|protected|private)>|::(?<functionName>(?!public$|protected$|private$)(?P>OOName)))?|::(?P>functionName)|\\\\?(?:(?P>OOName)\\\\)+(?P>functionName))';
private const VALID_CONTENT_REGEX = '(?:\\\\?(?:(?<OOName>[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\\\\)*(?P>OOName)(?:<extended>|::<[!]?(?:public|protected|private)>|::(?<functionName>(?!public$|protected$|private$)(?P>OOName)))?|::(?P>functionName)|::<[!]?(?:public|protected|private)>|\\\\?(?:(?P>OOName)\\\\)+(?P>functionName))';

/**
* Regex to check for deprecated `@covers ClassName<extended>` tag format.
*
* @link https://github.com/sebastianbergmann/phpunit/issues/3630 PHPUnit 9.0 deprecation.
* @link https://github.com/sebastianbergmann/phpunit/issues/3631 PHPUnit 10.0 removal.
*
* @var string
*/
private const DEPRECATED_FORMAT_EXTENDED = '`^(?:\\\\)?(?:(?<OOName>[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\\\\)*(?P>OOName)<extended>$`';

/**
* Regex to check for deprecated `@covers *::<[!]public|protected|private>` tag format.
*
* @link https://github.com/sebastianbergmann/phpunit/issues/3630 PHPUnit 9.0 deprecation.
* @link https://github.com/sebastianbergmann/phpunit/issues/3631 PHPUnit 10.0 removal.
*
* @var string
*/
private const DEPRECATED_FORMAT_VISIBILITY = '`::<[!]?(?:public|protected|private)>$`';

/**
* Base error message.
Expand Down Expand Up @@ -57,6 +78,9 @@ public function register() {
public function process( File $phpcsFile, $stackPtr ) {
$tokens = $phpcsFile->getTokens();

/*
* Find all relevant tags and check for common mistakes in the tag format.
*/
$firstCoversTag = false;
$coversTags = [];
$coversNothingTags = [];
Expand All @@ -76,7 +100,8 @@ public function process( File $phpcsFile, $stackPtr ) {

// Found a @covers tag.
$next = $phpcsFile->findNext( \T_DOC_COMMENT_WHITESPACE, ( $tag + 1 ), null, true );
if ( $tokens[ $next ]['code'] !== \T_DOC_COMMENT_STRING
if ( $next === false // Shouldn't be possible.
|| $tokens[ $next ]['code'] !== \T_DOC_COMMENT_STRING
|| $tokens[ $next ]['line'] !== $tokens[ $tag ]['line']
) {
$phpcsFile->addError(
Expand All @@ -91,6 +116,16 @@ public function process( File $phpcsFile, $stackPtr ) {
$annotation = $tokens[ $next ]['content'];
$coversTags[ "$tag-$next" ] = $annotation;

// Check for deprecated/removed @covers formats.
if ( \preg_match( self::DEPRECATED_FORMAT_EXTENDED, $annotation ) === 1
|| \preg_match( self::DEPRECATED_FORMAT_VISIBILITY, $annotation ) === 1
) {
$warning = 'Use of the "ClassName<*>" type values for @covers annotations has been deprecated in PHPUnit 9.0';
$warning .= ' and support has been removed in PHPUnit 10.0. Found: %s';
$data = [ $annotation ];
$phpcsFile->addWarning( $warning, $next, 'RemovedFormat', $data );
}

if ( \preg_match( '`^' . self::VALID_CONTENT_REGEX . '$`', $annotation ) === 1 ) {
continue;
}
Expand Down Expand Up @@ -146,6 +181,9 @@ public function process( File $phpcsFile, $stackPtr ) {
$phpcsFile->addError( $error, $next, 'Invalid', $data );
}

/*
* Check that a docblock doesn't contain both `@covers` tags as well as `@coversNothing` tag(s).
*/
$coversNothingCount = \count( $coversNothingTags );
if ( $firstCoversTag !== false && $coversNothingCount > 0 ) {
$error = 'A test can\'t both cover something as well as cover nothing. First @coversNothing tag encountered on line %d; first @covers tag encountered on line %d';
Expand All @@ -157,6 +195,9 @@ public function process( File $phpcsFile, $stackPtr ) {
$phpcsFile->addError( $error, $tokens[ $stackPtr ]['comment_closer'], 'Contradictory', $data );
}

/*
* Check for duplicate `@coversNothing` tags.
*/
if ( $coversNothingCount > 1 ) {
$error = 'Only one @coversNothing tag allowed per test';
$code = 'DuplicateCoversNothing';
Expand All @@ -177,7 +218,6 @@ public function process( File $phpcsFile, $stackPtr ) {
$phpcsFile->addError( $error, $tokens[ $stackPtr ]['comment_closer'], $code );
}
else {

$fix = $phpcsFile->addFixableError( $error, $tokens[ $stackPtr ]['comment_closer'], $code );
if ( $fix === true ) {
$skipFirst = ( $coversNothingCount === $removalCount );
Expand Down Expand Up @@ -205,6 +245,9 @@ public function process( File $phpcsFile, $stackPtr ) {
}
}

/*
* Check for duplicate `@covers ...` tags.
*/
$coversCount = \count( $coversTags );
if ( $coversCount > 1 ) {
$unique = \array_unique( $coversTags );
Expand All @@ -218,6 +261,7 @@ public function process( File $phpcsFile, $stackPtr ) {
}

$first = null;
$data = [];
foreach ( $coversTags as $ptrs => $annot ) {
if ( $annotation !== $annot ) {
continue;
Expand All @@ -231,13 +275,12 @@ public function process( File $phpcsFile, $stackPtr ) {

$ptrs = \explode( '-', $ptrs );

$fix = $phpcsFile->addFixableError( $error, $ptrs[0], $code, $data );
$fix = $phpcsFile->addFixableError( $error, (int) $ptrs[0], $code, $data );
if ( $fix === true ) {

$phpcsFile->fixer->beginChangeset();

// Remove the whole line.
for ( $i = ( $ptrs[1] ); $i >= 0; $i-- ) {
for ( $i = (int) $ptrs[1]; $i >= 0; $i-- ) {
if ( $tokens[ $i ]['line'] !== $tokens[ $ptrs[1] ]['line'] ) {
if ( $tokens[ $i ]['code'] === \T_DOC_COMMENT_WHITESPACE
&& $tokens[ $i ]['content'] === $phpcsFile->eolChar
Expand Down Expand Up @@ -269,7 +312,7 @@ public function process( File $phpcsFile, $stackPtr ) {
*
* @return bool Whether an error has been thrown or not.
*/
protected function fixSimpleError( File $phpcsFile, $stackPtr, $expected, $errorCode ) {
private function fixSimpleError( File $phpcsFile, $stackPtr, $expected, $errorCode ) {
$tokens = $phpcsFile->getTokens();
$annotation = $tokens[ $stackPtr ]['content'];

Expand Down Expand Up @@ -303,7 +346,7 @@ protected function fixSimpleError( File $phpcsFile, $stackPtr, $expected, $error
*
* @return bool Whether to skip the rest of the annotation examination or not.
*/
protected function fixAnnotationToSplit( File $phpcsFile, $stackPtr, $errorCode, $separator ) {
private function fixAnnotationToSplit( File $phpcsFile, $stackPtr, $errorCode, $separator ) {
$fix = $phpcsFile->addFixableError(
'Each @covers annotation should reference only one covered structure',
$stackPtr,
Expand Down
67 changes: 49 additions & 18 deletions Yoast/Tests/Commenting/CoversTagUnitTest.inc
View file
Original file line number Diff line number Diff line change
@@ -1,30 +1,14 @@
<?php

class ClassNameTest {

/**
* Docblock.
*
* Correct:
* @covers ::global_function
* @covers Name\Space\function_name
* @covers \Name\Space\function_name
* @covers Class_Name
* @covers Class_Name<extended>
* @covers Class_Name::<public>
* @covers Class_Name::<protected>
* @covers Class_Name::<private>
* @covers Class_Name::<!public>
* @covers Class_Name::<!protected>
* @covers Class_Name::<!private>
* @covers Name\Space\Class_Name
* @covers \Name\Space\Class_Name
* @covers Name\Space\Class_Name::<public>
* @covers Name\Space\Class_Name::<protected>
* @covers Name\Space\Class_Name::<private>
* @covers Name\Space\Class_Name::<!public>
* @covers Name\Space\Class_Name::<!protected>
* @covers Name\Space\Class_Name::<!private>
* @covers Class_Name::method_name
* @covers \Class_name::method_name
* @covers Name\Space\Class_Name::method_name
Expand All @@ -42,8 +26,24 @@ class ClassNameTest {
public function testCoversTag() {}

/**
* Docblock.
*
* @covers Class_Name<extended>
* @covers Class_Name::<public>
* @covers Class_Name::<protected>
* @covers Class_Name::<private>
* @covers Class_Name::<!public>
* @covers Class_Name::<!protected>
* @covers Class_Name::<!private>
* @covers \Name\Space\Class_Name<extended>
* @covers \Name\Space\Class_Name::<public>
* @covers Name\Space\Class_Name::<protected>
* @covers Name\Space\Class_Name::<private>
* @covers \Name\Space\Class_Name::<!public>
* @covers Name\Space\Class_Name::<!protected>
* @covers \Name\Space\Class_Name::<!private>
*/
public function testDeprecatedRemovedTagTypes() {}

/**
* @covers ::global_functionA && ::other_functionA
* @covers ::global_functionB & ::other_functionB
* @covers ::global_functionC|::other_functionC
Expand Down Expand Up @@ -164,4 +164,35 @@ class ClassNameTest {
* @covers \Name\Space\ClassName::MethodName
*/
public function testRecognizingNamesNotFollowingWPNamingConventions() {}

/**
* Docblock.
*/
public function testDocblockWithoutTags() {}

/**
* Docblock.
*
* @covers This is not a valid annotation.
* @covers \
* @covers <public>
*/
public function testCompletelyInvalidAnnotations() {}
}

/**
* The <*> formats can also be used in combination with a `@coversDefaultClass` tag.
*
* @coversDefaultClass Class_Name
*/
class BracketedPartialsTest {
/**
* @covers ::<public>
* @covers ::<protected>
* @covers ::<private>
* @covers ::<!public>
* @covers ::<!protected>
* @covers ::<!private>
*/
public function testPartialTagTypes() {}
}
Loading

0 comments on commit 9d00928

Please sign in to comment.