PHP 8.3 readonly anonymous classes: add tests to two sniffs #2550
Conversation
This is necessary to be able to create more test case files with syntax errors in a future commit.
… error to a separate file
…us classes This commit adds a few tests using readonly anonymous classes to the `WordPress.NamingConventions.ValidFunctionName` tests. This is just to ensure that the part of the sniff code that checks for `T_ANON_CLASS` tokens works correctly with readonly anonymous class added in PHP 8.3. The sniff was already handling readonly anonymous classes correctly, and no change to the sniff code is needed.
…nymous classes This commit modifies an existing test to use readonly anonymous classes to the `WordPress.NamingConventions.PrefixAllGlobals` tests. This is just to ensure that the part of the sniff code that checks for `T_ANON_CLASS` tokens works correctly with readonly anonymous class added in PHP 8.3. The sniff was already handling readonly anonymous classes correctly, and no change to the sniff code is needed.
There was a problem hiding this comment.
I have no objection to these changes and am fine with them being merged.
Having said that, in both these cases, the tests are superfluous, as the sniff code (nor the underlying functions from PHPCSUtils or from our own Helpers) doesn't do any token walking to find anything before the T_ANON_CLASS token, so there was no risk of anything breaking anyway.
I've done some token searches to verify if there are other sniffs which need tests and based on that, tests are actually needed for the following sniffs:
EscapeOutput- code in thecase \T_THROWblock will yield false negatives for readonly anonymous classes and anonymous classes with attributes for code likethrow new [...] class( $unescaped ) {};.
My approval is for the PR as-is and presumes the missing tests for EscapeOutput will be addressed in a separate PR.
|
Thanks for your review, @jrfnl!
I have a question about this, and I opened an issue for us to continue the conversation instead of asking here in this PR: #2552 |
3 participants
Readonly anonymous classes were introduced in PHP 8.3 (https://www.php.net/manual/en/language.oop5.anonymous.php#language.oop5.anonymous.readonly). PHPCS started supporting it in version 3.9.0 (PHPCSStandards/PHP_CodeSniffer#309). 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.ValidFunctionNameandWordPress.NamingConventions.PrefixAllGlobalsto safeguard that those two sniffs continue to handle readonly anonymous classes correctly, as both sniffs have code related to theT_ANON_CLASStoken. 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 readonly anonymous class syntax.
To be able to add tests to
WordPress.NamingConventions.ValidFunctionName, in two separate commits, I had to rename the test case file and move a test with an intentional syntax error to a separate file.