Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic/FunctionCallArgumentSpacing: bug fix - ignore commas in nested match structures + minor efficiency tweak #513

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented May 24, 2024

Description

Follow up on #497, see #497 (review)

Generic/FunctionCallArgumentSpacing: efficiency fix - skip over arrow functions

PHP 7.4 introduced arrow functions.

While arrow functions being passed as an argument in a function call will not lead to false positives for this sniff - at least, I haven't been able to come up with a code sample in which it would 1 -, skipping over them is still beneficial as it prevents unnecessary token walking.

Mind: the scope_closer of the arrow function may be the comma separating the arrow function argument from the next function call argument, so we need to step one token back to prevent false negatives on those comma's.

Either way, this is now handled.

Includes unit tests.

Generic/FunctionCallArgumentSpacing: bug fix - ignore commas in nested match structures

PHP 8.0 introduced match control structures, which can be passed in a function call (though probably/hopefully this is not very common as it makes for harder to read code).

The comma's within match control structures should be checked by a sniff which handled that control structure and should not be treated as comma's belonging to the function call.

As things are, this is currently not the case, which leads to false positives.

Fixed now.

Includes test.

Suggested changelog entry

  • Minor efficiency improvements for Generic.Functions.FunctionCallArgumentSpacing
  • Fixed bug: Generic.Functions.FunctionCallArgumentSpacing did not ignore the contents of a match expressions passed as a function argument.

Related issues/external references

Related to #497

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Footnotes

  1. comma's in arrow functions will always be nested within parentheses, within a short array or within a nested closure or anonymous class, all of which the sniff already ignores.

jrfnl added 2 commits May 24, 2024 10:49
… functions

PHP 7.4 introduced arrow functions.

While arrow functions being passed as an argument in a function call will not lead to false positives for this sniff - at least, I haven't been able to come up with a code sample in which it would [^1] -, skipping over them is still beneficial as it prevents unnecessary token walking.

Mind: the `scope_closer` of the arrow function _may_ be the comma separating the arrow function argument from the next function call argument, so we need to step one token back to prevent false negatives on those comma's.

Either way, this is now handled.

Includes unit tests.

[^1]: comma's in arrow functions will always be nested within parentheses, within a short array or within a nested closure or anonymous class, all of which the sniff already ignores.
…d match structures

PHP 8.0 introduced match control structures, which can be passed in a function call (though probably/hopefully this is not very common as it makes for hard to comprehend code).

The comma's within match control structures should be checked by a sniff which handled that control structure and should not be treated as comma's belonging to the function call.

As things are, this is currently not the case, which leads to false positives.

Fixed now.

Includes test.
@jrfnl jrfnl added this to the 3.10.x Next milestone May 24, 2024
@jrfnl jrfnl merged commit 1114d35 into master Jun 2, 2024
50 checks passed
@jrfnl jrfnl deleted the feature/generic-functioncallargument-spacing-bugfix-effciency-fix branch June 2, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant