Make WordPress Core

Opened 2 months ago

Last modified 34 hours ago

#58831 new task (blessed)

Coding Standards fixes for WP 6.4

Reported by: audrasjb's profile audrasjb Owned by:
Milestone: 6.4 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch has-unit-tests
Focuses: coding-standards Cc:

Description

Previously:

Attachments (18)

58831-add-missing-translators-comment.patch (1.6 KB) - added by jrf 2 months ago.
58831-align-equal-signs-for-consecutive-statements.patch (4.7 KB) - added by jrf 2 months ago.
58831-all-methods-should-have-visibility-declared.patch (1.0 KB) - added by jrf 2 months ago.
58831-always-use-parentheses-for-class-instantiation.patch (923 bytes) - added by jrf 2 months ago.
58831-no-precision-alignment.patch (884 bytes) - added by jrf 2 months ago.
58831-no-trailing-whitespace.patch (776 bytes) - added by jrf 2 months ago.
58831-use-correct-case-for-class-name.patch (807 bytes) - added by jrf 2 months ago.
58831-Modernize-use-__DIR__-instead-of-dirname-__FILE__.patch (790 bytes) - added by jrf 2 months ago.
58831-add-missing-escape.patch (569 bytes) - added by viralsampat 4 weeks ago.
58831-add-missing-escape-wp-admin-plugins.patch (433 bytes) - added by viralsampat 4 weeks ago.
58831-add-missing-global-wpdb.patch (472 bytes) - added by viralsampat 4 weeks ago.
58831-add-missing-global-wpdb-and-missing-escaping.patch (2.9 KB) - added by viralsampat 3 weeks ago.
58831-add-missing-escape-wp-includes.patch (1.8 KB) - added by viralsampat 3 weeks ago.
58831-missing-escaping.patch (1.7 KB) - added by himanshuc 2 weeks ago.
58831-escaping-function-missing.patch (922 bytes) - added by nidhidhandhukiya 2 weeks ago.
58831.patch (2.3 KB) - added by viralsampat 2 weeks ago.
I have checked above mentioned issue and founds another file. Here, I have added its patch.
58831-escaping-function-missing-twentyeleven.patch (555 bytes) - added by himanshuc 9 days ago.
58831-escaping-function-missing.2.patch (907 bytes) - added by viralsampat 7 days ago.
I have added another patch

Download all attachments as: .zip

Change History (72)

#1 follow-up: @jrf
2 months ago

I've just uploaded 8 pretty small patches.

About half are for things which are checked by WPCS, but flagged as a warning (which doesn't fail the build) and which have slipped into the codebase unnoticed.

The other half are things which are not (yet) checked by WPCS, but are rules/best practices which have been fixed up before, for which new issues have slipped into the codebase.

#2 @audrasjb
2 months ago

Thanks! These patches all look good to me.
Do you plan to commit them? If so, I think they're all good to ship, now that 6.3 has been branched.

#3 @jrf
2 months ago

Do you plan to commit them?

I'm neck-deep in WPCS at the moment. I've asked @SergeyBiryukov if he'd be so kind as to commit these.

#4 in reply to: ↑ 1 ; follow-up: @SergeyBiryukov
2 months ago

Replying to jrf:

I've just uploaded 8 pretty small patches.

Thank you!

As previously noted in comment:1:ticket:50085, the change to __DIR__ in wp-tests-config-sample.php was previously reverted in [47201] to avoid breaking unit tests created with the wp scaffold plugin WP-CLI command, see comment:15:ticket:48082 and #49377 for details. It might be worth checking if that concern is still relevant.

Other patches look good to me at a glance :)

#5 in reply to: ↑ 4 @jrf
2 months ago

Replying to SergeyBiryukov:

As previously noted in comment:1:ticket:50085, the change to __DIR__ in wp-tests-config-sample.php was previously reverted in [47201] to avoid breaking unit tests created with the wp scaffold plugin WP-CLI command, see comment:15:ticket:48082 and #49377 for details. It might be worth checking if that concern is still relevant.

Ah, that's good to know. We may need to put an exception in place for that file once WPCS 3.0.0 comes out (which will flag this).

Having read up on the discussion, I don't think this concern will ever really go away completely, as, even though the WP-CLI scaffold plugin script has been updated, the problem is not the script itself, but the fact that there are countless plugins using a copy of that script and getting all those copies updated to the latest version is a manual process (and in the mean time CI workflows would break).

