Coding Standards fixes for WP 6.3

[hellofromTonya]

Previously:

• #56791 (6.2)
• #55647 (6.1)
• #54728 (6.0)
• #53359 (5.9)
• #52627 (5.8)
• #51799 (5.7)
• #50767 (5.6)
• #49542 (5.5)
• #49222 (5.4)
• #47632 (5.3)
• #45934 (5.1)

[SergeyBiryukov]

In 55540:

Coding Standards: Bring some consistency to wp_validate_redirect() existence checks.

The wp_get_referer() and wp_get_original_referer() functions both depend on wp_validate_redirect() and check whether it is defined by the time they run, but do so in a slightly different way.

This commit ensures both functions return early if they are called before wp_validate_redirect() is defined.

Follow-up to [3908], [25399], [25400].

See #57839.

The WordPress coding standards recommend using 'elseif' instead of 'else if' for conditionals to ensure consistency in the code syntax. This is because 'else if' is not compatible with the colon syntax for if|elseif blocks.

However, in the WordPress core file '/wp-includes/ID3/module.audio.ogg.php', on line 608, 'else if' is used for conditionals. To ensure consistency with the WordPress coding standards and improve the readability and maintainability of the code, I propose replacing 'else if' with 'elseif' on this line.

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

The WordPress coding standards recommend adding spaces before and after function arguments to improve code readability and consistency. However, in the wp-includes/Text/Diff/Renderer/inline.php file, some functions do not follow this standard.

To improve the code quality and consistency, I have updated the affected functions by adding spaces before and after their arguments.

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

This pull request fixes the issue of using "else if" instead of "elseif" in the WordPress coding standard. According to the standard, "elseif" should be used instead of "else if" because the latter is not compatible with the colon syntax for if|elseif blocks.

I have identified wp-admin/includes/theme.php where this issue exists and have made the necessary changes to replace all occurrences of "else if" with "elseif".

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

Thanks for the PR @akramulhasan.

However, this is an actively maintained upstream library, so this has to be fixed in the related upstream repository, not in WordPress core.

Here is the lib: ​https://github.com/JamesHeinrich/getID3

Closing this PR for now.

According to the WordPress PHP coding standards, it is recommended to use require[_once] instead of include[_once] for unconditional includes. However, in two files of the twenty twenty one theme, I have found include_once used unconditionally.

To improve consistency and ensure better security and stability of the code, I propose replacing include_once with require_once in these two files.

This change will ensure that if the file cannot be found, a Fatal Error will be thrown, preventing any potential security leaks or other issues that may arise due to the file not being loaded properly.

I have created a pull request for these changes, and I kindly request a review from the WordPress core team.

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

Sorry @audrasjb
I was unable to revert the changes. Could you please ignore/delete this PR and I'm going to create another PR?

of course :)

This pull request fixes the issue of using "else if" instead of "elseif" in the WordPress coding standard. According to the standard, "elseif" should be used instead of "else if" because the latter is not compatible with the colon syntax for if|elseif blocks.

I have identified wp-admin/includes/theme.php where this issue exists and have made the necessary changes to replace all occurrences of "else if" with "elseif".

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

This pull request replace an unconditional include_once statement found in the file '/wp-admin/includes/class-wp-plugin-install-list-table.php' at line number 91, which violates the WordPress PHP coding standards.

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

[SergeyBiryukov]

In 55630:

I18N: Correct translator comments placement in wp-admin/edit-comments.php.

This ensures that the translator comments are displayed for the strings they refer to, e.g. to explain placeholders in comment action messages, and are not displayed for unrelated strings, e.g. "Undo", to avoid confusion.

Includes minor code layout fixes and wrapping a few long lines for better readability.

Follow-up to [35549], [45926], [45932].

Props afercia.
See #57839.

[SergeyBiryukov]

In 55631:

Coding Standards: Correct sprintf() calls for messages in wp-admin/upload.php.

