Generic/FunctionCallArgumentSpacing: improve code coverage

[rodrigoprimo]

Description

This PR improves code coverage for the Generic.Functions.FunctionCallArgumentSpacing sniff.

Related issues/external references

Part of #146

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
• Documentation improvement

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.
• [Required for new sniffs] I have added XML documentation for the sniff.

[jrfnl]

Thanks @rodrigoprimo !

The changes as-is are fine.

Having said that, I do think, there are more tests missing, all related to more modern PHP.

There are no tests with attributes, while the sniff does act on them:

#[AttributeName(1,2)]

There are no tests with first class callables. While these would not lead to errors being reported or false positives, I still think it would be good to have a test covering that code as the sniff will be triggered for them.

$callable = myCallable(...);

---

Other than that (not to be addressed in this PR):

The sniff skips over closures, anon classes and (short) arrays which may be passed as a parameter in a function call. It does so to prevent checking the spacing around commas in these (nested) structures, which may have their own rules.

The sniff, however, does not skip over inline match control structures, while I think that it should.

The sniff also doesn't skip over arrow functions, while I believe it should do so too. For arrow functions, I can't come up with a code sample which would lead to false positives as any commas would always have to be in nested parentheses or within a short array/match. Having said that, I think skipping them is still the right choice as it makes the sniff slightly more efficient.

Either way, I've prepped a PR to fix these last two (as writing up the issue reminder would take me longer than creating the PR) and will pull that once this PR has been merged (to prevent creating a conflict).

[rodrigoprimo]

Thanks for the review, @jrfnl!

There are no tests with attributes, while the sniff does act on them:
There are no tests with first class callable.

I added the tests that you suggested.

Regarding the attributes, I added the test you shared that triggers the sniff, and I also added another test that does not trigger the sniff to document the expected behavior. I did not add more tests as it seems to me, for the purposes of this sniff, the attributes behave just like a function call.

[jrfnl]

Regarding the attributes, I added the test you shared that triggers the sniff, and I also added another test that does not trigger the sniff to document the expected behavior.

Well, technically, both code samples trigger the sniff, but only one will trigger an error from the sniff....

[jrfnl]

Thanks @rodrigoprimo !

I will rebase and squash the last commit before merging.

[jrfnl]

FYI: I've pulled the follow up mentioned in one of my previous comments as PR #513