#6 @SergeyBiryukov
2 months ago

In 56273:

Coding Standards: Correct equals sign alignment in various files.

This resolves a few WPCS warnings:

Equals sign not aligned with surrounding statements

so that the output of composer format is clean.

Follow-up to [55971], [56033], [56056], [56143], [56214].

Props jrf.
See #58831.

#7 @SergeyBiryukov
2 months ago

In 56276:

I18N: Add missing translator comment in WP_Upgrader::generic_strings().

This resolves a WPCS warning:

A gettext call containing placeholders was found, but was not accompanied by a "translators:" comment on the line above to clarify the meaning of the placeholders.

Includes moving wp-content out of the translatable string in a similar message in _wp_delete_all_temp_backups().

Follow-up to [55720], [56117].

Props jrf.
See #58831.

#8 @SergeyBiryukov
2 months ago

In 56281:

Coding Standards: Always use parentheses for class instantiation.

This addresses a new stdClass() instance in _get_non_cached_ids() tests.

Follow-up to [55543].

Props jrf.
See #58831.

#9 @SergeyBiryukov
2 months ago

In 56283:

Coding Standards: Correct alignment in wp-includes/media.php.

This resolves a WPCS warning:

Found precision alignment of 1 space.

Follow-up to [55988].

Props jrf.
See #58831.

#10 @SergeyBiryukov
2 months ago

In 56285:

Coding Standards: Remove trailing tabs in wp-admin/about.php.

This resolves a WPCS warning:

Found precision alignment of 2 spaces.

Follow-up to [56263].

Props jrf.
See #58831, #58067.

#11 @SergeyBiryukov
2 months ago

In 56301:

Coding Standards: Always declare visibility for class methods.

This adds a missing public keyword for WP_HTML_Tag_Processor::get_attribute_names_with_prefix().

Follow-up to [55203].

Props jrf.
See #58831.

#12 follow-up: @dmsnell
2 months ago

@SergeyBiryukov thanks for the update - do you plan on creating the required backport to Gutenberg for your change to the Tag Processor?

#13 in reply to: ↑ 12 @SergeyBiryukov
2 months ago

Replying to dmsnell:

do you plan on creating the required backport to Gutenberg for your change to the Tag Processor?

Yup, created PR 52954. Thanks!

#16 @SergeyBiryukov
8 weeks ago

In 56319:

Coding Standards: Use correct case for class name in WP_Http tests.

Follow-up to [8516], [54968].

Props jrf.
See #58831.

#17 @SergeyBiryukov
8 weeks ago

In 56321:

Coding Standards: Use strict comparison in wp-includes/class-wp-roles.php.

Follow-up to [25695], [41625].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#18 @SergeyBiryukov
8 weeks ago

In 56324:

Coding Standards: Use strict comparison in wp-includes/feed-atom-comments.php.

Follow-up to [9818].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#19 @SergeyBiryukov
8 weeks ago

In 56325:

Coding Standards: Use strict comparison in wp-includes/formatting.php.

Follow-up to [1345], [4112], [6974], [24214], [25055], [28831], [32863].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#20 @SergeyBiryukov
7 weeks ago

In 56326:

Coding Standards: Use strict comparison in wp-includes/functions.php.

Follow-up to [5999], [6342], [7406], [8369], [10322], [11288], [11332], [11597], [12405], [13569], [14649], [15806], [19773], [26449], [26926], [39831], [40124].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#21 @SergeyBiryukov
7 weeks ago

In 56333:

Coding Standards: Use strict comparison in wp-includes/option.php.

Follow-up to [8784], [25109].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#22 @SergeyBiryukov
7 weeks ago

In 56359:

Coding Standards: Use strict comparison in wp-includes/revision.php.

Follow-up to [24414], [38118], [38433].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#23 @SergeyBiryukov
7 weeks ago

In 56360:

Coding Standards: Rewrite loose comparison in wp_list_categories().

A truthy check is more in line with similar checks elsewhere, including conditionals in the same exact code block.

Follow-up to [10275], [34696].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#24 @SergeyBiryukov
7 weeks ago

In 56361:

Coding Standards: Use strict comparison in wp-includes/class-wp-image-editor.php.

Follow-up to [22094].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#25 @SergeyBiryukov
6 weeks ago

In 56362:

Coding Standards: Use strict comparison in wp-includes/class-wp.php.

