Merge pull request #331 from Yoast/JRF/yoastcs-testshavecoverstag-var…

…ious-improvements

Commenting/TestsHaveCoversTag: support PHP 8.1 + 8.2, bug fix and various other improvements

Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php

@@ -5,8 +5,10 @@
 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;
 
 /**
  * Verifies that all test functions have at least one @covers tag.
@@ -45,9 +47,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 );
 	}
 
 	/**
@@ -59,13 +60,14 @@ 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 );
 
-		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'] ) ) {
@@ -76,12 +78,9 @@ protected function process_class( File $phpcsFile, $stackPtr ) {
 			return;
 		}
 
-		// @todo: Once PHPCSUtils is out, replace with call to new findCommentAboveOOStructure() method.
-		$ignore = [
-			\T_WHITESPACE => \T_WHITESPACE,
-			\T_ABSTRACT   => \T_ABSTRACT,
-			\T_FINAL      => \T_FINAL,
-		];
+		// @todo: Once PHPCSUtils 1.2.0 (?) is out, replace with call to new findCommentAboveOOStructure() method.
+		$ignore                  = Collections::classModifierKeywords();
+		$ignore[ \T_WHITESPACE ] = \T_WHITESPACE;
 
 		$commentEnd = $stackPtr;
 		for ( $commentEnd = ( $stackPtr - 1 ); $commentEnd >= 0; $commentEnd-- ) {
@@ -111,10 +110,12 @@ protected 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;
 			}
 		}
 
@@ -134,10 +135,15 @@ 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.
+		if ( Scopes::isOOMethod( $phpcsFile, $stackPtr ) === false ) {
+			// This is a global function, not a method in a test class.
+			return;
+		}
+
+		// @todo: Once PHPCSUtils 1.2.0 (?) is out, replace with call to new findCommentAboveFunction() method.
 		$ignore                  = Tokens::$methodPrefixes;
 		$ignore[ \T_WHITESPACE ] = \T_WHITESPACE;
 
@@ -183,6 +189,11 @@ protected 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;

Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc → Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.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.
@@ -154,3 +154,44 @@ 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) {}
+}
+
+/**
+ * Global functions should be ignored as PHPUnit tests should always be in a class.
+ */
+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() {}
+}

Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.2.inc

@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error test.
+// This must be the only test in the file.
+class NotATestClass {

Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.3.inc

@@ -0,0 +1,13 @@
+<?php
+
+// Live coding/parse error test.
+// This must be the only test in the file.
+
+/**
+ * @covers Something
+ */
+class LiveCodingTest {
+	/**
+	 * @covers SomethingElse
+	 */
+	public function testThis() {}

Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.4.inc

@@ -0,0 +1,5 @@
+<?php
+
+// Live coding/parse error test.
+// This must be the only test in the file.
+final class

Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.5.inc

@@ -0,0 +1,11 @@
+<?php
+
+// Live coding/parse error test.
+// This must be the only test in the file.
+
+class ParseErrorTest {
+	/**
+	 * This function is missing its name. This parse error should be ignored.
+	 */
+	public function {}
+}

Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.php

@@ -16,17 +16,27 @@ final class TestsHaveCoversTagUnitTest extends AbstractSniffUnitTest {
 	/**
 	 * Returns the lines where errors should occur.
 	 *
+	 * @param string $testFile The name of the file being tested.
+	 *
 	 * @return array<int, int> 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,
+					173 => 1,
+				];
+
+			default:
+				return [];
+		}
 	}
 
 	/**