This ensures that sprintf() is not unnecessarily applied to strings without any placeholders.

Follow-up to [31941], [31619], [36328], [55178].

See #57839.

[SergeyBiryukov]

In 55633:

Twenty Twenty-One: Replace include_once with require_once for required classes.

Per ​WordPress PHP coding standards, it is strongly recommended to use require[_once] for unconditional includes. When using include[_once], PHP will throw a warning when the file is not found but will continue execution, which will almost certainly lead to other errors/warnings/notices being thrown if your application depends on the file loaded, potentially leading to security leaks. For that reason, require[_once] is generally the better choice as it will throw a Fatal Error if the file cannot be found.

Follow-up to [49216], [49633].

Props wpfy.
See #57839.

Thanks for the PR! Merged in r55633.

[SergeyBiryukov]

In 55641:

Coding Standards: Replace include_once with require_once for required files.

Per ​WordPress PHP coding standards, it is strongly recommended to use require[_once] for unconditional includes. When using include[_once], PHP will throw a warning when the file is not found but will continue execution, which will almost certainly lead to other errors/warnings/notices being thrown if your application depends on the file loaded, potentially leading to security leaks. For that reason, require[_once] is generally the better choice as it will throw a Fatal Error if the file cannot be found.

Follow-up to [1674], [1812], [1964], [6779], [8540], [10521], [11005], [11911], [16065], [16149], [25421], [25466], [25823], [37714], [42981], [45448], [47198], [54276], [55633].

Props kausaralm, SergeyBiryukov.
See #57839.

Thanks for the PR! Merged in r55641 as part of a few other similar changes.

Trac ticket:
https://core.trac.wordpress.org/ticket/57839
https://core.trac.wordpress.org/ticket/56791

[SergeyBiryukov]

In 55642:

Coding Standards: Use strict comparison where count() is involved.

Follow-up to [1636], [6974], [8114], [10322], [13326], [14760], [18006], [18541], [19743], [23249], [24115], [33359].

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

Thanks for the PR! However:

• Text/Diff is an external library that has only been patched on rare occasions, and is excluded from coding standards checks.
• The <# } else if ( .. ) { #> fragments in widget classes are from Backbone/Underscore.js templates and use JavaScript syntax, not PHP, so elseif would not work there.

Thanks for the PR! However, please note that:

• class-pclzip.php is an external library that has only been patched on rare occasions, and is excluded from coding standards checks.
• The <# } else if ( .. ) { #> fragments in wp-admin/includes/theme.php are from Backbone/Underscore.js templates and use JavaScript syntax, not PHP, so elseif would not work there.

This pull request fixes the issue of using "else if" instead of "elseif" in the WordPress coding standard. According to the standard, "elseif" should be used instead of "else if" because the latter is not compatible with the colon syntax for if|elseif blocks.

I have identified in a couple of files where this issue exists and have made the necessary changes to replace all occurrences of "else if" with "elseif".

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

The issue with the code in /wp-includes/theme-compat/comments.php line 35 is that the comparison operator "==" is being used to compare the response code returned by get_comments_number() with the integer value 1. This is a non-strict comparison operator, which means that it will also return true if the response code is a string "1". This can lead to unexpected behavior or security vulnerabilities.

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

[SergeyBiryukov]

In 55645:

Coding Standards: Use strict comparison in wp-admin/maint/repair.php.

Follow-up to [12092], [19757].

Props azouamauriac, aristath, poena, afercia, SergeyBiryukov.
See #57839.

Hi,
At a glance it looks like that all files edited in this PR comes from included libraries, and should be fixed upstream.

[SergeyBiryukov]

In 55648:

Coding Standards: Rename the $qtInit and $mceInit variables in _WP_Editors.

This resolves 37 WPCS warnings along the lines of:

Variable "$qtInit" is not in valid snake_case format, try "$qt_init"
Variable "$mceInit" is not in valid snake_case format, try "$mce_init"

