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

Upgrade to WordPressCS 3.0.0 #5047

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Aug 21, 2023

CS: align equal signs for consecutive statements (pre-existing)

Composer: update CS dependencies

WordPressCS 3.0.0 has been released.

This commit updates the Composer dependencies to use the new version, including updating the underlying PHP_CodeSniffer dependency to the new minimum supported version for WPCS.

Note: the Composer PHPCS installer plugin is no longer explicitly required as it is now a dependency of WordPressCS, so we inherit the dependency automatically.

PHPCS: improve organisation of the WordPressCS based PHPCS ruleset

No functional changes.

This commit:

  • Adds section headers to the ruleset file.
  • Organizes all directives in their respective sections.

PHPCS: update the ruleset for WordPressCS 3.0.0

This commit:

  • Raises the memory limit to be on the safe side as WPCS 3.0.0 contains a lot more sniffs.
  • Removes explicit inclusions of extra rules, which have now been added to the WordPress-Core ruleset..
  • Updates property names for select sniffs.
  • Updates one exclusion - the WordPress.CodeAnalysis.AssignmentInCondition sniff has been (partially) replaced by the Generic.CodeAnalysis.AssignmentInCondition sniff.
  • Adds one new exclusion.

PHPCS: remove the custom property for the FileName sniff

Test files should comply with the naming conventions accepted by PHPUnit, not with the WP naming conventions.

While the WordPress.File.FileName sniff makes a best effort to prevent throwing errors for files containing test classes, the recommended manner to deal with test files is to exclude the complete test directory from being subject to this sniff.

This updates the ruleset to follow the recommendation.

PHPCS: downgrade one new error to a warning

The Generic.Files.OneObjectStructurePerFile sniff enforces that there is only one OO structure declaration per file.

At this time, this sniff would yield 29 errors.

By downgrading the sniff to a warning, the build can pass and we buy ourselves time to fix these 29 issues.

For the time being, the test directory will be excluded until the issues are fixed (as the test directory CS run does not allow for warnings).

CS: update ignore annotations for WPCS 3.0.0

CS: remove redundant ignore annotations [1] / never needed

Remove ignore annotations which are ignoring an error which would not be thrown for that code.

Includes tidying up the format of the ignore annotation:

  • Customary one space between the // and the start of the comment.
  • There should be no spaces in the comma-separated sniff list.

CS: remove redundant ignore annotations [2] / not needed

Remove ignore annotations which are unnecessary due to the configuration in the phpcs.xml.dist ruleset already taking care of this.

CS: remove redundant ignore annotations [3] / no longer needed

Remove ignore annotations about invalid function names for deprecated functions. Those are ignored by design by WPCS since version 2.2.0.

CS: remove redundant ignore annotations [4] / Unused WPCS rules

Remove ignore annotations related to sniffs which are not used by WP Core (like sniffs which are in the WordPress-Extra ruleset).

CS: remove redundant ignore annotations [5] / Non-WPCS rules

The VariableAnalysis standard is not used by WP Core.

CS: use pre-[in|de]crement instead of post-[in|de]crement for stand-alone statements

CS: remove superfluous blank line(s) at end of file

CS: remove redundant blank line(s) at end of classes

CS: remove redundant blank line(s) at end of functions

CS: fix spacing for spread operators

No space allowed between the operator and the variable it applies to.

Modernize: use dirname() with the $levels parameter

PHP 7.0 introduced the $levels parameter to the dirname() function, which means nested calls to dirname() are no longer needed.

Ref: https://www.php.net/manual/en/function.dirname.php

CS: one space after function keyword for closures

CS: remove redundant semi-colons

As these are not closures/anonymous classes, these semi-colons are redundant and create an empty PHP statement.

🆕 QA: don't use reserved keyword as parameter in newly introduced function

Introduced via Trac 41125.

GH Actions: tweak coding standards workflow

The PHPCS run can now be run on the latest PHP version as all known PHP 8.x compatibility issues (in WPCS) have been fixed.

Trac ticket: https://core.trac.wordpress.org/ticket/59161


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@jrfnl jrfnl force-pushed the trac-59161/upgrade-to-wpcs-3.0.0-phpcompat-3.0.0 branch from b5427de to 9762a5b Compare August 21, 2023 14:55
@jrfnl jrfnl force-pushed the trac-59161/upgrade-to-wpcs-3.0.0-phpcompat-3.0.0 branch from ec4030c to 1c0be61 Compare September 5, 2023 09:32
@jrfnl
Copy link
Member Author

