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

Update the minimum_wp_version to WP 5.8 #2121

Merged
merged 2 commits into from
Dec 4, 2022

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 3, 2022

The minimum version should be three versions behind the latest WP release, so what with 6.1 being out, it should now be 5.8.

Includes adding a little more variation to the example code in the property docblock in the MinimumWPVersionTrait.

Includes updating the list of deprecated functions in the WP.DeprecatedFunctions sniff based on the WP 6.1.1 release. Input for this based on @JDGrimes's WP deprecated code scanner, though that is getting more and more difficult to run as it is no longer maintained and is throwing lots of errors.

Notes:

Includes minor tweak to make the generation of the test lines array a little more descriptive.

Previous: #1881

Closes #2031


🆕 WP/DeprecatedFunctions: temporarily exclude one test on PHP 8.2

The logic in the PHP 8.2 Tokenizer needs to be adjusted to allow for PHP 8.2 Disjunctive Normal Types in combination with readonly properties.

While PHP 8.2 still allows for the readonly keyword being used as a function call - see: https://3v4l.org/hL5da -, the PHPCS tokenizer has not been adjusted yet to allow for this in a way which is compatible with DNT.

Also see:

The minimum version should be three versions behind the latest WP release, so what with 6.1 being out, it should now be 5.8.

Includes adding a little more variation to the example code in the property docblock in the `MinimumWPVersionTrait`.

Includes updating the list of deprecated functions in the `WP.DeprecatedFunctions` sniff based on the WP 6.1.1 release.
Input for this based on @JDGrimes's WP deprecated code scanner, though that is getting more and more difficult to run as it is no longer maintained and is throwing lots of errors.

Notes:
* The `sanitize_url()` function has been _un_deprecated in WP 5.9. See 2031.
* The replacement for the deprecated `wp_cache_reset()` function has been changed in WP Core.
    Ref: https://core.trac.wordpress.org/changeset/52705
* The `_filter_query_attachment_filenames()` function lists the wrong deprecation version in Core. WPCS now contains the correct version.
    A patch has been submitted to fix the listing in Core.
    Refs:
    - https://core.trac.wordpress.org/changeset/54524
    - https://core.trac.wordpress.org/ticket/56791#comment:10
* Other than the above, no verification has been executed on the newly added deprecated function information.

Includes minor tweak to make the generation of the test lines array a little more descriptive.

Previous: 1881

Closes 2031
Copy link
Member

@dingo-d dingo-d left a comment

Choose a reason for hiding this comment

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

What is causing the PHP 8.2 test failure?

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
Copy link
Member

What is causing the PHP 8.2 test failure?

The call to readonly().

The logic in the PHP 8.2 Tokenizer needs to be adjusted to allow for PHP 8.2 Disjunctive Normal Types in combination with `readonly` properties.

While PHP 8.2 still allows for the `readonly` keyword being used as a function call - see: https://3v4l.org/hL5da -, the PHPCS tokenizer has not been adjusted yet to allow for this in a way which is compatible with DNT.

Also see:
* The discussion about this in squizlabs/PHP_CodeSniffer 3667
* php/php-src 9512
@jrfnl
Copy link
Member Author

jrfnl commented Dec 4, 2022

@dingo-d Thanks for catching that - I didn't get notified of the run failure as we still have continue-on-error in place (let's remove that in a next PR).

And yes, @GaryJones is correct, this is about the readonly() test.

I've now added an extra commit to temporarily exclude that test on PHP 8.2+ as this is a issue with the PHPCS tokenizer. Function calls to readonly() are still allowed on PHP 8.2: https://3v4l.org/hL5da (i.e. the readonly keyword exception in PHP which was put in place at my request is still in play in PHP 8.2)

@dingo-d dingo-d merged commit 6132a3a into develop Dec 4, 2022
@dingo-d dingo-d deleted the feature/update-minimum-wp-version branch December 4, 2022 16:33
@jrfnl
Copy link
Member Author

jrfnl commented Dec 4, 2022

FYI: I've opened issue #2123 as a reminder that we should remove the test exception as soon as possible.

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.

Remove deprecation Warning on sanitize_url()
3 participants