Make WordPress Core

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: rockonshajib's profile rockonshajib 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

Trac ticket:

#2 @mukesh27
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 @audrasjb
2 years ago

  • Keywords commit removed

Removing commit for now, as there is still a remark to address in the PR.

#5 @costdev
2 years ago

  • Keywords commit added

PR 3878 implements the feedback. Adding for commit consideration.

#6 @johnbillion
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 @mukesh27
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 @adeltahri
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 @davidbaumwald
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.

Note: See TracTickets for help on using tickets.