Enforce absolute class path for coverage annotations

We renamed many classes to be namespaced, but the @Covers and @coversDefaultClass annotations weren't updated properly. This relies on the use statement, so it would not complain about class alias alone. Also, disabled the noisy multiple-class warning for the test. Bug: T183218 Change-Id: I7e6dce5bf11ab454e9677bacb527ef601c394f14
wikimedia · Jan 15, 2024 · 8c9cb70 · 8c9cb70
1 parent 4861986
commit 8c9cb70
Show file tree

Hide file tree

Showing 5 changed files with 163 additions and 31 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -20,6 +20,7 @@
 * `FunctionAnnotationsSniff`: Add `@phan-type` as an allowed annotation (Umherirrender)
 * `FunctionAnnotationsSniff`: Add `@phan-side-effect-free` as an allowed annotation (Bartosz Dziewoński)
 * `LowerCamelFunctionsNameSniff`: Ignore hook methods (DannyS712)
+* `PhpunitAnnotationsSniff`: Enforce absolute class path for coverage annotations (Func)
 
 ### Removed sniffs ###
 * `OneSpaceInlineArraySniff`: Superseded by `Universal.WhiteSpace.CommaSpacing`

diff --git a/MediaWiki/Sniffs/Commenting/PhpunitAnnotationsSniff.php b/MediaWiki/Sniffs/Commenting/PhpunitAnnotationsSniff.php
@@ -23,6 +23,7 @@
 use MediaWiki\Sniffs\PHPUnit\PHPUnitTestTrait;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Tokens;
 
 class PhpunitAnnotationsSniff implements Sniff {
 	use PHPUnitTestTrait;
@@ -140,13 +141,20 @@ class PhpunitAnnotationsSniff implements Sniff {
 		],
 	];
 
+	private const ABSOLUTE_CLASS_ANNOTATIONS = [
+		'@covers' => 'AbsoluteCovers',
+		'@coversDefaultClass' => 'AbsoluteCoversDefaultClass',
+	];
+
+	private array $useClasses = [];
+
 	/**
 	 * Returns an array of tokens this test wants to listen for.
 	 *
 	 * @return array
 	 */
 	public function register(): array {
-		return [ T_DOC_COMMENT_OPEN_TAG ];
+		return [ T_OPEN_TAG, T_USE, T_DOC_COMMENT_OPEN_TAG ];
 	}
 
 	/**
@@ -157,6 +165,48 @@ public function register(): array {
 	 * @return void
 	 */
 	public function process( File $phpcsFile, $stackPtr ) {
+		$tokens = $phpcsFile->getTokens();
+		if ( $tokens[$stackPtr]['code'] === T_OPEN_TAG ) {
+			// Run into a new file.
+			$this->useClasses = [];
+		} elseif ( $tokens[$stackPtr]['code'] === T_USE ) {
+			$this->processUse( $phpcsFile, $stackPtr );
+		} else {
+			$this->processDocBlock( $phpcsFile, $stackPtr );
+		}
+	}
+
+	/**
+	 * @param File $phpcsFile
+	 * @param int $stackPtr Position of T_USE
+	 * @return void
+	 */
+	private function processUse( File $phpcsFile, int $stackPtr ) {
+		$tokens = $phpcsFile->getTokens();
+		$next = $phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true );
+
+		$className = '';
+		while ( in_array( $tokens[$next]['code'], [ T_STRING, T_NS_SEPARATOR ] ) ) {
+			$className .= $tokens[$next++]['content'];
+		}
+		$className = ltrim( $className, '\\' );
+
+		if (
+			// Exclude: not `use` for classes, group of use statements or use of `as`;
+			$tokens[$next]['code'] === T_SEMICOLON &&
+			// Exclude: classes without namespaces.
+			str_contains( $className, '\\' )
+		) {
+			$this->useClasses[$tokens[$next - 1]['content']] = '\\' . $className;
+		}
+	}
+
+	/**
+	 * @param File $phpcsFile
+	 * @param int $stackPtr Position of T_DOC_COMMENT_OPEN_TAG
+	 * @return void
+	 */
+	public function processDocBlock( File $phpcsFile, int $stackPtr ) {
 		$tokens = $phpcsFile->getTokens();
 		$end = $tokens[$stackPtr]['comment_closer'];
 		foreach ( $tokens[$stackPtr]['comment_tags'] as $tag ) {
@@ -241,6 +291,20 @@ private function processDocTag( File $phpcsFile, array $tokens, int $tag, int $e
 					'The phpunit annotation %s must be followed by text.',
 					$tag, $this->createSniffCode( 'Empty', $tagText ), [ $tagText ]
 				);
+			} elseif ( isset( self::ABSOLUTE_CLASS_ANNOTATIONS[$tagText] ) ) {
+				$coveredClass = explode( '::', $tokens[$next]['content'] )[0];
+				if ( isset( $this->useClasses[$coveredClass] ) ) {
+					$fix = $phpcsFile->addFixableWarning(
+						'Use absolute class name (%s) for %s annotation instead',
+						$next, self::ABSOLUTE_CLASS_ANNOTATIONS[$tagText],
+						[ $this->useClasses[$coveredClass], $tagText ]
+					);
+					if ( $fix ) {
+						$replace = $this->useClasses[$coveredClass] .
+							substr( $tokens[$next]['content'], strlen( $coveredClass ) );
+						$phpcsFile->fixer->replaceToken( $next, $replace );
+					}
+				}
 			}
 		}
 