Includes minor code layout fixes for better readability.

Follow-up to [1043], [2534], [2584], [2627], [2958], [3252], [3564], [21818], [37356].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#26 @audrasjb
6 weeks ago

#58988 was marked as a duplicate.

#27 @SergeyBiryukov
6 weeks ago

In 56377:

Coding Standards: Use strict comparison in wp-includes/kses.php.

Follow-up to [649], [2896], [3418], [8386], [20540], [47219], [54933].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#28 @SergeyBiryukov
6 weeks ago

In 56394:

Coding Standards: Use strict comparison in wp-includes/cron.php.

Includes minor code layout fixes for better readability.

Follow-up to [3634], [4189].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#29 @SergeyBiryukov
6 weeks ago

In 56395:

Coding Standards: Bring more consistency to Last-Modified and ETag checks.

This updates two fragments for sending a 304 Not Modified header to better align with each other by using consistent variable names and formatting.

Follow-up to [1036], [1037], [1043], [2534], [2584], [2627], [12603], [12936], [56362].

See #58831.

#30 @SergeyBiryukov
5 weeks ago

In 56396:

Coding Standards: Use strict comparison in wp-admin/includes/class-wp-importer.php.

Follow-up to [14760], [50658].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#31 @SergeyBiryukov
5 weeks ago

In 56400:

Coding Standards: Use strict comparison in wp-admin/includes/image-edit.php.

Follow-up to [11911], [11965], [11984], [12155], [12163], [22094].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#32 @SergeyBiryukov
5 weeks ago

In 56411:

Coding Standards: Improve variable names in wp_save_image().

This resolves a few WPCS warnings:

Variable "$sX" is not in valid snake_case format, try "$s_x"
Variable "$sY" is not in valid snake_case format, try "$s_y"

The $sX and $sY variables are renamed to $original_width and $original_height, respectively.

Additionally, the $fwidth and $fheight variables are renamed to $full_width and $full_height, for clarity.

Follow-up to [11965], [22094], [56400].

See #58831.

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


5 weeks ago
#33

### PHPCS: improve organisation of the PHPCompatibility ruleset

No functional changes.

This commit:

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

### PHPCS: remove unnecessary directives in PHPCompatibility ruleset

This commit:

  • Removes the unnecessary exclusion patterns for the node_modules and the vendor directory.

As this ruleset only scans the src directory, those directories would never be scanned anyway.

  • Removes the selective excludes related to the Random Compat package.

This package was removed in WP 6.3, so these excludes are no longer necessary.

---

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

#34 @SergeyBiryukov
5 weeks ago

In 56420:

Coding Standards: Use strict comparison in wp-admin/includes/meta-boxes.php.

Follow-up to [38], [647], [2890], [3570], [11815], [11816], [31550], [52620].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#35 @SergeyBiryukov
4 weeks ago

In 56430:

Coding Standards: Use strict comparison in wp-includes/ms-files.php.

Follow-up to [12603], [12936], [56395].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#36 @SergeyBiryukov
4 weeks ago

In 56438:

Coding Standards: Use strict comparison in wp-includes/ms-site.php.

Follow-up to [12603], [43548], [43654], [44472].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#37 @SergeyBiryukov
4 weeks ago

In 56466:

Coding Standards: Use strict comparison in wp-includes/class-wp-widget.php.

Follow-up to [10764], [10912], [11427].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#38 @SergeyBiryukov
2 weeks ago

In 56511:

Coding Standards: Use strict comparison in wp-includes/class-wp-hook.php.

Follow-up to [4955], [38571].

Props aristath, poena, afercia, SergeyBiryukov.
See #58831.

#39 @SergeyBiryukov
2 weeks ago

In 56521:

Coding Standards: Correct equals sign alignment in various files.

This resolves a few WPCS warnings:

Equals sign not aligned with surrounding statements

so that the output of composer format is clean.

Follow-up to [56276], [56302].

Props jrf.
See #59161, #58831.

#40 @SergeyBiryukov
2 weeks ago

In 56536:

Coding Standards: Remove superfluous blank lines at the end of various files.

Note: This is enforced by WPCS 3.0.0.

Props jrf.
See #59161, #58831.

@viralsampat
2 weeks ago

I have checked above mentioned issue and founds another file. Here, I have added its patch.

#41 @SergeyBiryukov
2 weeks ago

In 56547:

Coding Standards: Remove superfluous blank lines at the end of various classes.

