Make WordPress Core

Opened 9 months ago

Closed 5 months ago

#59650 closed task (blessed) (fixed)

Coding Standards fixes for WP 6.5

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

Description

Previously:

Change History (26)

#1 @hellofromTonya
9 months ago

  • Summary changed from Coding Standards fixes for WP 6.4 to Coding Standards fixes for WP 6.5

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


9 months ago
#2

  • Keywords has-patch added

It is perfectly possible to write a commented regex with layout for readability by using the x modifier.

As per the manual:

x (PCRE_EXTENDED)

If this modifier is set, whitespace data characters in the pattern are totally ignored except when escaped or inside a character class, and characters between an unescaped # outside a character class and the next newline character, inclusive, are also ignored. This is equivalent to Perl's /x modifier, and makes it possible to include commentary inside complicated patterns.
Note, however, that this applies only to data characters. Whitespace characters may never appear within special character sequences in a pattern, for example within the sequence (?( which introduces a conditional subpattern.

Ref: https://www.php.net/manual/en/reference.pcre.pattern.modifiers.php

This commit rewrites this one regex to use the x modifier and gets rid of the unnecessary phpcs:disable comments.

The tests in the tests/phpunit/tests/kses.php file cover this change.

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

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


9 months ago
#3

It is perfectly possible to write a commented regex with layout for readability by using the x modifier.

As per the manual:

x (PCRE_EXTENDED)

If this modifier is set, whitespace data characters in the pattern are totally ignored except when escaped or inside a character class, and characters between an unescaped # outside a character class and the next newline character, inclusive, are also ignored. This is equivalent to Perl's /x modifier, and makes it possible to include commentary inside complicated patterns.
Note, however, that this applies only to data characters. Whitespace characters may never appear within special character sequences in a pattern, for example within the sequence (?( which introduces a conditional subpattern.

Ref: https://www.php.net/manual/en/reference.pcre.pattern.modifiers.php

This commit rewrites these two regexes to use the x modifier and gets rid of the unnecessary phpcs:disable comments.

The tests in the tests/phpunit/tests/db/dbDelta.php file cover this change.

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

#4 @jrf
9 months ago

I've opened two PRs on GitHub with patches for this ticket. Both rewrite a concatenated multi-line regex with comments to use the regex native "multi-line with comment" format.

This makes it more straight forward to update the regex, if needed, and gets rid of some unnecessary ignore annotations inline.

Both changes are already covered by pre-existing tests (and this has been verified by introducing a typo in the regex and seeing those tests fail - the typo has, of course been removed again after).

Note: the build failures are unrelated to these PRs, but related to the package.lock file not having been updated for trunk now being 6.5.

#5 @SergeyBiryukov
9 months ago

In 57053:

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 [56796], [56803], [56838], [56839], [56985].

See #59650.

#6 @SergeyBiryukov
9 months ago

In 57056:

Coding Standards: Remove unnecessary ignore annotation in wp_kses_hair_parse().

It is perfectly possible to write a commented regex with layout for readability by using the x modifier.

As per the manual:

x (PCRE_EXTENDED)

If this modifier is set, whitespace data characters in the pattern are totally ignored except when escaped or inside a character class, and characters between an unescaped # outside a character class and the next newline character, inclusive, are also ignored. This is equivalent to Perl's /x modifier, and makes it possible to include commentary inside complicated patterns.

Note, however, that this applies only to data characters. Whitespace characters may never appear within special character sequences in a pattern, for example within the sequence (?( which introduces a conditional subpattern.

Reference: PHP Manual: Pattern Modifiers.

This commit rewrites these two regexes to use the x modifier and gets rid of the unnecessary phpcs:disable comments.

The tests in the tests/phpunit/tests/db/dbDelta.php file cover this change.

Follow-up to [42249].

Props jrf.
See #59650.

@SergeyBiryukov commented on PR #5517:


9 months ago
#7

Thanks for the PR! Merged in r57056.

@jrf commented on PR #5517:


9 months ago
#8

Thanks again @SergeyBiryukov !

@jrf commented on PR #5517:


9 months ago
#9

@SergeyBiryukov Looks like the commit message is a bit borked - it references the changes from PR #5518, not the changes from this PR ?

#10 @SergeyBiryukov
9 months ago

In 57061:

Coding Standards: Remove unnecessary ignore annotations in dbDelta().

It is perfectly possible to write a commented regex with layout for readability by using the x modifier.

As per the manual:

x (PCRE_EXTENDED)

If this modifier is set, whitespace data characters in the pattern are totally ignored except when escaped or inside a character class, and characters between an unescaped # outside a character class and the next newline character, inclusive, are also ignored. This is equivalent to Perl's /x modifier, and makes it possible to include commentary inside complicated patterns.

Note, however, that this applies only to data characters. Whitespace characters may never appear within special character sequences in a pattern, for example within the sequence (?( which introduces a conditional subpattern.

Reference: PHP Manual: Pattern Modifiers.

This commit rewrites these two regexes to use the x modifier and gets rid of the unnecessary phpcs:disable comments.

The tests in the tests/phpunit/tests/db/dbDelta.php file cover this change.

Follow-up to [42249].

Props jrf.
See #59650.

@SergeyBiryukov commented on PR #5517:


9 months ago
#11

@SergeyBiryukov Looks like the commit message is a bit borked - it references the changes from PR #5518, not the changes from this PR ?

Yeah, I mixed up the descriptions of the two PRs, sorry for that! Commit messages cannot be edited afterwards. At least the first sentence is accurate 🙂

@SergeyBiryukov commented on PR #5518:


9 months ago
#12

Thanks for the PR! Merged in r57061.

@jrf commented on PR #5517:


9 months ago
#13

Looks like the commit message is a bit borked - it references the changes from PR #5518, not the changes from this PR ?

Yeah, I mixed up the descriptions of the two PRs, sorry for that! Commit messages cannot be edited afterwards. At least the first sentence is accurate 🙂

@SergeyBiryukov It happen ;-)

@jrf commented on PR #5518:


9 months ago
#14

Thanks @SergeyBiryukov !

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


9 months ago
#15

  • Keywords has-unit-tests added

#16 @peterwilsoncc
9 months ago

In 57100:

Coding Standards: Replace CRLF line breaks with LF.

Run npm run grunt format:php:error to correct the EOL sequence for:

  • tests/phpunit/tests/functions/wpAdminNotice.php
  • tests/phpunit/tests/functions/wpGetAdminNotice.php

See #59650.

#18 @SergeyBiryukov
8 months ago

In 57123:

Coding Standards: Rewrite a few capability checks for clarity and readability.

This aims to:

  • Perform the checks as early as possible to avoid redundant function calls.
  • Remove an empty conditiaonal branch and make the exit conditions clearer.
  • Bring the formatting in line with other multi-line conditionals in core.

Follow-up to [56836].

See #59650.

#19 @SergeyBiryukov
8 months ago

In 57126:

Code Modernization: Use str_starts_with() in WP_REST_Server::serve_request().

Follow-up to [55703], [56834].

See #59650.

#20 @SergeyBiryukov
8 months ago

In 57139:

Coding Standards: Reorder conditionals in is_random_header_image().

This aims to slightly improve performance by checking the faster empty() language construct first and potentially avoiding an unnecessary function call. Additionally, this better matches a similar conditional a few lines below.

Follow-up to [17757], [17770].

Props Cybr.
See #59650.

#21 @SergeyBiryukov
6 months ago

In 57530:

Coding Standards: Rename the $ID parameter to $post_id in trackback().

This resolves a few WPCS warnings:

Variable "$ID" is not in valid snake_case format, try "$i_d"

See #59650.

#22 @SergeyBiryukov
6 months ago

In 57532:

Coding Standards: Rename the $expires_offset variable in cache_javascript_headers().

This resolves a WPCS warning:

Variable "$expiresOffset" is not in valid snake_case format, try "$expires_offset"

Follow-up to [4109], [21996].

See #59650.

#23 @SergeyBiryukov
6 months ago

In 57538:

Coding Standards: Rename the $oSelf variable in WP_MatchesMapRegex::apply().

This resolves a WPCS warning:

Variable "$oSelf" is not in valid snake_case format, try "$o_self"

Follow-up to [11853], [38376].

See #59650.

#24 @SergeyBiryukov
5 months ago

In 57633:

Coding Standards: Allow $newlineEscape parameter in WP_Text_Diff_Renderer_inline::_splitOnWords().

This resolves a WPCS warning:

Variable "$newlineEscape" is not in valid snake_case format, try "$newline_escape"

The WP_Text_Diff_Renderer_inline class extends the Text_Diff_Renderer_inline class from the Text_Diff package and should have the same parameters as the parent class method, per the Task 1 section of ticket #51553.

Follow-up to [7747], [55163].

See #59650.

#25 @SergeyBiryukov
5 months ago

In 57694:

Coding Standards: Correct alignment in wp_get_attachment_image_src().

This resolves a WPCS warning:

Equals sign not aligned with surrounding statements

so that the output of composer format is clean.

Follow-up to [57687].

See #59650.

#26 @swissspidy
5 months ago

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

Closing for now since 6.5 has been branched, so any new updates like this can happen in trunk for 6.6.

Note: See TracTickets for help on using tickets.