diff --git a/MediaWiki/Tests/files/Commenting/phpunit_annotations.php b/MediaWiki/Tests/files/Commenting/phpunit_annotations.php
@@ -1,5 +1,7 @@
 <?php
 
+// phpcs:disable Generic.Files.OneObjectStructurePerFile.MultipleFound
+
 /**
  * Not a Test file
  * @coversDefaultClass Test
@@ -118,3 +120,34 @@ public function testDoc() {
 	}
 
 }
+
+use \Path\To\AnotherClass;
+use Path\To\OtherClass;
+
+/**
+ * @covers OtherClass
+ */
+class RelativeCoversTest {
+
+	/**
+	 * @covers OtherClass::foo
+	 */
+	public function testFoo() {
+		$instance = new OtherClass();
+		$this->assertTrue( $instance->foo() );
+	}
+}
+
+/**
+ * @coversDefaultClass AnotherClass
+ */
+class RelativeCoversDefaultClassTest {
+
+	/**
+	 * @covers ::foo
+	 */
+	public function testFoo() {
+		$instance = new AnotherClass();
+		$this->assertTrue( $instance->foo() );
+	}
+}
diff --git a/MediaWiki/Tests/files/Commenting/phpunit_annotations.php.expect b/MediaWiki/Tests/files/Commenting/phpunit_annotations.php.expect
@@ -1,51 +1,52 @@
-   5 | WARNING | [ ] The phpunit annotation @coversDefaultClass should only be used inside test
+   7 | WARNING | [ ] The phpunit annotation @coversDefaultClass should only be used inside test
      |         |     classes. (MediaWiki.Commenting.PhpunitAnnotations.NotTestClass)
-  10 | WARNING | [ ] The phpunit annotation @cover should only be used inside test classes.
+  12 | WARNING | [ ] The phpunit annotation @cover should only be used inside test classes.
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.NotTestClass)
-  16 | WARNING | [ ] The phpunit annotation @backupGlobals should not be used.
+  18 | WARNING | [ ] The phpunit annotation @backupGlobals should not be used.
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.ForbiddenBackupGlobals)
-  22 | WARNING | [ ] The phpunit annotation @coversNothing should only be used inside test
+  24 | WARNING | [ ] The phpunit annotation @coversNothing should only be used inside test
      |         |     classes. (MediaWiki.Commenting.PhpunitAnnotations.NotTestClass)
-  28 | WARNING | [x] Use @coversDefaultClass annotation instead of @coverDefaultClass
+  30 | WARNING | [x] Use @coversDefaultClass annotation instead of @coverDefaultClass
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.SingularCoverDefaultClass)
-  29 | WARNING | [x] Use @group small annotation instead of @small
+  31 | WARNING | [x] Use @group small annotation instead of @small
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.GroupAliasSmall)
-  31 | ERROR   | [ ] Only one object structure is allowed in a file
-     |         |     (Generic.Files.OneObjectStructurePerFile.MultipleFound)
-  34 | WARNING | [x] Use @covers annotation instead of @cover
+  36 | WARNING | [x] Use @covers annotation instead of @cover
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.SingularCover)
-  35 | WARNING | [ ] The phpunit annotation @covers must be followed by text.
+  37 | WARNING | [ ] The phpunit annotation @covers must be followed by text.
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.EmptyCovers)
-  41 | WARNING | [x] Use @coversNothing annotation instead of @coverNothing
+  43 | WARNING | [x] Use @coversNothing annotation instead of @coverNothing
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.SingularCoverNothing)
-  43 | WARNING | [ ] The testNothing test method has no @covers tags
+  45 | WARNING | [ ] The testNothing test method has no @covers tags
      |         |     (MediaWiki.Commenting.MissingCovers.MissingCovers)
-  47 | WARNING | [ ] The phpunit annotation @after should only be used for tearDown functions
+  49 | WARNING | [ ] The phpunit annotation @after should only be used for tearDown functions
      |         |     (*TearDown). (MediaWiki.Commenting.PhpunitAnnotations.NotTearDownFunction)
-  53 | WARNING | [ ] The phpunit annotation @dataProvider should only be used for test
+  55 | WARNING | [ ] The phpunit annotation @dataProvider should only be used for test
      |         |     functions. (MediaWiki.Commenting.PhpunitAnnotations.NotTestFunction)
-  71 | WARNING | [ ] Do not use @expectedException, use $this->expectException().
+  73 | WARNING | [ ] Do not use @expectedException, use $this->expectException().
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.ForbiddenExpectedException)
-  72 | WARNING | [ ] Do not use @expectedExceptionMessage, use $this->expectExceptionMessage().
+  74 | WARNING | [ ] Do not use @expectedExceptionMessage, use $this->expectExceptionMessage().
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.ForbiddenExpectedExceptionMessage)
-  78 | ERROR   | [ ] Only one object structure is allowed in a file
-     |         |     (Generic.Files.OneObjectStructurePerFile.MultipleFound)
-  88 | WARNING | [ ] The phpunit annotation @group should only be used in class level comments.
+  90 | WARNING | [ ] The phpunit annotation @group should only be used in class level comments.
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.NotClass)
-  93 | WARNING | [ ] The phpunit annotation @group should only be used in class level comments.
+  95 | WARNING | [ ] The phpunit annotation @group should only be used in class level comments.
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.NotClass)
-  95 | ERROR   | [ ] Only one object structure is allowed in a file
-     |         |     (Generic.Files.OneObjectStructurePerFile.MultipleFound)
- 100 | WARNING | [ ] The phpunit annotation @dataProvider should only be used inside classes or
+ 102 | WARNING | [ ] The phpunit annotation @dataProvider should only be used inside classes or
      |         |     traits. (MediaWiki.Commenting.PhpunitAnnotations.NotInClassTrait)
- 107 | WARNING | [ ] The phpunit annotation @coverDefaultClass should only be used in class level
+ 109 | WARNING | [ ] The phpunit annotation @coverDefaultClass should only be used in class level
      |         |     comments. (MediaWiki.Commenting.PhpunitAnnotations.NotClass)
- 110 | ERROR   | [ ] Only one object structure is allowed in a file
-     |         |     (Generic.Files.OneObjectStructurePerFile.MultipleFound)
- 114 | WARNING | [ ] The phpunit annotation @group should only be used for test functions.
+ 116 | WARNING | [ ] The phpunit annotation @group should only be used for test functions.
      |         |     (MediaWiki.Commenting.PhpunitAnnotations.NotTestFunction)
- 115 | ERROR   | [ ] There must be no blank lines after the function comment
+ 117 | ERROR   | [ ] There must be no blank lines after the function comment
      |         |     (MediaWiki.Commenting.FunctionComment.SpacingAfter)
- 117 | WARNING | [ ] The testDoc test method has no @covers tags
+ 119 | WARNING | [ ] The testDoc test method has no @covers tags
      |         |     (MediaWiki.Commenting.MissingCovers.MissingCovers)
-PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
+ 124 | ERROR   | [x] Import statements must not begin with a leading backslash
+     |         |     (PSR12.Files.ImportStatement.LeadingSlash)
+ 128 | WARNING | [x] Use absolute class name (\Path\To\OtherClass) for @covers annotation
+     |         |     instead (MediaWiki.Commenting.PhpunitAnnotations.AbsoluteCovers)
+ 133 | WARNING | [x] Use absolute class name (\Path\To\OtherClass) for @covers annotation
+     |         |     instead (MediaWiki.Commenting.PhpunitAnnotations.AbsoluteCovers)
+ 142 | WARNING | [x] Use absolute class name (\Path\To\AnotherClass) for @coversDefaultClass
+     |         |     annotation instead
+     |         |     (MediaWiki.Commenting.PhpunitAnnotations.AbsoluteCoversDefaultClass)
+PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
diff --git a/MediaWiki/Tests/files/Commenting/phpunit_annotations.php.fixed b/MediaWiki/Tests/files/Commenting/phpunit_annotations.php.fixed
@@ -1,5 +1,7 @@
 <?php
 
+// phpcs:disable Generic.Files.OneObjectStructurePerFile.MultipleFound
+
 /**
  * Not a Test file
  * @coversDefaultClass Test
@@ -118,3 +120,34 @@ class Examples2Test {
 	}
 
 }
+
+use Path\To\AnotherClass;
+use Path\To\OtherClass;
+
+/**
+ * @covers \Path\To\OtherClass
+ */
+class RelativeCoversTest {
+
+	/**
+	 * @covers \Path\To\OtherClass::foo
+	 */
+	public function testFoo() {
+		$instance = new OtherClass();
+		$this->assertTrue( $instance->foo() );
+	}
+}
+
+/**
+ * @coversDefaultClass \Path\To\AnotherClass
+ */
+class RelativeCoversDefaultClassTest {
+
+	/**
+	 * @covers ::foo
+	 */
+	public function testFoo() {
+		$instance = new AnotherClass();
+		$this->assertTrue( $instance->foo() );
+	}
+}