Follow-up to [18498].

See #57839.

Thanks for the PR!

As noted above, these are external libraries, and they are excluded from coding standards checks:

• ID3 and SimplePie have active upstream repositories, but they don't really have to follow the WordPress coding standards, so these changes might not be accepted upstream.
• IXR and AtomLib can be considered "adopted", but they have only been patched on rare occasions in the past, and don't have to follow the WordPress coding standards either.

Hi there, thanks for the PR!

This is a non-strict comparison operator, which means that it will also return true if the response code is a string "1".

Please note that get_comments_number() does in fact return the number of comments as a numeric string, not an integer (unless the post does not exist), so 1 === get_comments_number() won't work as expected.

The correct comparison would be '1' === get_comments_number(), see r55420 for example.

[SergeyBiryukov]

In 55650:

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

Follow-up to [17914], [55642].

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

[SergeyBiryukov]

In 55652:

Coding Standards: Use strict comparison where strlen() is involved.

Follow-up to [649], [1345], [3034], [6132], [6314], [6974], [55642].

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

[SergeyBiryukov]

In 55653:

Coding Standards: Use strict comparison where substr() is involved.

Follow-up to [3606], [10738], [33359], [55642], [55652].

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

[SergeyBiryukov]

In 55654:

Coding Standards: Use strict comparison where trim() is involved.

Follow-up to [17189], [24623], [55642], [55652], [55653].

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

[ugyensupport]

@markparnell Yes, as you mentioned in #58141

[SergeyBiryukov]

In 55660:

Coding Standards: Use more meaningful variable names in various comment functions.

This aims to bring more consistency between the functions, as well as in filter parameters.

Includes minor code layout fixes for better readability.

Follow-up to [45667], [47287], [48579], [53719], [53723], [55308].

See #57839.

[SergeyBiryukov]

In 55661:

Tests: Correct the expected quotes in get_comment_author_url_link() tests.

Follow-up to [55660].

See #57839.

[SergeyBiryukov]

In 55676:

Coding Standards: Remove unused $key variable in wp-admin/network/sites.php.

Rename $val to $site_id for clarity.

Follow-up to [12603], [18562], [30020].

See #57839.

[SergeyBiryukov]

In 55677:

Coding Standards: Use strict comparison where strtolower() is involved.

Follow-up to [649], [7736], [18821], [19444], [20886], [20893], [23303], [55642], [55652], [55653], [55654].

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

[SergeyBiryukov]

In 55678:

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

Follow-up to [32994], [35299].

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

[SergeyBiryukov]

In 55683:

Coding Standards: Update code layout in default_password_nag() for readability.

This aims to better match similar fragments in other core functions.

Follow-up to [11162], [13844], [14170], [55682].

See #57839.

[SergeyBiryukov]

In 55697:

Coding Standards: Bring some consistency to REST API revisions initialization.

The autosaves and revisions controllers used to set the same class properties in a slightly different order.

This commit makes the ::__construct() methods of both classes more consistent to simplify future maintenance.

Follow-up to [46272], [51962].

See #57839.

[SergeyBiryukov]

In 55726:

Coding Standards: Bring more consistency to PHP 8.0 string function polyfills.

This adjusts str_contains() code layout to have an early exit for an empty $needle, matching similar fragments in str_starts_with() and str_ends_with() for better readability.

Follow-up to [52039], [52040].

See #57839.

[audrasjb]

In 55751:

Coding Standards: Use esc_url() to escape link URL value in wp-admin/edit-link-form.php.

Props dilipbheda, mukesh27.
Fixes #58282.
See #57839.

[SergeyBiryukov]

In 55849:

Coding Standards: Use strict comparison in wp-includes/theme-compat/comments.php.

Follow-up to [31941], [41285], [55420].

Props sarequl.
See #57839.

Merged in r55849.

