Opened 3 months ago
Last modified 44 hours ago
#57839 new enhancement
Coding Standards fixes for WP 6.3
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 6.3 | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | has-patch has-unit-tests |
Focuses: | coding-standards | Cc: |
Change History (56)
This ticket was mentioned in PR #4294 on WordPress/wordpress-develop by @wpfy.
2 months ago
#2
- Keywords has-patch added
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
This ticket was mentioned in PR #4296 on WordPress/wordpress-develop by @faisalahammad.
2 months ago
#3
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 ticket was mentioned in PR #4299 on WordPress/wordpress-develop by @faisalahammad.
2 months ago
#4
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
@audrasjb commented on PR #4294:
2 months ago
#5
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.
This ticket was mentioned in PR #4302 on WordPress/wordpress-develop by @wpfy.
2 months ago
#6
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
@faisalahammad commented on PR #4299:
2 months ago
#7
Sorry @audrasjb
I was unable to revert the changes. Could you please ignore/delete this PR and I'm going to create another PR?
@audrasjb commented on PR #4299:
2 months ago
#8
of course :)
This ticket was mentioned in PR #4303 on WordPress/wordpress-develop by @faisalahammad.
2 months ago
#9
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 ticket was mentioned in PR #4304 on WordPress/wordpress-develop by @kausaralm.
2 months ago
#10
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 commented on PR #4302:
2 months ago
#14
Thanks for the PR! Merged in r55633.
@SergeyBiryukov commented on PR #4304:
2 months ago
#16
Thanks for the PR! Merged in r55641 as part of a few other similar changes.
This ticket was mentioned in PR #3873 on WordPress/wordpress-develop by @SergeyBiryukov.
2 months ago
#17
- Keywords has-unit-tests added
@SergeyBiryukov commented on PR #4296:
2 months ago
#19
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, soelseif
would not work there.
@SergeyBiryukov commented on PR #4303:
2 months ago
#20
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 inwp-admin/includes/theme.php
are from Backbone/Underscore.js templates and use JavaScript syntax, not PHP, soelseif
would not work there.
This ticket was mentioned in PR #4312 on WordPress/wordpress-develop by @faisalahammad.
2 months ago
#21
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
This ticket was mentioned in PR #4313 on WordPress/wordpress-develop by @sarequl.
2 months ago
#22
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
2 months ago
#24
Hi,
At a glance it looks like that all files edited in this PR comes from included libraries, and should be fixed upstream.
@SergeyBiryukov commented on PR #4312:
2 months ago
#26
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.
@SergeyBiryukov commented on PR #4313:
2 months ago
#27
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 commented on PR #4313:
2 weeks ago
#43
Merged in r55849.
In 55540: