Page MenuHomePhabricator

ParenthesesAroundKeywordSniff leaves extraneous whitespace
Closed, ResolvedPublic

Description

Running only ParenthesesAroundKeywordSniff leaves behind extraneous whitespace when whitespace was present before the removed close-paren. 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

require( 'a.php' );
if ( include( 'b.php' ) ) {
    echo "b loaded.\n";
}
EOF
$ cat <<EOF > .phpcs.xml
<?xml version="1.0"?>
<ruleset>
    <rule ref="./vendor/mediawiki/mediawiki-codesniffer/MediaWiki/Sniffs/ExtraCharacters/ParenthesesAroundKeywordSniff.php" />
</ruleset>
EOF
$ vendor/bin/phpcbf test.php

Expected result

test.php is

<?php

require 'a.php';
if ( include 'b.php' ) {
    echo "b loaded.\n";
}

Actual result

test.php is

<?php

require 'a.php' ;
if ( include 'b.php'  ) {
    echo "b loaded.\n";
}

Notes

Fixing it, at least for common cases, seems pretty straightforward:

diff --git a/MediaWiki/Sniffs/ExtraCharacters/ParenthesesAroundKeywordSniff.php b/MediaWiki/Sniffs/ExtraCharacters/ParenthesesAroundKeywordSniff.php
index 9ff14f0..4b307b5 100644
--- a/MediaWiki/Sniffs/ExtraCharacters/ParenthesesAroundKeywordSniff.php
+++ b/MediaWiki/Sniffs/ExtraCharacters/ParenthesesAroundKeywordSniff.php
@@ -76,15 +76,20 @@ class ParenthesesAroundKeywordSniff implements Sniff {
                                                $phpcsFile->fixer->replaceToken( $stackPtr + 1, ' ' );
                                        }
                                        $closer = $nextToken['parenthesis_closer'];
-                                       $phpcsFile->fixer->replaceToken( $closer, '' );
                                } else {
                                        $phpcsFile->fixer->replaceToken( $stackPtr + 2, '' );
                                        $closer = $nextSecondToken['parenthesis_closer'];
-                                       $phpcsFile->fixer->replaceToken( $closer, '' );
                                        if ( $tokens[$stackPtr + 3]['code'] === T_WHITESPACE ) {
                                                $phpcsFile->fixer->replaceToken( $stackPtr + 3, '' );
                                        }
                                }
+                               $phpcsFile->fixer->replaceToken( $closer, '' );
+                               // Remove whitespace before the closing paren if appropriate.
+                               if ( $tokens[$closer - 1]['code'] === T_WHITESPACE &&
+                                       ( !isset( $tokens[$closer + 1] ) || in_array( $tokens[$closer + 1]['code'], [ T_WHITESPACE, T_SEMICOLON, T_COMMA ], true ) )
+                               ) {
+                                       $phpcsFile->fixer->replaceToken( $closer - 1, '' );
+                               }
                        }
                }
        }

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

Event Timeline

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

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

Patch sent based on changes in task description

Change 650595 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/tools/codesniffer@master] Rewrite ParenthesesAroundKeywordSniff to fix whitespace handling

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

Change 650595 merged by jenkins-bot:
[mediawiki/tools/codesniffer@master] Rewrite ParenthesesAroundKeywordSniff to fix whitespace handling

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

Change 647286 abandoned by DannyS712:
[mediawiki/tools/codesniffer@master] ParenthesesAroundKeywordSniff whitespace handling cleanup

Reason:
alternative implementation was merged

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

Umherirrender claimed this task.

Change 647286 restored by DannyS712:
[mediawiki/tools/codesniffer@master] ParenthesesAroundKeywordSniff whitespace handling cleanup

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

Change 647286 abandoned by DannyS712:
[mediawiki/tools/codesniffer@master] ParenthesesAroundKeywordSniff whitespace handling cleanup

Reason:

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