Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#57887 closed defect (bug) (invalid)

The WordPress core does not support calling non-static methods for hook filters and actions within a namespace

Reported by: markcarbonell2013's profile markcarbonell2013 Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: General Keywords:
Focuses: rest-api Cc:

Description

Technical Data
Wordpress Version: 6.1.1
OS: Ubuntu 20.04
PHP Version: 8.0

Current Behavior:
Non-static methods cannot be called by the WordPress core when the class is namespaced. Only static methods can.

Steps to reproduce:

  1. Create a fresh wordpress installation
  2. Install the WP Code Snippets Plugin
  3. Create a new snippet that runs everywhere with the code written below.
  4. Observe how an error ocurrs the moment you save and activate the snippet.
namespace test;

class MyPlugin {
  public function __construct() {
    add_action('rest_api_init', [$this, 'do_something_static']);
    add_action('rest_api_init', [$this, 'do_something_non_static']);
  }

  public function do_something_non_static() {
    error_log("If you read this, calling " . __FUNCTION__ . " worked");
  }

  public static function do_something_static() {
    error_log("If you read this, calling " . __FUNCTION__ . " worked");
  }
}

new MyPlugin();

Expected behavior:
Both non-static and static methods can be called by WordPress regardless of whether the class/instance they belong to is namespaced or not.

Consequences:
This bug forces developers who structure their code professionally with namespaces to exclusively use static methods every time a new WordPress action or filter hook is added. Subsequently, every other class method used by those classes must be static too. Resulting in a class composed primarily of static methods. Thereby forcing developers to violate the Single Responsibility Principle of OOP https://medium.com/att-israel/should-you-avoid-using-static-ae4b58ca1de5#:~:text=Static%20methods%20are%20bad%20for,that%20simple%20for%20some%20languages

If the developer is unaware of the error, no logs are written to the log files and no error messages are displayed in the frontend. Making this error particularly hard to track.

Change History (9)

#1 @jrf
2 years ago

  • Keywords close added; changes-requested needs-refresh needs-patch needs-testing removed
  • Severity changed from blocker to normal

@markcarbonell2013 I haven't tested your code, but I know from experience that your statement that "only static methods are supported when using namespaced classes" just isn't true.

There are plenty of plugins out there - including in the official WP repository - which use namespaces and non-static methods and everything works perfectly fine.

There is one restriction, and that is that methods being used as callbacks need to be public, but that's a limitation in PHP, not WP.

Based on your code example, however, that doesn't seem to be the problem here.

To figure out what's going wrong with your plugin, I think you'll need to dig a little deeper as this report doesn't contain the right information to help you.

Also please note: if you are looking for support when building a plugin, the support forums are the channel to use. Trac is really only for bugs in WP Core and this is not one of them.

#2 @hellofromTonya
2 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed
  • Version 6.1.1 deleted

Hello @markcarbonell2013,

Welcome to WordPress Core's Trac!

WordPress does allow a static method to be registered as the callback for a hook as @jrf points out. A fully qualified class name is needed (rather than $this) (for example https://3v4l.org/JUdlA).

As this is not a Core bug, I'm closing this ticket.

If you need further help, the Support forums are available.

#3 follow-up: @TobiasBg
2 years ago

Hi!

Just for completeness: @hellofromTonya, I believe that @markcarbonell2013 has issues with non-static methods, but @jrf's remarks still apply.

@markcarbonell2013: It would indeed be helpful to see the full error message that is raised here, and, to rule out influences from the Code Snippets plugin, it would be good to test this directly in a custom plugin. This might already help you finding the problem.

#4 in reply to: ↑ 3 @hellofromTonya
2 years ago

Replying to TobiasBg:

Hi!

Just for completeness: @hellofromTonya, I believe that @markcarbonell2013 has issues with non-static methods, but @jrf's remarks still apply.

Well duh on my part. You're right @TobiasBg. Imagine me doing a facepalm right now lol

Apologies @markcarbonell2013 for my misunderstanding.

I do still think this report is best reported in the Support Forum. As @TobiasBg, providing additional information can help folks to help you.

#5 @knutsp
2 years ago

  • Focuses coding-standards removed
  • Keywords close removed

I actually tested the code in a simple test plugin. Result on error log:

[08-Mar-2023 17:48:49 UTC] If you read this, calling do_something_static worked
[08-Mar-2023 17:48:49 UTC] If you read this, calling do_something_non_static worked

#6 @markcarbonell2013
2 years ago

Hey @TobiasBg @hellofromTonya @jrf and @knutsp. @jrf you were correct. I meant calling non-static methods. Sorry for the confusion in the bug's title. I just tested the snippet myself and it does not reproduce the bug correctly. I corrected it to illustrate my point appropriately.

namespace test;

class MyPlugin {
  public function __construct() {
    add_action('foo', [__CLASS__ , 'do_something_static'], 10, 0);
    add_action('foo', [__CLASS__ , 'do_something_non_static'], 10, 0);
    add_action('foo', [$this, 'do_something_static'], 10, 0);
    add_action('foo', [$this, 'do_something_non_static'], 10, 0);
  }

  public function do_something_non_static() {
    error_log("If you read this, calling " . __FUNCTION__ . " worked");
  }

  public static function do_something_static() {
    error_log("If you read this, calling " . __FUNCTION__ . " worked");
  }
}

new MyPlugin();
do_action('foo');

When I run it in WP Snippets I get the error notification depicted below:


Don't Panic
The code snippet you are trying to save produced a fatal error on line 306:

Non-static method test\MyPlugin::do_something_non_static() should not be called statically
The previous version of the snippet is unchanged, and the rest of this site should be functioning normally as before.

Please use the back button in your browser to return to the previous page and try to fix the code error. If you prefer, you can close this page and discard the changes you just made. No changes will be made to this site.


In my error log I see:

[09-Mar-2023 11:54:23 UTC] PHP Deprecated: Non-static method test\MyPlugin::do_something_non_static() should not be called statically in /var/www/html/wp-includes/class-wp-hook.php on line 306
[09-Mar-2023 11:54:23 UTC] If you read this, calling do_something_non_static worked
[09-Mar-2023 11:54:23 UTC] If you read this, calling do_something_static worked
[09-Mar-2023 11:54:23 UTC] If you read this, calling do_something_non_static worked


The error is caused because calling [__CLASS__, 'do_something_non_static'] calls the method statically, even if the method is non-static. Is this expected behavior?

#7 @TobiasBg
2 years ago

Hi @markcarbonell2013,

thanks for the confirmation!

The deprecation warning is expected and correct here. Using [__CLASS__, 'do_something_non_static'] for a non-static method is incorrect. For non-static methods, you must use a reference to the object instance in the callback, like $this or a variable pointing to that object instance.
So, in your case, you can use these two variants, depending on what you prefer:

add_action('foo', [__CLASS__ , 'do_something_static'], 10, 0);
add_action('foo', [$this, 'do_something_non_static'], 10, 0);

#8 @markcarbonell2013
2 years ago

Hey @TobiasBg thanks for clarifying. I will do it that way in the future. Feel free to close this bug. Thanks you for the quick answer and the great support!

#9 @jrf
2 years ago

@markcarbonell2013 Just FYI: as @TobiasBg said, this error is expected. However, the error has nothing to do with WordPress itself. It is just how PHP works.

If you'd like to read up on this some more, please have a look here in the PHP manual: https://www.php.net/manual/en/language.oop5.static.php#language.oop5.static.methods

Note: See TracTickets for help on using tickets.