Opened 2 years ago
Last modified 2 years ago
#57213 new enhancement
Coding standards : Use Strict Comparison except Loose Comparison wp-includes/class-wp-hook.php
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | General | Keywords: | has-patch needs-testing |
Focuses: | coding-standards | Cc: |
Description
It's help for coding standardization when get the Integers data type then use Strict Comparison except Loose Comparison
Change History (10)
This ticket was mentioned in PR #3690 on WordPress/wordpress-develop by RockonShajib3.
2 years ago
#1
- Keywords has-patch added
#2
@
2 years ago
- Keywords commit added
- Milestone changed from Awaiting Review to 6.2
- Version changed from 6.1.1 to 4.7
Hi there! Welcome to trac!
Thanks for the ticket and PR.
Moving to 6.2
milestone.
#3
@
2 years ago
- Keywords commit removed
Removing commit
for now, as there is still a remark to address in the PR.
This ticket was mentioned in PR #3878 on WordPress/wordpress-develop by @costdev.
2 years ago
#4
This PR updates https://github.com/WordPress/wordpress-develop/pull/3690 with an int
cast for BC.
Trac ticket: https://core.trac.wordpress.org/ticket/57213
#5
@
2 years ago
- Keywords commit added
PR 3878 implements the feedback. Adding for commit
consideration.
#6
@
2 years ago
- Keywords needs-testing added; commit removed
Is there a performance impact to this change? I doubt it but I'd like to see some numbers, possibly with PHPBench similar to #53218. We're allowed to nitpick and micro-optimise apply_filters()
and do_action()
because they're used so frequently :)
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
2 years ago
#8
@
2 years ago
Thanks @rockonshajib for the ticket.
This ticket was discussed in the recent bug scrub.
Can someone from the testing team (@adeltahri or @robinwpdeveloper) and performance team (@flixos90) take a look and share the feedback?
Props to @costdev
#9
@
2 years ago
Test Report:
There were no PHP errors on the frontend or backend of the general test.
Patch tested: 3878
Environment
- OS: macOS 13.0.1
- Web Server: Nginx
- PHP: 7.4.33
- WordPress: 6.2-alpha-54642
- Browser: Chrome 109
- Theme: Twenty Twenty-Two
- Active Plugins:
- Gutenberg
PHPUnit Test
I have added a test unit to applyFilters.php:
public function test_apply_filters_with_integer_argument() { $a = new MockAction(); $callback = array( $a, 'filter' ); $hook = new WP_Hook(); $hook_name = __FUNCTION__; $priority = 1; $accepted_args = 2; $arg = 0; $hook->add_filter( $hook_name, $callback, $priority, $accepted_args ); $returned = $hook->apply_filters( $arg, array( $arg ) ); $this->assertSame( $returned, $arg ); } public function test_apply_filters_with_string_argument() { $a = new MockAction(); $callback = array( $a, 'filter' ); $hook = new WP_Hook(); $hook_name = __FUNCTION__; $priority = 1; $accepted_args = 2; $arg = '0'; $hook->add_filter( $hook_name, $callback, $priority, $accepted_args ); $returned = $hook->apply_filters( $arg, array( $arg ) ); $this->assertSame( $returned, $arg ); }
- Test pass.
#10
@
2 years ago
- Milestone changed from 6.2 to Future Release
With 6.2 Beta 1 scheduled to be released tomorrow(2023-02-06) and @johnbillion's request for some performance data on this change, this ticket is being moved to Future Release
.
If any committer wishes to assume ownership of this ticket during a specific cycle, or if the remaining issues can be resolved in time, feel free to update the milestone accordingly.
Trac ticket: