Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#53978 closed defect (bug) (invalid)

Non-static method 'get_default_primary_column_name' should not be called statically

Reported by: volodymyrkolesnykov's profile volodymyrkolesnykov Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.3
Component: Administration Keywords: dev-feedback
Focuses: administration, coding-standards Cc:

Description

WP_List_Table::get_primary_column_name() invokes get_default_primary_column_name() as a static method, although it is not static:

$default = $this->get_default_primary_column_name();

// If the primary column doesn't exist,
// fall back to the first non-checkbox column.
if ( ! isset( $columns[ $default ] ) ) {
	$default = self::get_default_primary_column_name();
}

Also, the second call to get_default_primary_column_name() probably does not make sense because it is going to return the same value as $default already has. And, unless I am missing something, the entire if clause can be dropped. If so, I am happy to create a PR.

Attachments (1)

55131.diff (575 bytes) - added by azouamauriac 3 years ago.

Download all attachments as: .zip

Change History (10)

#2 @SergeyBiryukov
3 years ago

  • Component changed from General to Administration

#3 follow-up: @SergeyBiryukov
3 years ago

  • Keywords reporter-feedback added
  • Version changed from 5.8 to 4.3

Hi there, welcome back to WordPress Trac! Thanks for the report.

This was introduced in [33266] / #32996 for WordPress 4.3, and further adjusted in [49192]. Changing the Version field accordingly.

Could you provide the steps to reproduce the issue on a clean install? That would be helpful to move the ticket forward.

#4 @volodymyrkolesnykov
3 years ago

  • Resolution set to invalid
  • Status changed from new to closed

Sorry, it turns out that was a false positive produced by Intelephense. I cannot reproduce the issue with any unit tests.

#5 @SergeyBiryukov
3 years ago

  • Keywords reporter-feedback removed
  • Milestone Awaiting Review deleted

Thanks for the follow-up!

#6 @SergeyBiryukov
3 years ago

#55131 was marked as a duplicate.

@azouamauriac
3 years ago

#7 in reply to: ↑ 3 @azouamauriac
3 years ago

  • Focuses coding-standards added
  • Keywords dev-feedback added
  • Resolution invalid deleted
  • Status changed from closed to reopened

Replying to SergeyBiryukov:

Hi there, welcome back to WordPress Trac! Thanks for the report.

This was introduced in [33266] / #32996 for WordPress 4.3, and further adjusted in [49192]. Changing the Version field accordingly.

Could you provide the steps to reproduce the issue on a clean install? That would be helpful to move the ticket forward.

Hello, it's not an issue properly say, it's a coding standardhttps://prnt.sc/26sw7eq, since the method get_default_primary_column_name is not static; I believe, it's a wrong practice to use it as it was static; and it may be create some confusions.

I think the good practice would be to call parent method(get_default_primary_column_name) from subclass instead to call it statically.

see #51559

Last edited 3 years ago by azouamauriac (previous) (diff)

#8 follow-up: @jrf
3 years ago

  • Resolution set to invalid
  • Status changed from reopened to closed

I think there is a misconception here - using self::, parent:: or static:: for a method call does NOT make it a static method call. Calling a method using these keywords is about scope resolution in a class hierarchy, not about whether a method is declared as static or not.

It would be a static call if the method was called like so: WP_List_Table::get_default_primary_column_name()

#9 in reply to: ↑ 8 @knutsp
3 years ago

Replying to jrf:

I think there is a misconception here - using self::, parent:: or static:: for a method call does NOT make it a static method call. Calling a method using these keywords is about scope resolution in a class hierarchy, not about whether a method is declared as static or not.

:: is the static call operator, so I would say it's a static call. It is, however, allowed to call a non-static method statically from inside the class scope, even in PHP 8.0 https://php.watch/versions/8.0/non-static-static-call-fatal-error.

See

It would be a static call if the method was called like so: WP_List_Table::get_default_primary_column_name()

Indeed.

Note: See TracTickets for help on using tickets.