WordPress.org

Make WordPress Core

Opened 16 months ago

Last modified 3 months ago

#43627 reviewing defect (bug)

Method call uses 2 parameters, but method signature uses 1 parameters

Reported by: kaggdesign Owned by: SergeyBiryukov
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.9.4
Component: Script Loader Keywords: has-patch dev-feedback early
Focuses: Cc:

Description

wp-includes/class.wp-dependencies.php, line 106

Method do_item() called with 2 parameters:

				if ( $this->do_item( $handle, $group ) ) {
					$this->done[] = $handle;
				}

Method signature has 1 parameter (same file, line 125)

	public function do_item( $handle ) {
		return isset( $this->registered[ $handle ] );
	}

Even taking into consideration further inheritance, second parameter $group should be added to method signature, to avoid code misunderstanding, and Code Smell warning in PhpStorm.

Attachments (1)

43627.diff (1.5 KB) - added by soulseekah 16 months ago.

Download all attachments as: .zip

Change History (8)

#1 @SergeyBiryukov
16 months ago

  • Component changed from General to Script Loader
  • Milestone changed from Awaiting Review to 5.0

Previously: #24770

This is simple class inheritance. The do_item() method in class.wp-dependencies.php is a placeholder meant to be overwritten when extending the class. When handling scripts WP_Dependencies is extended with WP_Scripts. So $this->do_item() executes the do_item() method of WP_Scripts that expects the second argument. (The WP_Dependencies::do_item() is still used in WP_Scripts but note how it's called with parent::do_item($handle).)

#24770 was not about the signature though, but rather about a missing wp_footer() call in a theme.

Let's use this ticket to add $group parameter to WP_Dependencies::do_item() to avoid further confusion.

Version 0, edited 16 months ago by SergeyBiryukov (next)

@soulseekah
16 months ago

#2 @SergeyBiryukov
16 months ago

  • Keywords has-patch added

#3 @SergeyBiryukov
15 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#4 @pento
9 months ago

  • Milestone changed from 5.0 to 5.1

#5 @pento
6 months ago

  • Keywords dev-feedback added
  • Milestone changed from 5.1 to 5.2

This change needs review, particularly considering side effects of changing the signature of WP_Dependencies::do_item().

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 months ago

#7 @audrasjb
3 months ago

  • Keywords early added
  • Milestone changed from 5.2 to 5.3

As per today's bug scrub, we are going to punt this one to the next release since beta 3 and RC are approaching.
Adding early keyword so it can be tested early in the release cycle.

Note: See TracTickets for help on using tickets.