Note: This is enforced by WPCS 3.0.0.

Follow-up to [56536].

Props jrf.
See #59161, #58831.

#42 @SergeyBiryukov
2 weeks ago

In 56548:

Coding Standards: Remove superfluous blank lines at the end of various functions.

Note: This is enforced by WPCS 3.0.0.

Follow-up to [56536], [56547].

Props jrf.
See #59161, #58831.

#43 @SergeyBiryukov
12 days ago

In 56549:

Coding Standards: Use pre-increment/decrement for stand-alone statements.

Note: This is enforced by WPCS 3.0.0:

  1. There should be no space between an increment/decrement operator and the variable it applies to.
  2. Pre-increment/decrement should be favoured over post-increment/decrement for stand-alone statements. “Pre” will in/decrement and then return, “post” will return and then in/decrement. Using the “pre” version is slightly more performant and can prevent future bugs when code gets moved around.

References:

Props jrf.
See #59161, #58831.

#44 @SergeyBiryukov
11 days ago

In 56551:

Coding Standards: Correct spacing for spread operators.

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

Note: This is enforced by WPCS 3.0.0.

Follow-up to [46133].

Props jrf.
See #59161, #58831.

#45 @SergeyBiryukov
11 days ago

In 56552:

Code Modernization: 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.

Note: This is enforced by WPCS 3.0.0.

Reference: PHP Manual: dirname().

Follow-up to [56141].

Props jrf.
See #59161, #58831.

#46 @SergeyBiryukov
9 days ago

In 56559:

Coding Standards: Include one space after function keyword for closures.

Note: This is enforced by WPCS 3.0.0.

Reference: WPCS: PR #2328 Core: properly check formatting of function declaration statements.

Props jrf.
See #59161, #58831.

#47 @SergeyBiryukov
7 days ago

In 56586:

Coding Standards: Restore more descriptive variable names in a few class methods.

When various methods parameters in child classes were renamed to $item to match the parent class for PHP 8 named parameter support, most of the methods restored the more descriptive, specific name at the beginning for better readability, with several exceptions for methods consisting only of a few lines.

To avoid confusion about why some methods do that and some don't, this commit aims to bring more consistency to the code, specifically in list tables' ::column_default() methods.

Follow-up to [51728], [51737], [51786].

See #58831.

#48 follow-up: @jrf
7 days ago

@SergeyBiryukov Re commit [56586] - FYI: the proposed change in ticket #59232 should also help in making it clearer for the future that the signatures of those methods need to stay in sync with the signature of the parent method.

@viralsampat
7 days ago

I have added another patch

#49 in reply to: ↑ 48 ; follow-up: @SergeyBiryukov
7 days ago

Replying to jrf:

Re commit [56586] - FYI: the proposed change in ticket #59232 should also help in making it clearer for the future that the signatures of those methods need to stay in sync with the signature of the parent method.

Thanks! I have a draft PR there as well as part of coding sessions with @poena, @andrea, and @aristath :)

#50 in reply to: ↑ 49 @jrf
7 days ago

Replying to SergeyBiryukov:

Thanks! I have a draft PR there as well as part of coding sessions with @poena, @andrea, and @aristath :)

Awesome!

#51 @SergeyBiryukov
6 days ago

In 56596:

Coding Standards: Remove extra parentheses in a few str_contains() conditionals.

Follow-up to [55988].

Props Cybr.
See #58831.

#52 @SergeyBiryukov
5 days ago

In 56598:

Coding Standards: Fix a few newly introduced WPCS issues.

Follow-up to [56515], [56557], [56560].

Props jrf.
See #59161, #58831.

#53 @audrasjb
35 hours ago

In 56632:

Coding Standards: Add missing escaping functions in wp-admin/export.php

Props viralsampat.
See #58831.

#54 @SergeyBiryukov
34 hours ago

In 56633:

Coding Standards: Escape the whole attribute in wp-admin/export.php.

It is best to always escape the complete value of an attribute, not a partial value, as otherwise the escaping could be (partially) undone when the values are joined together.

While the hardcoded hyphen in this case don't necessarily create that risk, it may change to a value which could be problematic, so making it a habit to escape the value in one go is best practice.

Escaping the complete value also means that a single esc_attr() call can be used instead of two.

Follow-up to [14444], [16652], [55616], [56632].

See #58831.

Note: See TracTickets for help on using tickets.