[SergeyBiryukov]

In 55853:

Help/About: Reverse the order of conditionals for displaying the "Go to Updates" link.

isset() is faster than current_user_can(), so should be checked first.

Includes updating code layout for better readability.

Follow-up to [19442], [19524], [28477], [33466], [55848].

See #57839.

[SergeyBiryukov]

In 55866:

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

Follow-up to [12756], [12862], [12880], [13070], [14485], [17928], [18899], [41348], [43628].

Props faisalahammad, aristath, poena, afercia, SergeyBiryukov.
Fixes #58042, #58047. See #57839.

[SergeyBiryukov]

In 55873:

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

Includes minor code layout fixes for better readability.

Follow-up to [3536], [4093], [9578], [11092], [18034].

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

[SergeyBiryukov]

In 55874:

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

Includes minor code layout fixes for better readability.

Follow-up to [9441], [25540].

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

[SergeyBiryukov]

In 55876:

Coding Standards: Use strict comparison in wp-admin/network/sites.php.

Follow-up to [12603], [18562], [55676].

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

[SergeyBiryukov]

In 55877:

Coding Standards: Use strict comparison in wp-admin/network/users.php.

Follow-up to [12603], [18562].

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

[SergeyBiryukov]

In 55878:

Coding Standards: Use strict comparison in wp-admin/users.php.

Includes minor code layout fixes for better readability.

Follow-up to [3061], [31941], [47848], [55622], [55623].

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

[SergeyBiryukov]

In 55880:

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

Follow-up to [3511], [4173], [5525].

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

[costdev]

Related: #58459

[SergeyBiryukov]

In 55883:

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

Follow-up to [3033], [3704], [4576], [4580], [5530], [15847], [36008].

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

[SergeyBiryukov]

In 55886:

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

Follow-up to [12424], [25224], [25303], [29852], [45667], [47808], [51825].

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

[SergeyBiryukov]

In 55904:

Coding Standards: Use strict comparison in wp-admin/options-writing.php.

Follow-up to [4326], [19304].

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

[SergeyBiryukov]

In 55905:

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

Follow-up to [3704], [16100].

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

[SergeyBiryukov]

In 55906:

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

Follow-up to [3704], [9299], [9485], [16100], [44416].

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

[SergeyBiryukov]

In 55908:

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

Follow-up to [18606], [38928], [48413].

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

[SergeyBiryukov]

In 55914:

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

Follow-up to [13996].

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

[jeremyfelt]

Just a heads up: The ​strict comparison added to WP_List_Util::filter via [55908] broke a test on a project I maintain.

// $created contains objects with maybe property disabled === '0'
$active = wp_filter_object_list( $created, [ 'disabled' => 0 ] );

I like the change to strict comparison, so no complaint here. But, I'm not sure how much 3rd party code there may be that accidentally relies on this being loose. :)

[SergeyBiryukov]

In 55927:

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

Follow-up to [18528].

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

[SergeyBiryukov]

In 55931:

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

Follow-up to [10584], [12174].

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

[SergeyBiryukov]

In 55934:

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

Follow-up to [10584], [11626], [12174].

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

[SergeyBiryukov]

In 55948:

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

Follow-up to [10584], [12079].

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

[david.binda]

