Page MenuHomePhabricator

Add sniff to detect spaces before comma in argument lists
Closed, ResolvedPublic

Description

For a function call the sniff Generic.Functions.FunctionCallArgumentSpacing.SpaceBeforeComma checks for spaces before comma. But there is no sniff which checks for spaces before comma in argument lists like in functions, constructs, closures implements (and maybe more situations - array creation etc).

Here is a test case with some places where too much whitespace

class MyClass implements I1  ,  I2 {
	public function __construct( $arg1  ,  $arg2 ) {
	}

	public function func( $arg1  ,  $arg2, $arg3, $arg4 ) {
		return function ( $arg1  , $arg2 ) use ( $arg3  , $arg4 ) {
			return in_array(
				$arg1  ,
				[ $arg2  ,  $arg3, $arg4 ]
			);
		};
	}

	public function doSomething() {
		$this->func( 'one'  ,  'two', 'three', 'four' );
	}
}

By the way: There is no sniff which checks the count of spaces after the comma, but this could be trigger, if each argument is on its own line

Related Objects

Duplicates Merged Here
T172551: Double spacing

Event Timeline

Is there ever a situation where there should be any whitespace before a comma?

Not sure, but possible is:

list( , , $var )
thiemowmde subscribed.

More edge-cases I found by scanning all my local codebases:

function foo( $a /* comment */, $b )
list( $a, , $b )
list( $a, $b, , )
$array = [
    'content' => <<<HEREDOC
blob
HEREDOC
    ,
    'more' => null,
]

The heredoc syntax is fascinating, because the end tag can only be followed by a semicolon, but nothing else.

The part about function declaration is fixed by enable Squiz.Functions.FunctionDeclarationArgumentSpacing - https://gerrit.wikimedia.org/r/#/c/mediawiki/tools/codesniffer/+/455225/

It seems there is no standard sniff for the comma in implements or even a sniff to check for spacing around implements

The comma in function calls or array declaration are more complex, maybe that is already tracked by Generic.Functions.FunctionCallArgumentSpacing

comma in for loop (when init two vars in first argument) is not mentioned here

Umherirrender claimed this task.

It seems there is no standard sniff for the comma in implements or even a sniff to check for spacing around implements

PSR2.Classes.ClassDeclaration is now enabled - https://gerrit.wikimedia.org/r/#/c/mediawiki/tools/codesniffer/+/455626/

When there are remaining parts, it should be handled in separate task, each for a part.