Opened 12 months ago
Closed 10 months ago
#20776 closed enhancement (wontfix)
Improve callback verification while executing hooks
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | Plugins | Version: | |
| Severity: | normal | Keywords: | needs-patch |
| Cc: |
Description
When executing hooks, we currently have a simple callback verification:
if ( !is_null($the_['function']) ) call_user_func_array($the_['function'], array_slice($args, 0, (int) $the_['accepted_args']));
This works in most cases, if everything is coded properly in the theme / plugins. However, there are cases when a bad callback makes it through this validation, such as dropping this in a theme or plugin:
add_action('init', array( NULL, 'method') );
Will cause an error you may be familiar with:
Warning: call_user_func_array() [function.call-user-func-array]: First argument is expected to be a valid callback, 'Array' was given in /path/to/wp/wp-includes/plugin.php on line 405.
Which can be a tricky issue to debug.
I suggest using the is_callable() php function for verification instead, which should remove this warning from ever showing up.
Attachments (2)
Change History (8)
I disagree with this. One of the best things we can do here is offer immediate and loud feedback to a developer in these cases. This is a developer error, not a user error. Issuing a warning should raise enough alarms without actually terminating execution flow.
We've all made a typo in a callback, only to spend quite a bit of time figuring out what was going wrong. It's worse when the issue is not reflected in the logs.
I'm actually not sure why the NULL check is there. I would rather remove it for both meager performance reasons and developer feedback reasons.
The check for NULL was done in [1394].
You can see the full reasoning here: http://wordpress.org/support/topic/bug-in-remove_filterapply_filters?replies=3.
As $wp_filter's structure has changed over the years, so has the need for this is_null() check. In [6320], some code went into remove_filter() which caused empty priority arrays to be unset(). I can no longer reproduce the original issue that caused [1394].
I would not be surprised if we could remove a few other checks as well, such as the array cast on the foreach's. I did not investigate that, however.
Yeah, after thinking about it, I agree. No validation at all would be better to create the proper PHP errors, while silent invalidation could lead to even more hard to track issues.
It looks like the array cast on $wp_filter[$tag] came from #4394, marked as code clean-up. I'd imagine that's good for taking out as well.
- Keywords needs-patch added; has-patch removed
+1 for removing the is_null() check.
remove is_null check from apply_filters, apply_filters_ref_array, do_action, do_action_ref_array, _wp_call_all_hook. These functions can use more dev-review for possible performance gains (remove array casting suggested).

replace is_null with is_callable