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

Ensure Compatibility with PHP 8.0 and 8.1 #21

Closed
kevinfodness opened this issue Apr 11, 2022 · 6 comments · Fixed by #30
Closed

Ensure Compatibility with PHP 8.0 and 8.1 #21

kevinfodness opened this issue Apr 11, 2022 · 6 comments · Fixed by #30

Comments

@kevinfodness
Copy link
Member

In preparation for upgrading environments to use PHP 8.0 and 8.1, ensure these coding standards and their dependencies do not throw deprecation warnings or any other unexpected output when used in an environment that is set to PHP 8.0 or 8.1.

For example:

An error occurred during processing; checking has been aborted. The error message was: trim(): Passing null to parameter #1 ($string) of type string is deprecated in ~/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php on line 280phpcs

(Note that the above may have already been fixed in this repo by bumping dependency versions, but we need to validate this)

@srtfisher
Copy link
Member

Following along to this note, PHP 8.0 is supported mostly but 8.1 does not pass at all. The latest version of https://github.com/WordPress/WordPress-Coding-Standards/ (2.3) fails terribly with that error message you noted: WordPress/WordPress-Coding-Standards#2035

Hopefully, we'll be getting a 3.0 soon: WordPress/WordPress-Coding-Standards#1877

@emilyatmobtown
Copy link

emilyatmobtown commented Feb 16, 2023

WPCS is failing when it checks for a runtime value for prefixes and text_domain. When it does this a null value is passed to trim() which triggers a deprecation warning in PHP 8.1. This results in false phpcs errors.

An interim workaround is to set runtime values for prefixes and text_domain until the update is released.

@kevinfodness
Copy link
Member Author

Alternately, we could modify the README to indicate that users should define these values in their config when using this standard (which we are already doing internally, but could formalize this expectation within this package).

@emilyatmobtown
Copy link

emilyatmobtown commented Feb 17, 2023

@kevinfodness To clarify, setting the values in the ruleset (ours or the user's) doesn't address the bug. The values must be set on the command line with --runtime-set text_domain <domain> --runtime-set prefixes <prefixes>, whether or not they're also set in the ruleset. This differs from our internal approach, but I agree we can include it in the README as a necessary workaround for 8.1 support until WPCS releases their fix.

@kevinfodness
Copy link
Member Author

Ah, yes, that is an issue then. Thanks for flagging!

@srtfisher
Copy link
Member

#30 will fix this with documentation. We'll need to wait for the full wp-coding-standards/wpcs 3.0 release to fully fix this. The timing on that is unclear at the moment: WordPress/WordPress-Coding-Standards#1877

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants