Page MenuHomePhabricator

PHPUnitAssertEqualsSniff leaves extraneous whitespace
Closed, ResolvedPublic

Description

Running only PHPUnitAssertEqualsSniff leaves behind extraneous whitespace when changing assertEquals to assertFalse, assertTrue, or assertNull. You don't see this in normal usage because the extra whitespace is taken care of by other sniffs.

Test case

$ mkdir /tmp/test && cd /tmp/test
$ composer require mediawiki/mediawiki-codesniffer
$ cat <<'EOF' > test.php
<?php

class Test {
    function test() {
        $this->assertEquals( true, $x );
        $this->assertEquals( false, $x );
        $this->assertEquals( null, $x );
    }
}
EOF
$ cat <<EOF > .phpcs.xml
<?xml version="1.0"?>
<ruleset>
    <rule ref="./vendor/mediawiki/mediawiki-codesniffer/MediaWiki/Sniffs/Usage/PHPUnitAssertEqualsSniff.php" />
</ruleset>
EOF
$ vendor/bin/phpcbf test.php

Expected result

test.php is

<?php

class Test {
    function test() {
        $this->assertTrue( $x );
        $this->assertFalse( $x );
        $this->assertNull( $x );
    }
}

Actual result

test.php is

<?php

class Test {
    function test() {
        $this->assertTrue(  $x );
        $this->assertFalse(  $x );
        $this->assertNull(  $x );
    }
}

Notes

Fixing it seems pretty straightforward:

diff --git a/MediaWiki/Sniffs/Usage/PHPUnitAssertEqualsSniff.php b/MediaWiki/Sniffs/Usage/PHPUnitAssertEqualsSniff.php
index f733db6..4892ac8 100644
--- a/MediaWiki/Sniffs/Usage/PHPUnitAssertEqualsSniff.php
+++ b/MediaWiki/Sniffs/Usage/PHPUnitAssertEqualsSniff.php
@@ -116,6 +116,9 @@ class PHPUnitAssertEqualsSniff implements Sniff {
                        $fixer->replaceToken( $stackPtr, $fix );
                        $fixer->replaceToken( $expected, '' );
                        $fixer->replaceToken( $expected + 1, '' );
+                       if ( $tokens[$expected + 2]['code'] === T_WHITESPACE ) {
+                               $fixer->replaceToken( $expected + 2, '' );
+                       }
                } elseif ( $fix ) {
                        $fixer->replaceToken( $stackPtr, 'assertSame' );
                }

I'd submit that to Gerrit myself, but my Gerrit account seems to be non-functional.

Event Timeline

Change 647287 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/tools/codesniffer@master] AssertEqualsSniff whitespace handling cleanup

https://gerrit.wikimedia.org/r/647287

Patch sent based on changes in task description

Change 647287 merged by jenkins-bot:
[mediawiki/tools/codesniffer@master] AssertEqualsSniff whitespace handling cleanup

https://gerrit.wikimedia.org/r/647287

DannyS712 claimed this task.