jrfnl commented Sep 5, 2023

FYI: I've recreated the PR to get it out of conflict state and to get rid of newly introduced issues. Includes one new commit (annotated in the PR description).

Would be lovely if we could get some movement on this PR to prevent having to keep recreating the PR.....

@jrfnl jrfnl force-pushed the trac-59161/upgrade-to-wpcs-3.0.0-phpcompat-3.0.0 branch from 1c0be61 to b6c0baf Compare September 12, 2023 23:16
@jrfnl
Copy link
Member Author

jrfnl commented Sep 12, 2023

Rebased the PR to allow for seeing the current status after a number of commits have gone in (thanks @SergeyBiryukov !).

Includes one new commit to address newly introduced issues.

@jrfnl jrfnl force-pushed the trac-59161/upgrade-to-wpcs-3.0.0-phpcompat-3.0.0 branch from b6c0baf to eb80f7c Compare September 23, 2023 04:09
@jrfnl
Copy link
Member Author

jrfnl commented Sep 23, 2023

Rebased the PR to allow for seeing the current status after a number of commits have gone in (thanks @SergeyBiryukov !).

Includes three (!) new commits to address newly introduced issues.

@@ -2333,7 +2333,6 @@ private function matches() {
* See https://html.spec.whatwg.org/#space-separated-tokens
*/
while (
// phpcs:ignore WordPress.CodeAnalysis.AssignmentInCondition.FoundInWhileCondition
Copy link
Member

Choose a reason for hiding this comment

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

thanks for the update! this line should disappear from the PR on the next rebase, as the code it's modifying is now gone

@SergeyBiryukov SergeyBiryukov force-pushed the trac-59161/upgrade-to-wpcs-3.0.0-phpcompat-3.0.0 branch from b79dd75 to eb80f7c Compare October 2, 2023 12:23
@jrfnl
Copy link
Member Author

jrfnl commented Oct 2, 2023

@SergeyBiryukov Looks like some merge artifact crept in with that merge ? Want me to rebase instead ?

@SergeyBiryukov
Copy link
Member

Looks like some merge artifact crept in with that merge ?

Yeah, I tried to rebase to see what's left and move those changes to Gutenberg, but something went wrong, and I reverted it.

Want me to rebase instead ?

That would be great 🙂

@jrfnl jrfnl force-pushed the trac-59161/upgrade-to-wpcs-3.0.0-phpcompat-3.0.0 branch from eb80f7c to 0565ce5 Compare October 5, 2023 22:07
@jrfnl
Copy link
Member Author

jrfnl commented Oct 5, 2023

Want me to rebase instead ?

That would be great 🙂

Done!

Remove ignore annotations which are unnecessary due to the configuration in the `phpcs.xml.dist` ruleset already taking care of this.
Remove ignore annotations related to sniffs which are not used by WP Core (like sniffs which are in the `WordPress-Extra` ruleset).
@jrfnl jrfnl force-pushed the trac-59161/upgrade-to-wpcs-3.0.0-phpcompat-3.0.0 branch from 0565ce5 to 6cedcd7 Compare October 19, 2023 01:33
@jrfnl
Copy link
Member Author

jrfnl commented Oct 19, 2023

Rebased again just so we can see what's left.

@SergeyBiryukov
Copy link
Member

Rebased again just so we can see what's left.

@jrfnl Thanks! I have opened WordPress/gutenberg#55615 to handle the change in wp-includes/blocks/calendar.php.

For the change in wp-includes/class-wp-block-parser-block.php, however, it appears that it cannot be merged as-is upstream, since Gutenberg does not have a corresponding section in phpcs.xml.dist.

I think we can instead remove the section from WP Core's phpcs.xml.dist file, as it does not seem necessary with the affected lines already ignored in the file itself. See 59161.wp-block-parser-block.diff. What do you think?

@SergeyBiryukov
Copy link
Member

I think we can instead remove the section from WP Core's phpcs.xml.dist file, as it does not seem necessary with the affected lines already ignored in the file itself. See 59161.wp-block-parser-block.diff.

Done in r57017. That was the last of the changes here, so I believe this PR is now fully addressed.

@jrfnl jrfnl deleted the trac-59161/upgrade-to-wpcs-3.0.0-phpcompat-3.0.0 branch October 27, 2023 23:35
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 this pull request may close these issues.

3 participants