From b36a0bca6ebfd3d59294b8a090e6eab790c1670b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 13 Oct 2023 22:09:32 +0200 Subject: [PATCH 1/6] Commenting/CoversTag: add some extra tests ... for situations previously not (yet) covered by tests. --- Yoast/Tests/Commenting/CoversTagUnitTest.inc | 14 ++++++++++++++ Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed | 14 ++++++++++++++ Yoast/Tests/Commenting/CoversTagUnitTest.php | 3 +++ 3 files changed, 31 insertions(+) diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.inc b/Yoast/Tests/Commenting/CoversTagUnitTest.inc index 326e26b3..990e2abc 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.inc +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.inc @@ -164,4 +164,18 @@ 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 function testCompletelyInvalidAnnotations() {} } diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed b/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed index 53a0b1a0..a147d4a5 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed @@ -163,4 +163,18 @@ 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 function testCompletelyInvalidAnnotations() {} } diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.php b/Yoast/Tests/Commenting/CoversTagUnitTest.php index dc13a899..415cea7d 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.php +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.php @@ -46,6 +46,9 @@ public function getErrorList(): array { 140 => 1, 150 => 1, 151 => 1, + 176 => 1, + 177 => 1, + 178 => 1, ]; } From e7b91ba9216839d81b349cf9a22084fabc4a86ea Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 18 Sep 2023 05:19:59 +0200 Subject: [PATCH 2/6] Commenting/CoversTag: minor tweaks * Ensure (make it obvious) that `$data` is always available. * Use the correct data type for the stack pointers retrieved via the `explode()`. * Defense in depth tweak. * Improve the readability of the code a little. --- Yoast/Sniffs/Commenting/CoversTagSniff.php | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/Yoast/Sniffs/Commenting/CoversTagSniff.php b/Yoast/Sniffs/Commenting/CoversTagSniff.php index f51c94bf..94416020 100644 --- a/Yoast/Sniffs/Commenting/CoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/CoversTagSniff.php @@ -57,6 +57,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 = []; @@ -76,7 +79,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( @@ -146,6 +150,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'; @@ -157,6 +164,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'; @@ -177,7 +187,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 ); @@ -205,6 +214,9 @@ public function process( File $phpcsFile, $stackPtr ) { } } + /* + * Check for duplicate `@covers ...` tags. + */ $coversCount = \count( $coversTags ); if ( $coversCount > 1 ) { $unique = \array_unique( $coversTags ); @@ -218,6 +230,7 @@ public function process( File $phpcsFile, $stackPtr ) { } $first = null; + $data = []; foreach ( $coversTags as $ptrs => $annot ) { if ( $annotation !== $annot ) { continue; @@ -231,13 +244,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 From b0625077cc920c9f873ecbc175efc088291c2152 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 31 Oct 2023 03:41:47 +0100 Subject: [PATCH 3/6] Commenting/CoversTag: make non-interface methods `private` As the sniff class is now `final` (since PR 319), there is no need for any `protected` methods, so let's make these `private`. --- Yoast/Sniffs/Commenting/CoversTagSniff.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Yoast/Sniffs/Commenting/CoversTagSniff.php b/Yoast/Sniffs/Commenting/CoversTagSniff.php index 94416020..60760206 100644 --- a/Yoast/Sniffs/Commenting/CoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/CoversTagSniff.php @@ -281,7 +281,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']; @@ -315,7 +315,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, From fbd81329d458c95cbfc50000db6aa3fcb369b467 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 13 Oct 2023 19:21:37 +0200 Subject: [PATCH 4/6] Commenting/CoversTag: improve XML documentation Update external reference links and minor textual improvements. --- Yoast/Docs/Commenting/CoversTagStandard.xml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Yoast/Docs/Commenting/CoversTagStandard.xml b/Yoast/Docs/Commenting/CoversTagStandard.xml index 659041d5..47ad4dff 100644 --- a/Yoast/Docs/Commenting/CoversTagStandard.xml +++ b/Yoast/Docs/Commenting/CoversTagStandard.xml @@ -5,10 +5,11 @@ > @@ -24,7 +25,7 @@ class Some_Test extends TestCase { } ]]> - + From 6e668b775c15429902bf858e637480b1cbe9f5f5 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 13 Oct 2023 21:40:47 +0200 Subject: [PATCH 5/6] Commenting/CoversTag: bug fix - allow for <[!]visibility> format without class name This is perfectly valid when used in combination with a `@coversDefaultClass` class level tag, but would be flagged by the sniff (false positives). Fixed now. Includes tests. --- Yoast/Sniffs/Commenting/CoversTagSniff.php | 2 +- Yoast/Tests/Commenting/CoversTagUnitTest.inc | 17 +++++++++++++++++ .../Commenting/CoversTagUnitTest.inc.fixed | 17 +++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/Yoast/Sniffs/Commenting/CoversTagSniff.php b/Yoast/Sniffs/Commenting/CoversTagSniff.php index 60760206..246c6778 100644 --- a/Yoast/Sniffs/Commenting/CoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/CoversTagSniff.php @@ -24,7 +24,7 @@ final class CoversTagSniff implements Sniff { * * @var string */ - private const VALID_CONTENT_REGEX = '(?:\\\\?(?:(?[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\\\\)*(?P>OOName)(?:|::<[!]?(?:public|protected|private)>|::(?(?!public$|protected$|private$)(?P>OOName)))?|::(?P>functionName)|\\\\?(?:(?P>OOName)\\\\)+(?P>functionName))'; + private const VALID_CONTENT_REGEX = '(?:\\\\?(?:(?[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\\\\)*(?P>OOName)(?:|::<[!]?(?:public|protected|private)>|::(?(?!public$|protected$|private$)(?P>OOName)))?|::(?P>functionName)|::<[!]?(?:public|protected|private)>|\\\\?(?:(?P>OOName)\\\\)+(?P>functionName))'; /** * Base error message. diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.inc b/Yoast/Tests/Commenting/CoversTagUnitTest.inc index 990e2abc..63fa3c41 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.inc +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.inc @@ -179,3 +179,20 @@ class ClassNameTest { */ public function testCompletelyInvalidAnnotations() {} } + +/** + * The <*> formats can also be used in combination with a `@coversDefaultClass` tag. + * + * @coversDefaultClass Class_Name + */ +class BracketedPartialsTest { + /** + * @covers :: + * @covers :: + * @covers :: + * @covers :: + * @covers :: + * @covers :: + */ + public function testPartialTagTypes() {} +} diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed b/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed index a147d4a5..bef7bee0 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed @@ -178,3 +178,20 @@ class ClassNameTest { */ public function testCompletelyInvalidAnnotations() {} } + +/** + * The <*> formats can also be used in combination with a `@coversDefaultClass` tag. + * + * @coversDefaultClass Class_Name + */ +class BracketedPartialsTest { + /** + * @covers :: + * @covers :: + * @covers :: + * @covers :: + * @covers :: + * @covers :: + */ + public function testPartialTagTypes() {} +} From cd94f239c11b9a8afd4277d846f8d27442a1d699 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 13 Oct 2023 21:51:17 +0200 Subject: [PATCH 6/6] Commenting/CoversTag: flag deprecated @covers tag formats PHPUnit 9.0 (soft) deprecated the use of `ClassName<*>` type `@covers` annotations. Support for these type of annotation has been removed completely in PHPUnit 10.0.0. This commit adds a new _warning_ to the sniff, which will detect the use of these deprecated formats and flag them. The choice for _warning_ vs _error_ is deliberate. The Yoast repos, for now, do not use PHPUnit 10.x yet, so for now, a warning is sufficient. Once the repos would be upgraded to start using PHPUnit 10.x, this sniff should be updated and the `warning` changed to an `error`. Includes tests. Includes a new section in the XML docs about this new warning. Refs: * sebastianbergmann/phpunit 3630 (PHPUnit 9.0 deprecation). * sebastianbergmann/phpunit 3631 (PHPUnit 10.0 removal). --- Yoast/Docs/Commenting/CoversTagStandard.xml | 35 ++++++++++++++++++ Yoast/Sniffs/Commenting/CoversTagSniff.php | 33 ++++++++++++++++- Yoast/Tests/Commenting/CoversTagUnitTest.inc | 36 +++++++++---------- .../Commenting/CoversTagUnitTest.inc.fixed | 36 +++++++++---------- Yoast/Tests/Commenting/CoversTagUnitTest.php | 33 +++++++++++++---- 5 files changed, 130 insertions(+), 43 deletions(-) diff --git a/Yoast/Docs/Commenting/CoversTagStandard.xml b/Yoast/Docs/Commenting/CoversTagStandard.xml index 47ad4dff..b898bf4b 100644 --- a/Yoast/Docs/Commenting/CoversTagStandard.xml +++ b/Yoast/Docs/Commenting/CoversTagStandard.xml @@ -107,6 +107,41 @@ class Some_Test extends TestCase { * @covers ::globalFunction */ function test_something() {} +} + ]]> + + + + tags is deprecated since PHPUnit 9.0 and support has been removed in PHPUnit 10.0. + These type of annotations should not be used. + ]]> + + + + ::globalFunction + * @covers \ClassName::methodName + */ + function test_something() {} +} + ]]> + + + \ClassName:: + * @covers \ClassName:: + * @covers \ClassName + */ + function test_something() {} } ]]> diff --git a/Yoast/Sniffs/Commenting/CoversTagSniff.php b/Yoast/Sniffs/Commenting/CoversTagSniff.php index 246c6778..5f6e8bc3 100644 --- a/Yoast/Sniffs/Commenting/CoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/CoversTagSniff.php @@ -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 */ @@ -26,6 +27,26 @@ final class CoversTagSniff implements Sniff { */ private const VALID_CONTENT_REGEX = '(?:\\\\?(?:(?[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\\\\)*(?P>OOName)(?:|::<[!]?(?:public|protected|private)>|::(?(?!public$|protected$|private$)(?P>OOName)))?|::(?P>functionName)|::<[!]?(?:public|protected|private)>|\\\\?(?:(?P>OOName)\\\\)+(?P>functionName))'; + /** + * Regex to check for deprecated `@covers ClassName` 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 = '`^(?:\\\\)?(?:(?[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\\\\)*(?P>OOName)$`'; + + /** + * 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. * @@ -95,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; } diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.inc b/Yoast/Tests/Commenting/CoversTagUnitTest.inc index 63fa3c41..4ff3c918 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.inc +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.inc @@ -1,30 +1,14 @@ - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: * @covers Name\Space\Class_Name * @covers \Name\Space\Class_Name - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: * @covers Class_Name::method_name * @covers \Class_name::method_name * @covers Name\Space\Class_Name::method_name @@ -42,8 +26,24 @@ class ClassNameTest { public function testCoversTag() {} /** - * Docblock. - * + * @covers Class_Name + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers \Name\Space\Class_Name + * @covers \Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers \Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers \Name\Space\Class_Name:: + */ + public function testDeprecatedRemovedTagTypes() {} + + /** * @covers ::global_functionA && ::other_functionA * @covers ::global_functionB & ::other_functionB * @covers ::global_functionC|::other_functionC diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed b/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed index bef7bee0..cc03668d 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed @@ -1,30 +1,14 @@ - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: * @covers Name\Space\Class_Name * @covers \Name\Space\Class_Name - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: * @covers Class_Name::method_name * @covers \Class_name::method_name * @covers Name\Space\Class_Name::method_name @@ -42,8 +26,24 @@ class ClassNameTest { public function testCoversTag() {} /** - * Docblock. - * + * @covers Class_Name + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers \Name\Space\Class_Name + * @covers \Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers \Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers \Name\Space\Class_Name:: + */ + public function testDeprecatedRemovedTagTypes() {} + + /** * @covers ::global_functionA * @covers ::other_functionA * @covers ::global_functionB diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.php b/Yoast/Tests/Commenting/CoversTagUnitTest.php index 415cea7d..f85d2463 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.php +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.php @@ -20,11 +20,11 @@ final class CoversTagUnitTest extends AbstractSniffUnitTest { */ public function getErrorList(): array { return [ - 36 => 1, - 37 => 1, - 38 => 1, - 39 => 1, - 40 => 1, + 20 => 1, + 21 => 1, + 22 => 1, + 23 => 1, + 24 => 1, 47 => 1, 48 => 1, 49 => 1, @@ -58,6 +58,27 @@ public function getErrorList(): array { * @return array Key is the line number, value is the number of expected warnings. */ public function getWarningList(): array { - return []; + return [ + 29 => 1, + 30 => 1, + 31 => 1, + 32 => 1, + 33 => 1, + 34 => 1, + 35 => 1, + 36 => 1, + 37 => 1, + 38 => 1, + 39 => 1, + 40 => 1, + 41 => 1, + 42 => 1, + 190 => 1, + 191 => 1, + 192 => 1, + 193 => 1, + 194 => 1, + 195 => 1, + ]; } }