Generic/FunctionCallArgumentSpacing: minor tweaks + bug fix for anonymous classes

[jrfnl]

Make the sniff more efficient by sniffing for fewer tokens

The Tokens::$functionNameTokens array includes a number of tokens which will never trigger errors from this sniff as they only expect one parameter. Think: empty(), exit().
However, as the tokens are registered for the sniff, the sniff will be triggered each time those tokens are encountered.

I have changed the register() method to only register the tokens which will/could actually trigger errors, making the sniff more efficient.

Confirm correct behaviour with PHP 7.3 trailing commas in function calls

Added an extra unit test to confirm that the sniff handles PHP 7.3 trailing commas in function calls correctly.

Improve handling of anonymous classes

Spacing around equal operators within anonymous classes should be ignored by this sniff, but wasn't.

This fixes false positives being thrown by this sniff and most likely also fixes a fixer conflict, though I haven't gone looking for specifics, but I imagine that the false positives caused a fixer conflict with a function declaration sniff which demands no spaces around the equal sign for parameter default values. (as is used in the PHPCS native standard)

Includes unit test.

As a sidenote, I wonder whether the check for spacing around the equal operator should even be included in this sniff as "default values" in function calls is not a thing.

[jrfnl]

I've added a second commit to this PR containing a bug fix and have updated the PR description with info on this commit.

[gsherwood]

As a sidenote, I wonder whether the check for spacing around the equal operator should even be included in this sniff as "default values" in function calls is not a thing.

That's a really good point :) I'm not sure why that code is in this sniff - maybe I did a copy/paste from one of the function declaration sniffs back in the day.

[jrfnl]

@gsherwood I'd be happy to send in a PR to remove the "equal sign spacing" check from the sniff, especially as it probably also causes fixer conflicts with sniffs which just and only check spacing around assignment operators and where the default spacing is not 1 or is configurable.

The reason for suggesting a separate PR is that this PR is marked for the 3.4.1 milestone and I imagine removing those checks would have to go into a minor, not a patch version.

[gsherwood]

The reason for suggesting a separate PR is that this PR is marked for the 3.4.1 milestone and I imagine removing those checks would have to go into a minor, not a patch version.

If the errors can never be generated because they are checking for code that can't exist, I'm happy to include those removals in a bug fix release.

[gsherwood]

Thanks for this change.

[jrfnl]

If the errors can never be generated because they are checking for code that can't exist, I'm happy to include those removals in a bug fix release.

@gsherwood The errors can exist, code like function_call( $a = 100 ) does work and won't cause a parse error, but it's uncommon and doesn't set a "default" value.

The code in effect is the same as the below:

$a = 100;
function_call( $a );

I.e. it sets the variable within the scope and that variable will be available after the function call as well, and then it passes the variable to the function call.

[jrfnl]

@gsherwood I've done a little digging and with the existing standards, these codes at least don't cause any fixer conflicts, though depending on the standard, they do cause duplicate notices.

• PEAR, PSR2 and Zend don't show duplicate notices, but also don't show notices if a $a=100; appears anywhere else in the code.
• PSR12 shows duplicate notices via the PSR12.Operators.OperatorSpacing sniff (which is the more appropriate sniff to use for this).
• Squiz shows duplicate notices via the Squiz.WhiteSpace.OperatorSpacing sniff (which is the more appropriate sniff to use for this) and will also complain about "assignments must be the first block of code on a line" via Squiz.PHP.DisallowMultipleAssignments.

All in all, I think removing the errorcodes is appropriate as either the PSR12.Operators.OperatorSpacing or the Squiz.WhiteSpace.OperatorSpacing sniff can be used if spacing around assignment operators should be checked within a standard.

As neither of these sniffs are included in the PEAR, PSR2 or Zend standards, I presume those standards don't actually have an opinion on spacing around assignment operators and if that's the case, the error currently being thrown is inappropriate anyway.

[jrfnl]

PR #2399 takes care of removing the assignment operator checks.