WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 8 weeks ago

Last modified 8 weeks ago

#43627 closed defect (bug) (maybelater)

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

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

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 20 months ago.

Download all attachments as: .zip

Change History (10)

#1 @SergeyBiryukov
20 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() for consistency with WP_Scripts::do_item(), to avoid further confusion.

Last edited 20 months ago by SergeyBiryukov (previous) (diff)

@soulseekah
20 months ago

#2 @SergeyBiryukov
20 months ago

  • Keywords has-patch added

#3 @SergeyBiryukov
19 months ago

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

#4 @pento
13 months ago

  • Milestone changed from 5.0 to 5.1

#5 @pento
10 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.


7 months ago

#7 @audrasjb
7 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.

#8 @azaozz
8 weeks ago

  • Milestone 5.3 deleted
  • Resolution set to worksforme
  • Status changed from reviewing to closed

At some point it was possible (still is?) to use WP_Dependencies as a separate "package". As per comment 5, thinking it's not a good idea to change the signature of WP_Dependencies::do_item().

Closing as worksforme, please feel free to reopen with an investigation of possible side effects.

#9 @SergeyBiryukov
8 weeks ago

  • Resolution changed from worksforme to maybelater
Note: See TracTickets for help on using tickets.