Having the r55908 deployed on the WordPress.com revealed that the change may easily affect any code which has been (unintentionally) relying on type juggling when using the wp_list_filter function (in our case, it's been a string vs. int type juggling which got broken with the strict comparison).

It's a similar thing to the one reported above in https://core.trac.wordpress.org/ticket/57839#comment:63

I do understand the benefits of the strict comparison, but wanted to report the issue, since it's hard to spot and may easily break things in existing code in plugins/themes which is taking advantage of those handy functions.

[azaozz]

@SergeyBiryukov Yea, tseems [55908] introduced a regression in WP_List_Util::filter(). That function was doing loose comparison (no documentation about the types too), and now the way it works has changed.

Wondering if this change "can get away with" casting to strings before comparing. Seems int is also a valid key and/or value type there too.

The same is probably true for the strict comparison in WP_List_Util::sort_callback(). May miss things when comparing strings to int.

[SergeyBiryukov]

In 56137:

General: Compare values as strings in WP_List_Util::filter() and ::sort_callback().

This aims to preserve backward compatibility for code relying on type juggling when using the wp_list_filter() function, e.g. comparing a numeric string to an integer.

Follow-up to [55908].

Props azaozz, jeremyfelt, david.binda.
See #57839.

[SergeyBiryukov]

In 56138:

General: Revert strict comparison in WP_List_Util for now.

Casting the values to a string causes Array to string conversion PHP warnings, so using loose comparison restores the previous behavior for code relying on type juggling when using the wp_list_filter() function, e.g. comparing a numeric string to an integer.

The unit test case added for this scenario in the previous commit remains.

Follow-up to [55908], [56137].

See #57839.

[SergeyBiryukov]

Replying to jeremyfelt:

Just a heads up: The ​strict comparison added to WP_List_Util::filter via [55908] broke a test on a project I maintain.

// $created contains objects with maybe property disabled === '0'
$active = wp_filter_object_list( $created, [ 'disabled' => 0 ] );

Replying to david.binda:

Having the r55908 deployed on the WordPress.com revealed that the change may easily affect any code which has been (unintentionally) relying on type juggling when using the wp_list_filter function (in our case, it's been a string vs. int type juggling which got broken with the strict comparison).

Good catch, thanks!

Replying to azaozz:

Yea, seems [55908] introduced a regression in WP_List_Util::filter(). That function was doing loose comparison (no documentation about the types too), and now the way it works has changed.

Wondering if this change "can get away with" casting to strings before comparing. Seems int is also a valid key and/or value type there too.

Seems reasonable, we already do that in a few other places, e.g. __checked_selected_helper(). But [56137] caused a few Array to string conversion warnings in wp_filter_object_list() tests, so restoring the loose comparison appears to be a safer option at this point.

Should be fixed in [56138] :)

[SergeyBiryukov]

In 56158:

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

Follow-up to [10687].

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

[SergeyBiryukov]

In 56166:

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

Follow-up to [16413], [16849].

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

[SergeyBiryukov]

In 56169:

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

Follow-up to [31669], [47808].

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

[SergeyBiryukov]

In 56184:

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

Follow-up to [15919], [21866], [41624].

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

[SergeyBiryukov]

In 56208:

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

Follow-up to [6614], [13689], [19743].

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

[SergeyBiryukov]

In 56210:

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

Follow-up to [3638], [13689], [19743].

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

[SergeyBiryukov]

In 56219:

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

Follow-up to [41380], [41861], [45910].

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

[SergeyBiryukov]

In 56238:

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

Includes minor code layout fixes for better readability.

Follow-up to [6058], [6208], [10217], [12732], [37677].

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

[audrasjb]

As we are now entering the RC cycle of WP 6.3, let's close this umbrella ticket as fixed.
Further coding standard fixes can ship in #58831.
Thanks!

[SergeyBiryukov]

In 56260:

Coding Standards: Use strict comparison for static strings in wp-admin/includes/class-wp-list-table.php.

Follow-up to [55971], [56004].

See #32170, #57839.

[SergeyBiryukov]

In 56261:

Administration: Add a missing closing </span> tag for column sorting indicators.

Includes wrapping a few other long markup lines for better readability.

Follow-up to [55971], [56004], [56260].

See #32170, #57839.

[hellofromTonya]

#58831 is open for the 6.4 cycle.

This will need tests before it can be merged:

Indeed, this is basically still a draft where I cherry-pick certain changes after a closer review, making sure they are covered by new or existing tests. I will investigate anything related to wp_parse_args() and WP_Query, thanks!