From 4c0b5fa82ad966f8ff9028ffbf79e5247d7aeee5 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 14 Oct 2023 07:12:08 +0200 Subject: [PATCH 1/9] Commenting/TestsHaveCoversTag: minor test improvements Add some more variation of class/method prefixes to the tests to better safeguard the handling of those. --- Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc index 63123a2b..d7c2ad5b 100644 --- a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc +++ b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc @@ -29,7 +29,7 @@ class ClassLevelCoversTest { /** * @coversNothing */ -class ClassLevelCoversNothingTest { +final class ClassLevelCoversNothingTest { /** * Docblock. @@ -85,7 +85,7 @@ class ClassNameTest { * * @test */ - public function annotatedTestMissingCoversTag() {} + public static function annotatedTestMissingCoversTag() {} /** * Docblock. @@ -101,14 +101,14 @@ class ClassNameTest { * @test * @covers ::globalFunction */ - public function annotatedTestHasCoversTag() {} + final public function annotatedTestHasCoversTag() {} } /** * @covers ClassName */ #[Some_Attribute] -class ClassLevelCoversWithAttributeTest { +final class ClassLevelCoversWithAttributeTest { /** * Docblock. From 835f7a124d36af5a86504a12a8b753b66a411748 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 14 Oct 2023 06:10:18 +0200 Subject: [PATCH 2/9] Commenting/TestsHaveCoversTag: rename test case file ... to allow for additional test case files to be added. --- ...t.inc => TestsHaveCoversTagUnitTest.1.inc} | 0 .../Commenting/TestsHaveCoversTagUnitTest.php | 27 ++++++++++++------- 2 files changed, 18 insertions(+), 9 deletions(-) rename Yoast/Tests/Commenting/{TestsHaveCoversTagUnitTest.inc => TestsHaveCoversTagUnitTest.1.inc} (100%) diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc similarity index 100% rename from Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc rename to Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.php b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.php index 0c728219..334f819e 100644 --- a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.php +++ b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.php @@ -16,17 +16,26 @@ final class TestsHaveCoversTagUnitTest extends AbstractSniffUnitTest { /** * Returns the lines where errors should occur. * + * @param string $testFile The name of the file being tested. + * * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList(): array { - return [ - 49 => 1, - 59 => 1, - 88 => 1, - 126 => 1, - 142 => 1, - 150 => 1, - ]; + public function getErrorList( string $testFile = '' ): array { + + switch ( $testFile ) { + case 'TestsHaveCoversTagUnitTest.1.inc': + return [ + 49 => 1, + 59 => 1, + 88 => 1, + 126 => 1, + 142 => 1, + 150 => 1, + ]; + + default: + return []; + } } /** From 11cfe5b7b8e4acdc18a719065e7279dc3f5669c9 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 14 Oct 2023 06:21:44 +0200 Subject: [PATCH 3/9] Commenting/TestsHaveCoversTag: add some extra tests ... for situations previously not (yet) covered by tests. --- .../TestsHaveCoversTagUnitTest.1.inc | 18 ++++++++++++++++++ .../TestsHaveCoversTagUnitTest.2.inc | 5 +++++ .../TestsHaveCoversTagUnitTest.3.inc | 13 +++++++++++++ .../Commenting/TestsHaveCoversTagUnitTest.php | 1 + 4 files changed, 37 insertions(+) create mode 100644 Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.2.inc create mode 100644 Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.3.inc diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc index d7c2ad5b..0ede80a7 100644 --- a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc +++ b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc @@ -154,3 +154,21 @@ abstract class ContainsTestMethodsTestCase { */ abstract function testToBeImplementedInChildClass(); } + +/** + * Docblock. + * + * @sometag + * @anothertag + */ +class DoesNotHaveOurTargetTagsTest { + /** + * Docblock with tags, but not any of the tags we're looking for. + * + * @param mixed $input + * @param string $expected + * + * @return void + */ + final public function testSomething($input, $expected) {} +} diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.2.inc b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.2.inc new file mode 100644 index 00000000..c5d0b545 --- /dev/null +++ b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.2.inc @@ -0,0 +1,5 @@ + 1, 142 => 1, 150 => 1, + 173 => 1, ]; default: From 3431d8e1df26ec113620751215f55916f6328830 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 31 Oct 2023 03:42:08 +0100 Subject: [PATCH 4/9] Commenting/TestsHaveCoversTag: 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/TestsHaveCoversTagSniff.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php index 4c3e84a3..ea1185f9 100644 --- a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php @@ -59,7 +59,7 @@ public function process( File $phpcsFile, $stackPtr ) { * @return void|int If covers annotations were found (or this is not a test class), * will return the stack pointer to the end of the class. */ - protected function process_class( File $phpcsFile, $stackPtr ) { + private function process_class( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); $name = ObjectDeclarations::getName( $phpcsFile, $stackPtr ); @@ -134,7 +134,7 @@ protected function process_class( File $phpcsFile, $stackPtr ) { * * @return void */ - protected function process_function( File $phpcsFile, $stackPtr ) { + private function process_function( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); // @todo: Once PHPCSUtils is out, replace with call to new findCommentAboveFunction() method. From cfad3a195e995337bb0039779a65baece68567dc Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 14 Oct 2023 06:38:06 +0200 Subject: [PATCH 5/9] Commenting/TestsHaveCoversTag: bug fix - prevent PHP 8.1 deprecation notice during live coding [1] When scanning code during live coding in an IDE, the `ObjectDeclarations::getName()` method could return `null`. This fix prevents a PHP 8.1 "substr(): Passing null to parameter #1 ($string) of type string is deprecated" deprecation notice in that situation. Includes test. --- Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php | 7 ++++--- Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.4.inc | 5 +++++ 2 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.4.inc diff --git a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php index ea1185f9..a7bd2269 100644 --- a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php @@ -63,9 +63,10 @@ private function process_class( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); $name = ObjectDeclarations::getName( $phpcsFile, $stackPtr ); - if ( \substr( $name, -4 ) !== 'Test' - && \substr( $name, -8 ) !== 'TestCase' - && \substr( $name, 0, 4 ) !== 'Test' + if ( empty( $name ) + || ( \substr( $name, -4 ) !== 'Test' + && \substr( $name, -8 ) !== 'TestCase' + && \substr( $name, 0, 4 ) !== 'Test' ) ) { // Not a test class. if ( isset( $tokens[ $stackPtr ]['scope_closer'] ) ) { diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.4.inc b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.4.inc new file mode 100644 index 00000000..31b968d8 --- /dev/null +++ b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.4.inc @@ -0,0 +1,5 @@ + Date: Sat, 14 Oct 2023 06:54:19 +0200 Subject: [PATCH 6/9] Commenting/TestsHaveCoversTag: bug fix - prevent PHP 8.1 deprecation notice during live coding [2] When scanning code during live coding/while scanning code containing a parse error, the `FunctionDeclarations::getName()` method could return `null`. This fix prevents a PHP 8.1 "stripos(): Passing null to parameter #1 ($string) of type string is deprecated" deprecation notice in that situation. Includes test. --- Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php | 5 +++++ .../Tests/Commenting/TestsHaveCoversTagUnitTest.5.inc | 11 +++++++++++ 2 files changed, 16 insertions(+) create mode 100644 Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.5.inc diff --git a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php index a7bd2269..60c3a376 100644 --- a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php @@ -184,6 +184,11 @@ private function process_function( File $phpcsFile, $stackPtr ) { } $name = FunctionDeclarations::getName( $phpcsFile, $stackPtr ); + if ( empty( $name ) ) { + // Parse error. Ignore this method as it will never be run as a test. + return; + } + if ( \stripos( $name, 'test' ) !== 0 && $foundTest === false ) { // Not a test method. return; diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.5.inc b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.5.inc new file mode 100644 index 00000000..44fd4b7d --- /dev/null +++ b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.5.inc @@ -0,0 +1,11 @@ + Date: Sat, 14 Oct 2023 06:46:42 +0200 Subject: [PATCH 7/9] Commenting/TestsHaveCoversTag: bug fix - prevent false positive on global function Test functions in the context of PHPUnit tests **must** be OO methods in a child class of the PHPUnit native `TestCase` class. Global functions will never be run as tests, so can be disregarded for the purposes of this sniff without further examination. Includes test. --- Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php | 6 ++++++ Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php index 60c3a376..2822048b 100644 --- a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php @@ -7,6 +7,7 @@ use PHP_CodeSniffer\Util\Tokens; use PHPCSUtils\Utils\FunctionDeclarations; use PHPCSUtils\Utils\ObjectDeclarations; +use PHPCSUtils\Utils\Scopes; /** * Verifies that all test functions have at least one @covers tag. @@ -138,6 +139,11 @@ private function process_class( File $phpcsFile, $stackPtr ) { private function process_function( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); + if ( Scopes::isOOMethod( $phpcsFile, $stackPtr ) === false ) { + // This is a global function, not a method in a test class. + return; + } + // @todo: Once PHPCSUtils is out, replace with call to new findCommentAboveFunction() method. $ignore = Tokens::$methodPrefixes; $ignore[ \T_WHITESPACE ] = \T_WHITESPACE; diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc index 0ede80a7..590bd3dc 100644 --- a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc +++ b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc @@ -172,3 +172,9 @@ class DoesNotHaveOurTargetTagsTest { */ final public function testSomething($input, $expected) {} } + +/** + * Global functions should be ignored as PHPUnit tests should always be in a class. + */ +function testMe() {} + From d59dcf675695c8e2d63e1c16ef052705fa34095b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 14 Oct 2023 07:09:54 +0200 Subject: [PATCH 8/9] Commenting/TestsHaveCoversTag: minor tweaks * Remove an unnecessary condition. * Remove a `return` which will never receive a value to return. * Bow out earlier if something useful was found. No need to continue examining. * Minor doc updates for future reference. --- Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php index 2822048b..2858bb0d 100644 --- a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php @@ -46,9 +46,8 @@ public function process( File $phpcsFile, $stackPtr ) { return $this->process_class( $phpcsFile, $stackPtr ); } - if ( $tokens[ $stackPtr ]['code'] === \T_FUNCTION ) { - return $this->process_function( $phpcsFile, $stackPtr ); - } + // This must be a T_FUNCTION token. + $this->process_function( $phpcsFile, $stackPtr ); } /** @@ -78,7 +77,7 @@ private function process_class( File $phpcsFile, $stackPtr ) { return; } - // @todo: Once PHPCSUtils is out, replace with call to new findCommentAboveOOStructure() method. + // @todo: Once PHPCSUtils 1.2.0 (?) is out, replace with call to new findCommentAboveOOStructure() method. $ignore = [ \T_WHITESPACE => \T_WHITESPACE, \T_ABSTRACT => \T_ABSTRACT, @@ -113,10 +112,12 @@ private function process_class( File $phpcsFile, $stackPtr ) { foreach ( $tokens[ $commentStart ]['comment_tags'] as $tag ) { if ( $tokens[ $tag ]['content'] === '@covers' ) { $foundCovers = true; + break; } if ( $tokens[ $tag ]['content'] === '@coversNothing' ) { $foundCoversNothing = true; + break; } } @@ -144,7 +145,7 @@ private function process_function( File $phpcsFile, $stackPtr ) { return; } - // @todo: Once PHPCSUtils is out, replace with call to new findCommentAboveFunction() method. + // @todo: Once PHPCSUtils 1.2.0 (?) is out, replace with call to new findCommentAboveFunction() method. $ignore = Tokens::$methodPrefixes; $ignore[ \T_WHITESPACE ] = \T_WHITESPACE; From d36d2ae77e27223be60cb6fc60579554cb655e96 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 14 Oct 2023 07:17:20 +0200 Subject: [PATCH 9/9] Commenting/TestsHaveCoversTag: handle PHP 8.2 readonly classes While probably rare, the sniff should still find the docblock for a test class marked as `readonly` without breaking on the `readonly` keyword. Fixed now. Includes tests. --- .../Commenting/TestsHaveCoversTagSniff.php | 8 +++----- .../Commenting/TestsHaveCoversTagUnitTest.1.inc | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php index 2858bb0d..8355a5d7 100644 --- a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php @@ -5,6 +5,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Tokens\Collections; use PHPCSUtils\Utils\FunctionDeclarations; use PHPCSUtils\Utils\ObjectDeclarations; use PHPCSUtils\Utils\Scopes; @@ -78,11 +79,8 @@ private function process_class( File $phpcsFile, $stackPtr ) { } // @todo: Once PHPCSUtils 1.2.0 (?) is out, replace with call to new findCommentAboveOOStructure() method. - $ignore = [ - \T_WHITESPACE => \T_WHITESPACE, - \T_ABSTRACT => \T_ABSTRACT, - \T_FINAL => \T_FINAL, - ]; + $ignore = Collections::classModifierKeywords(); + $ignore[ \T_WHITESPACE ] = \T_WHITESPACE; $commentEnd = $stackPtr; for ( $commentEnd = ( $stackPtr - 1 ); $commentEnd >= 0; $commentEnd-- ) { diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc index 590bd3dc..0947d7d0 100644 --- a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc +++ b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc @@ -178,3 +178,20 @@ class DoesNotHaveOurTargetTagsTest { */ function testMe() {} +/** + * Docblocks above PHP 8.2 readonly test classes should be recognized correctly. + * + * @covers \Some\Class::methodName + */ +readonly class ReadonlyTest { + public function testMe() {} +} + +/** + * Docblocks above PHP 8.2 readonly test classes should be recognized correctly. + * + * @covers \Some\Class::methodName + */ +final readonly class FinalReadonlyTest { + public function testMe() {} +}