Sniff::get_function_call_parameters(): correctly handle closures when passed as param #1387
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrfnl
force-pushed
the
feature/fix-function-call-params-method
branch
from
83e1666
to
cfc54c5
Compare
… passed as param Closures can be declared within function calls when the function expects a call-back. This situation was only partially handled within the `Sniff::get_function_call_parameters()` method. Most of the time when a comma is encountered within a closure, it will be at a different nesting level than the original function call/array, which meant it would be disregarded and the method would return the correct results. However, there are some, albeit rare, situations in which the comma would be at the target nesting level, causing the method to split the closure into two or more "parameters". As far as I know, this bug has not caused any issues for any of the WPCS sniffs so far, but the situation should be handled correctly even so. I came across this while working on 1371, the PR for which will contain a unit test covering this fix. Additionally, the same fix has been pulled to the PHPCompatibility standard with dedicated unit tests. See: PHPCompatibility/682 Related to 764
|
Any unit tests needed for this? |
While the unit tests in PHPCompatibility are of course in a different repo, the function used is the same in both (as I wrote & pulled it) and PHPCompatibility is better set up to unit test utility methods, while WPCS is not. The unit test in the |
GaryJones
approved these changes
Jun 26, 2018
JDGrimes
approved these changes
Jun 26, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closures can be declared within function calls when the function expects a call-back.
This situation was only partially handled within the
Sniff::get_function_call_parameters()method.Most of the time when a comma is encountered within a closure, it will be at a different nesting level than the original function call/array, which meant it would be disregarded and the method would return the correct results.
However, there are some, albeit rare, situations in which the comma would be at the target nesting level, causing the method to split the closure into two or more "parameters".
As far as I know, this bug has not caused any issues for any of the WPCS sniffs so far, but the situation should be handled correctly even so.
I came across this while working on #1371, the PR for which will contain a unit test covering this fix.
Additionally, the same fix has been pulled to the PHPCompatibility standard with dedicated unit tests.
See: PHPCompatibility/PHPCompatibility#682
Related to #764