MockBoilerplateSniff: fix and re-enable equalTo() handling

This reverts commit 423d5c8.

Also fix the name of the issue to ConstraintEqualTo

Reason for revert: fix the logic instead

Bug: T340892
Change-Id: Ic21efcdc1d4c732cb8dd3e71a5dbf102563a1711

MediaWiki/Sniffs/PHPUnit/MockBoilerplateSniff.php

@@ -348,9 +348,10 @@ public function handleWith( File $phpcsFile, $withOpener ) {
 				$withCloser,
 				true
 			);
+			// if its $this->logicalNot() or similar we want to skip past the closing
+			// parenthesis, just make sure its a function call here
 			if ( !$methodPtr
 				|| $tokens[$methodPtr]['code'] !== T_STRING
-				|| $tokens[$methodPtr]['content'] !== 'equalTo'
 			) {
 				continue;
 			}
@@ -368,6 +369,10 @@ public function handleWith( File $phpcsFile, $withOpener ) {
 				continue;
 			}
 			$methodCloser = $tokens[$methodOpener]['parenthesis_closer'];
+			if ( $tokens[$methodPtr]['content'] !== 'equalTo' ) {
+				$thisPtr = $methodCloser;
+				continue;
+			}
 
 			// Check for equalTo() with a second parameter, which we cannot fix
 			$shouldSkip = false;
@@ -399,7 +404,7 @@ public function handleWith( File $phpcsFile, $withOpener ) {
 			$fix = $phpcsFile->addFixableWarning(
 				'Default constraint equalTo() is unneeded and should be removed',
 				$methodPtr,
-				'ConstaintEqualTo'
+				'ConstraintEqualTo'
 			);
 			if ( !$fix ) {
 				// Next $this->equalTo() cannot be until after the current one

MediaWiki/Tests/files/PHPUnit/mock_boilerplate.php

@@ -34,7 +34,7 @@ public function testWithEqualTo() {
 		$mock->method( 'multi' )->with(
 			$this->equalTo( 'first' ),
 			'second',
-			$this->logicalOr( $this->equalTo( 'third-a' ), 'third-b' )
+			$this->equalTo( 'third' )
 		)->willReturn( false );
 		// equalTo() with a parameter that includes a comma (function call)
 		$mock->method( 'calculated' )->with(
@@ -53,6 +53,26 @@ public function testNotChanged() {
 		$mock->method( 'baz' )->with(
 			$this->equalTo( 10, 0.1 )
 		)->willReturn( false );
+
+		// equalTo() within a logicalNot()
+		$mock->method( 'qux' )
+			->with(
+				$this->logicalNot(
+					$this->equalTo( 'quxParam' )
+				)
+			)
+			->willThrowException(
+				$this->createMock( InvalidArgumentException::class )
+			);
+		// equalTo() within a logicalOr()
+		$mock->method( 'other' )
+			->with(
+				$this->logicalOr(
+					$this->equalTo( 'other1' ),
+					$this->equalTo( 'other2' )
+				)
+			)
+			->willReturn( true );
 	}
 
 }

MediaWiki/Tests/files/PHPUnit/mock_boilerplate.php.expect

@@ -16,4 +16,14 @@
     |         |     (MediaWiki.PHPUnit.MockBoilerplate.ExactlyOnce)
  25 | WARNING | [x] Matcher ->exactly( 0 ) should be replaced with shortcut ->never()
     |         |     (MediaWiki.PHPUnit.MockBoilerplate.ExactlyNever)
-PHPCBF CAN FIX THE 9 MARKED SNIFF VIOLATIONS AUTOMATICALLY
+ 32 | WARNING | [x] Default constraint equalTo() is unneeded and should be removed
+    |         |     (MediaWiki.PHPUnit.MockBoilerplate.ConstraintEqualTo)
+ 33 | WARNING | [x] Default constraint equalTo() is unneeded and should be removed
+    |         |     (MediaWiki.PHPUnit.MockBoilerplate.ConstraintEqualTo)
+ 35 | WARNING | [x] Default constraint equalTo() is unneeded and should be removed
+    |         |     (MediaWiki.PHPUnit.MockBoilerplate.ConstraintEqualTo)
+ 37 | WARNING | [x] Default constraint equalTo() is unneeded and should be removed
+    |         |     (MediaWiki.PHPUnit.MockBoilerplate.ConstraintEqualTo)
+ 41 | WARNING | [x] Default constraint equalTo() is unneeded and should be removed
+    |         |     (MediaWiki.PHPUnit.MockBoilerplate.ConstraintEqualTo)
+PHPCBF CAN FIX THE 14 MARKED SNIFF VIOLATIONS AUTOMATICALLY

MediaWiki/Tests/files/PHPUnit/mock_boilerplate.php.fixed

@@ -29,16 +29,16 @@ class MockBoilerplateTest extends \PHPUnit\Framework\TestCase {
 	/** @coversNothing */
 	public function testWithEqualTo() {
 		$mock = $this->createMock( FooBar::class );
-		$mock->method( 'has' )->with( $this->equalTo( 'value' ) )->willReturn( true );
-		$mock->method( 'get' )->with( $this->equalTo( 'value' ) )->willReturn( 1 );
+		$mock->method( 'has' )->with( 'value' )->willReturn( true );
+		$mock->method( 'get' )->with( 'value' )->willReturn( 1 );
 		$mock->method( 'multi' )->with(
-			$this->equalTo( 'first' ),
+			'first',
 			'second',
-			$this->logicalOr( $this->equalTo( 'third-a' ), 'third-b' )
+			'third'
 		)->willReturn( false );
 		// equalTo() with a parameter that includes a comma (function call)
 		$mock->method( 'calculated' )->with(
-			$this->equalTo( addValues( 1, 2 ) )
+			addValues( 1, 2 )
 		)->willReturn( false );
 	}
 
@@ -53,6 +53,26 @@ class MockBoilerplateTest extends \PHPUnit\Framework\TestCase {
 		$mock->method( 'baz' )->with(
 			$this->equalTo( 10, 0.1 )
 		)->willReturn( false );
+
+		// equalTo() within a logicalNot()
+		$mock->method( 'qux' )
+			->with(
+				$this->logicalNot(
+					$this->equalTo( 'quxParam' )
+				)
+			)
+			->willThrowException(
+				$this->createMock( InvalidArgumentException::class )
+			);
+		// equalTo() within a logicalOr()
+		$mock->method( 'other' )
+			->with(
+				$this->logicalOr(
+					$this->equalTo( 'other1' ),
+					$this->equalTo( 'other2' )
+				)
+			)
+			->willReturn( true );
 	}
 
 }

MediaWiki/ruleset.xml

@@ -211,11 +211,6 @@
 		<severity>0</severity>
 	</rule>
 
-	<rule ref="MediaWiki.PHPUnit.MockBoilerplate.ConstaintEqualTo">
-		<!-- Set this to 5 or higher in your .phpcs.xml if you want to use this sniff -->
-		<severity>0</severity>
-	</rule>
-
 	<!-- Exclude common folders from version control or build tools -->
 	<exclude-pattern type="relative">^(\.git|coverage|node_modules|vendor)/*</exclude-pattern>
 </ruleset>