PHP 8.4 asymmetric visibility properties: add tests to four sniffs #2551
Conversation
This file contains hidden or 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
…lity properties This commit adds a few tests using asymmetric visibility properties (including in constructor property promotion) to the `WordPress.NamingConventions.PrefixAllGlobals` tests. This is just to ensure that the part of the sniff code that ignores properties or method parameters (in the case of constructor property promotion) continues to handle PHP 8.4+ asymmetric visibility properties correctly. No change to the sniff code is needed.
…ility properties This commit adds a few tests using asymmetric visibility properties (including in constructor property promotion) to the `WordPress.NamingConventions.ValidVariableName` tests. This is just to ensure that the sniff continues to apply its variable name rules when dealing with asymmetric visibility properties added in PHP 8.4. The sniff was already handling asymmetric visibility properties correctly, and no change to the sniff code is needed.
…erties This commit adds a test using asymmetric visibility properties to the `WordPress.Security.NonceVerification` tests. This is just to ensure that the sniff continues to ignore PHP 8.4+ asymmetric visibility properties. The sniff was already handling asymmetric visibility properties correctly, and no change to the sniff code is needed.
Also update code comment related to the moved parse error test to include one more case where the code might bow out.
…perties This commit adds a few tests using asymmetric visibility properties (including in constructor property promotion) to the `WordPress.WP.GlobalVariablesOverride` tests. This is just to ensure that the part of the sniff code that ignores properties or method parameters (in the case of constructor property promotion) continues to handle PHP 8.4+ asymmetric visibility properties correctly. No change to the sniff code is needed.
rodrigoprimo
force-pushed
the
asymmetric-visibility-properties
branch
from
7f8a941 to
a1f0108
Compare
|
I had to force push without changes to rerun the build because of an unrelated |
jrfnl
approved these changes
Jul 25, 2025
There was a problem hiding this comment.
Thanks for setting this up @rodrigoprimo !
The test for the ValidVariableName sniff is actually testing something.
The other tests aren't, but don't do any harm either. No objections to those going in, but they also don't add much value.
dingo-d
approved these changes
Aug 5, 2025
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.
Asymmetric visibility properties were introduced in PHP 8.4 (https://wiki.php.net/rfc/asymmetric-visibility-v2). PHPCS started supporting it in version 3.13.0 (PHPCSStandards/PHP_CodeSniffer#871). Since WPCS requires PHPCS 3.13.x, with @jrfnl's help, I investigated what needed to be updated in the WPCS repository to accommodate this new syntax.
In this PR, I'm adding tests to
WordPress.NamingConventions.PrefixAllGlobals,WordPress.NamingConventions.ValidVariableName,WordPress.Security.NonceVerification, andWordPress.WP.GlobalVariablesOverrideto safeguard that those four sniffs continue to handle asymmetric visibility properties correctly, as those sniffs have code related to class properties or parameters passed to the class constructor (in the case of asymmetric visibility combined with constructor promoted properties). No changes were required to the code of those sniffs.As far as I could check, no further changes are required in the WPCS codebase to accommodate the new asymmetric visibility properties syntax.
To be able to add tests to
WordPress.WP.GlobalVariablesOverride, I had to move a test with an intentional syntax error to a separate file. While doing that, I made a small improvement to the code comment in the sniff related to the moved test.