php-coding-standards icon indicating copy to clipboard operation
php-coding-standards copied to clipboard

Warning or Error when PHP_INT_MAX or PHP_INT_MIN is used in add_action or add_filter

Open luislard opened this issue 4 years ago • 1 comments
trafficstars

Is your feature request related to a problem? Please describe. This is more a question:

Would it be a good idea to throw an error or a warning when the priority argument of add_action and add_filter is PHP_INT_MAX or PHP_INT_MIN?

Describe the solution you'd like phpcs throwing a warning or an error.

Describe alternatives you've considered

Additional context Someone wrote "I do not like the usage of PHP_INT_MAX for the hook order, because it is not possible to remove them with another plugin. Yes, maybe not relevant in this case of custom plugin for a customer, but see it too often and have no chance to remove them silently."

Also wrote "But good style in coding is not always a topic of rules and standards, more a process of learning. The value for php int max is always different and is dependent from the client, from the machine."

luislard avatar May 06 '21 09:05 luislard

PHP_INT_MAX

I do not like the usage of PHP_INT_MAX for the hook order, because it is not possible to remove them with another plugin

Why not? Actually, having PHP_INT_MAX ensures you can remove the hook, because that's the latest possible hook.

In a plugin:

add_action('foo', 'bar', PHP_INT_MAX);

In another plugin:

remove_action('foo', 'bar', PHP_INT_MAX);

it works :) But, there's and issue: the remove_action needs to happen after the add_action. How do you ensures that? Sometimes that's easy sometimes not. Something that works pretty much always, is to use the same hook to remove the call, but with a earlier priority:

add_action('foo', fn() => remove_action('foo', 'bar', PHP_INT_MAX));

Thanks to the fact that PHP_INT_MAX is very late, having something that runs before was pretty easy, and did not required a custom priority, considering the default 10 is much lower than PHP_INT_MAX.

Besides that, removing an action always requires the exact same priority used to add the hook. if the hook is added with priority 32, the removal has to use the 32 as well. And it isn't better to use a constant instead of an hardcoed value like 999 often used when people whant a later hook?

Regarding the fact that PHP_INT_MAX depends on the PHP installation: that is totally unrelevant. The PHP binary that will process the add_action will always be the same PHP binary that process the remove_action, so in every given context the value of PHP_INT_MAX is the same. (So, for example, if both add_action and remove_action uses PHP_INT_MAX that will always work). And if the actual value in my machine is different than in your machine... how is that a problem?

The only issue with PHP_INT_MAX is not that is not possible to remove hook that uses it, but that is not possible to execute something after that given callaback for the same hook. That is especially relevant for filters. For actions it is possible to use a later action to solve the problem. But imagine something like:

add_filter('the_title', 'edit_title', `PHP_INT_MAX`);

It will never be possible to reliably filter the title with something different, because the filter above will run as last one.

Even if there're cases in which that make sense, those are usually when the developer is aware of the whole system (website), single themes and plugins should always offer a way to change the behavior, considering the wider context is not known.

So I guess that a warning when PHP_INT_MAX is used in add_filter could make sense.

PHP_INT_MIN

What said above for PHP_INT_MAX does not apply to PHP_INT_MIN. An hook added with PHP_INT_MIN could actually be hard to remove, because the trick of using the same hook won't work. E.g.:

add_action('foo', fn() => remove_action('foo', 'bar', PHP_INT_MIN));

There's no way to remove an hook"foo" using the same "foo" hook, if PHP_INT_MIN was used as priority. Which means that we need to ensure in some other way that remove_action is called after add_action, and that might not always be possible.

SUMMARY

IMO we could:

  • Raise a warning if PHP_INT_MAX is used for add_filter
  • Raise a warning if PHP_INT_MIN is used for add_action and add_filter

gmazzap avatar May 06 '21 10:05 gmazzap

I started to work on this, I ahve a part of the sniff done but nothing to commit yet. I have no time to complete it today.

antonioeatgoat avatar Sep 29 '23 15:09 antonioeatgoat

I am going to close this. It has been implemented in https://github.com/inpsyde/php-coding-standards/pull/75.

antonioeatgoat avatar Oct 27 '23 08:10 antonioeatgoat