Make WordPress Core

Opened 4 weeks ago

Closed 3 weeks ago

Last modified 2 weeks ago

#64627 closed defect (bug) (fixed)

Coding Standards: Remove or update stale PHPCS annotations

Reported by: rodrigosprimo's profile rodrigosprimo Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 7.0 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch has-unit-tests
Focuses: coding-standards Cc:

Description

The WordPress Core codebase contains a number of stale inline PHPCS annotations (phpcs:ignore, phpcs:disable) that are no longer necessary. I would like to suggest updating or removing them depending on the case.

They fall into several categories:

Annotations with outdated error codes

In schema.php and ms-site.php, the phpcs:ignore annotations reference WordPress.DB.PreparedSQL.NotPrepared, but the actual violation is WordPress.DB.PreparedSQL.InterpolatedNotPrepared. The error code was split in WPCS 2.0.0 (see WordPress/WordPress-Coding-Standards#1601).

Annotations for sniffs that no longer flag the code

In deprecated.php, the PHPCompatibility.FunctionNameRestrictions.ReservedFunctionNames.FunctionDoubleUnderscore annotations on __ngettext() and __ngettext_noop() are no longer needed. Since PHPCompatibility 9.3.2 (see PHPCompatibility/PHPCompatibility#917), the sniff skips functions with a @deprecated docblock tag.

In class-twentytwenty-svg-icons.php, the WordPress.WP.CapitalPDangit.Misspelled annotation on the 'wordpress' array key is unnecessary because the CapitalPDangit sniff skips array definitions by design.

Annotations for standards not installed in this repository

Three WPThemeReview.CoreFunctionality.FileInclude.FileIncludeFound annotations in Twenty Twenty-One and one VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable annotation in WP_REST_Font_Collections_Controller reference standards that are not installed. These were likely carried over from upstream repositories.

Legacy annotation format no longer recognized by PHPCS

Two // WPCS: XSS OK. comments in Twenty Nineteen use a legacy annotation format that was deprecated in WPCS 2.0.0 (WordPress/WordPress-Coding-Standards#1580) and removed in WPCS 3.0.0 (WordPress/WordPress-Coding-Standards#1908).

Annotations for sniffs not checked in this repository

About 30 annotations across multiple files reference sniffs from the WordPress-Extra standard (EscapeOutput, NonceVerification, AlternativeFunctions, DevelopmentFunctions, IniSet), which is not included in the phpcs.xml.dist configuration (the repository uses WordPress-Core).

Expected impact

Running composer lint after applying the changes suggested here produces two fewer warnings compared to trunk: the WordPress.DB.PreparedSQL.InterpolatedNotPrepared warnings in schema.php and ms-site.php are now properly suppressed by the corrected annotations. The remaining changes have no effect on the output of composer lint or composer compat, as all other removed annotations were already ineffective.

Related: #42808.

Change History (13)

This ticket was mentioned in PR #10903 on WordPress/wordpress-develop by @rodrigosprimo.


4 weeks ago
#1

  • Keywords has-patch has-unit-tests added

#2 @SergeyBiryukov
4 weeks ago

  • Milestone changed from Awaiting Review to 7.0
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#3 @SergeyBiryukov
4 weeks ago

In 61615:

Coding Standards: Correct two ignore annotations for WordPress.DB.PreparedSQL.

The WordPress.DB.PreparedSQL sniff contains two different error messages, which initially shared the same error code. A unique error code for the second message was added in WPCS 2.0.0.

Reference: PreparedSQL: use unique errorcode for messages.

Follow-up to [43628], [43630], [43654], [44472].

Props rodrigosprimo.
See #64627.

@mukesh27 commented on PR #10903:


4 weeks ago
#4

@rodrigoprimo Could you please rebase your branch? Some parts of this PR were already committed in https://github.com/WordPress/wordpress-develop/commit/259d1f95fe664737e19cbb7028e4d319d992d913, so we’ll need to sync it with the latest trunk before proceeding.

@rodrigosprimo commented on PR #10903:


4 weeks ago
#5

@mukeshpanchal27, thanks for the ping. I just rebased this PR with the latest from trunk.

#7 @SergeyBiryukov
3 weeks ago

In 61641:

Coding Standards: Remove unnecessary annotations for __ngettext() and __ngettext_noop().

Since PHPCompatibility 9.3.2, the ReservedFunctionNames sniff skips functions with a @deprecated tag.

Reference: WPCS: ReservedFunctionNames: ignore deprecated functions.

Follow-up to [46290].

Props rodrigosprimo.
See #64627.

#8 @SergeyBiryukov
3 weeks ago

In 61643:

Twenty Twenty: Remove EscapeOutput annotations.

These are from the WordPress-Extra standard, which is not included in the phpcs.xml.dist configuration (the repository uses WordPress-Core).

Includes removing unnecessary WordPress.WP.CapitalPDangit.Misspelled annotation on the wordpress array key, as the CapitalPDangit sniff skips array definitions by design.

Follow-up to [46271], [46445].

Props rodrigosprimo.
See #64627.

#9 @SergeyBiryukov
3 weeks ago

In 61654:

Twenty Twenty-One: Remove EscapeOutput annotations.

These are from the WordPress-Extra standard, which is not included in the phpcs.xml.dist configuration (the repository uses WordPress-Core).

Includes removing unnecessary WPThemeReview annotations carried over from upstream.

Follow-up to [49216].

Props rodrigosprimo.
See #64627.

#10 @SergeyBiryukov
3 weeks ago

In 61668:

Coding Standards: Remove a few unnecessary PHPCS annotations.

Follow-up to [57337], [57548], [60295], [61120].

Props rodrigosprimo.
See #64627.

@SergeyBiryukov commented on PR #10903:


3 weeks ago
#11

Thanks for the PR! Merged in r61615, r61630, r61641, r61643, r61654, and r61668.

#12 @SergeyBiryukov
3 weeks ago

  • Resolution set to fixed
  • Status changed from accepted to closed

#13 @dmsnell
2 weeks ago

Great work @rodrigosprimo! This is helpful work removing noise from our codebase. 👏

Note: See TracTickets for help on using tickets.