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

WP/EnqueuedResourceParameters: implement PHPCSUtils and support modern PHP #2177

Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 21, 2022

WP/EnqueuedResourceParameters: add support for PHP 8.0+ named parameters

  1. Adjusted the way the correct parameters are retrieved to use the new PHPCSUtils 1.0.0-alpha4 PassedParameters::getParameterFromStack() method.
  2. Verified the parameter names used are in line with the name as per the WP 6.1 release.
    WP has been renaming parameters and is probably not done yet, but it doesn't look like those changes (so far) made it into changelog entries....
    For the purposes of this exercise, I've taken the current parameter name as the "truth" as support for named parameters hasn't officially been announced yet, so any renames after this moment are the only ones relevant.

Includes additional unit tests.

WP/EnqueuedResourceParameters: prevent false positives

The 'raw' key in the parameter arrays returned from the PassedParameters class contains - as per the name - the raw contents of the parameter.

Since PHPCSUtils 1.0.0-alpha4, the return array also contain a 'clean' index, which contains the contents of the parameter cleaned of comments.

By switching to using that key, a null value accompanied by a comment will no longer throw the wrong error.

Includes unit test demonstrating the issue and safeguarding the fix.

WP/EnqueuedResourceParameters: improve error position precision

... for multi-line function calls.

WP/EnqueuedResourceParameters: add support for PHP 7.4/8.1 numbers

PHP 7.4 introduced numeric literals with underscores.
PHP 8.1 introduced explicit octal notation.

While PHPCS backfills the tokens for this, this sniff uses an eval() call and if either of these PHP 7.4/8.1 syntaxes would be passed to this eval() while PHPCS is being run on PHP < 7.4, this will result in a parse error on the code passed to eval().

By using the PHPCSUtils Numbers::getCompleteNumber() method, we can get access to the decimal value of the encountered number, which allows us to bypass the issue by using that in the code string passed to eval().

Includes unit tests.

1. Adjusted the way the correct parameters are retrieved to use the new PHPCSUtils 1.0.0-alpha4 `PassedParameters::getParameterFromStack()` method.
2. Verified the parameter names used are in line with the name as per the WP 6.1 release.
    WP has been renaming parameters and is probably not done yet, but it doesn't look like those changes (so far) made it into changelog entries....
    For the purposes of this exercise, I've taken the _current_ parameter name as the "truth" as support for named parameters hasn't officially been announced yet, so any renames _after_ this moment are the only ones relevant.

Includes additional unit tests.
The `'raw'` key in the parameter arrays returned from the `PassedParameters` class contains - as per the name - the _raw_ contents of the parameter.

Since PHPCSUtils 1.0.0-alpha4, the return array also contain a `'clean'` index, which contains the contents of the parameter cleaned of comments.

By switching to using that key, a `null` value accompanied by a comment will no longer throw the wrong error.

Includes unit test demonstrating the issue and safeguarding the fix.
PHP 7.4 introduced numeric literals with underscores.
PHP 8.1 introduced explicit octal notation.

While PHPCS backfills the tokens for this, this sniff uses an `eval()` call and if either of these PHP 7.4/8.1 syntaxes would be passed to this `eval()` while PHPCS is being run on PHP < 7.4, this will result in a parse error on the code passed to `eval()`.

By using the PHPCSUtils `Numbers::getCompleteNumber()` method, we can get access to the decimal value of the encountered number, which allows us to bypass the issue by using that in the code string passed to `eval()`.

Includes unit tests.
Copy link
Member

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GaryJones GaryJones merged commit 4bae5f0 into develop Dec 22, 2022
@GaryJones GaryJones deleted the feature/enqueuedresourceparams-phpcsutils-and-modern-php branch December 22, 2022 10:00
@jrfnl jrfnl mentioned this pull request Jan 7, 2023
19 tasks
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.

3 participants