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

Commenting/CoversTag: flag deprecated syntaxes, bug fix and various other improvements #330

Merged
merged 6 commits into from
Nov 4, 2023
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
48 changes: 42 additions & 6 deletions Yoast/Docs/Commenting/CoversTagStandard.xml
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
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
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