-
Notifications
You must be signed in to change notification settings - Fork 3
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
Yoast/AlternativeFunctions: rename sniff, bug fix for fixer, support modern PHP and other improvements #338
Merged
jrfnl
merged 8 commits into
develop
from
JRF/yoastcs-alternativefunctions-various-improvements
Nov 4, 2023
Merged
Yoast/AlternativeFunctions: rename sniff, bug fix for fixer, support modern PHP and other improvements #338
jrfnl
merged 8 commits into
develop
from
JRF/yoastcs-alternativefunctions-various-improvements
Nov 4, 2023
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
…alls with trailing commas As of PHP 7.3, the parameter list for a function call can have a trailing comma. This is already handled correctly by the sniff. This commit just adjusts some pre-existing tests to safeguard this for the future.
…erator ... to safeguard that the sniff does not recognize method calls prefixed with the nullsafe object operator as calls to the global functions. Note: this is already handled by the WPCS parent sniff, this test just safeguards it.
If the original function call was already fully qualified in a namespaced file, the fixer would add a superfluous extra namespace separator - `\\WPSEO_Utils::format_json_encode()`, effectively creating a parse error in the resulting file. As fully qualified calls to global functions in namespaced files are now the norm in the Yoast repos, this is a clear bug which should be prevented. Fixed now by removing a potential leading namespace separator before doing the replacement. Includes additional tests.
…packing When a function call uses parameter unpacking, the exact parameter count is undetermined, so the auto-fixer should not kick in. The sniff did not take this into account. Fixed now. Includes tests.
... in anticipation of adding support for PHP 8.0+ function calls using named parameters. Supporting this would too complex if there is an open ended possibility of detection of additional function groups being added to this sniff. The renamed sniff will only handle the `[wp_]json_encode()` function call detection and replacement. If in the future additional functions would have Yoast specific alternative and need sniffs, this should be handled by a separate sniff/separate sniffs.
…unction calls with named parameters PHP 8.0 added support for parameter labels in function calls. This has two significant consequences for this sniff: 1. We can no longer rely on the parameter _count_ to determine whether the function call can be auto-fixed. Instead we need to specifically check for the `$value` parameter having been passed and no other parameters being passed. 2. For the auto-fixer, we can no longer just swop out the function call for its replacement. We also need to check if the passed parameter was a named parameter and if so, we may need to fix the parameter name as well. This commit refactors the sniff to take both the above into account. The refactor also removes some flexibility from the sniff, as, now the sniff has been renamed and will only check for the `[wp_]json_encode()` function calls, we no longer need to take the potential of additional function groups being checked via this sniff into account. Includes tests.
PHP 8.1 added support for "first class callables". As the number of parameters which will be passed to the callable is unknown, the sniff should flag these functions when used as first class callable, but should not auto-fix. Includes tests.
jrfnl
deleted the
JRF/yoastcs-alternativefunctions-various-improvements
branch
November 4, 2023 11:32
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.
Yoast/AlternativeFunctions: minor documentation improvements
Yoast/AlternativeFunctions: safeguard handling of PHP 7.3+ function calls with trailing commas
As of PHP 7.3, the parameter list for a function call can have a trailing comma.
This is already handled correctly by the sniff. This commit just adjusts some pre-existing tests to safeguard this for the future.
Yoast/AlternativeFunctions: add test with PHP 8.0+ nullsafe object operator
... to safeguard that the sniff does not recognize method calls prefixed with the nullsafe object operator as calls to the global functions.
Note: this is already handled by the WPCS parent sniff, this test just safeguards it.
Yoast/AlternativeFunctions: bug fix - fixer could create parse error
If the original function call was already fully qualified in a namespaced file, the fixer would add a superfluous extra namespace separator -
\\WPSEO_Utils::format_json_encode()
, effectively creating a parse error in the resulting file.As fully qualified calls to global functions in namespaced files are now the norm in the Yoast repos, this is a clear bug which should be prevented.
Fixed now by removing a potential leading namespace separator before doing the replacement.
Includes additional tests.
Yoast/AlternativeFunctions: bug fix - allow for PHP 5.6+ parameter unpacking
When a function call uses parameter unpacking, the exact parameter count is undetermined, so the auto-fixer should not kick in.
The sniff did not take this into account. Fixed now.
Includes tests.
Yoast/AlternativeFunctions: rename the sniff to JsonEncodeAlternative
... in anticipation of adding support for PHP 8.0+ function calls using named parameters. Supporting this would too complex if there is an open ended possibility of detection of additional function groups being added to this sniff.
The renamed sniff will only handle the
[wp_]json_encode()
function call detection and replacement.If in the future additional functions would have Yoast specific alternative and need sniffs, this should be handled by a separate sniff/separate sniffs.
Yoast/JsonEncodeAlternative: refactor the sniff to support PHP 8.0+ function calls with named parameters
PHP 8.0 added support for parameter labels in function calls.
This has two significant consequences for this sniff:
Instead we need to specifically check for the
$value
parameter having been passed and no other parameters being passed.This commit refactors the sniff to take both the above into account.
The refactor also removes some flexibility from the sniff, as, now the sniff has been renamed and will only check for the
[wp_]json_encode()
function calls, we no longer need to take the potential of additional function groups being checked via this sniff into account.Includes tests.
Yoast/JsonEncodeAlternative: handle PHP 8.1+ first class callables
PHP 8.1 added support for "first class callables".
As the number of parameters which will be passed to the callable is unknown, the sniff should flag these functions when used as first class callable, but should not auto-fix.
Includes tests.