[wp-trac] [WordPress Trac] #51525: Add new functions apply_filters_single_type() and apply_filters_ref_array_single_type()

[WordPress Trac]

#51525: Add new functions apply_filters_single_type() and
apply_filters_ref_array_single_type()
-------------------------------------------------+-------------------------
 Reporter:  jrf                                  |       Owner:  (none)
     Type:  feature request                      |      Status:  new
 Priority:  normal                               |   Milestone:  Future
                                                 |  Release
Component:  General                              |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  php8.x early has-patch has-unit-     |     Focuses:
  tests close                                    |
-------------------------------------------------+-------------------------
Changes (by johnbillion):

 * keywords:  2nd-opinion php8.x early has-patch has-unit-tests => php8.x
     early has-patch has-unit-tests close


Comment:

 I'm a big fan of type strictness and type safety. I've spent years fixing
 docblocks in WordPress core so their types are more correct, I've
 contributed extensively to phpstan-wordpress, I've written libraries that
 ensure the correctness of argument types passed to WordPress functions,
 and I would love to add `declare(strict_types=1)` to files in core and get
 rid of type coercion.

 All that said, I think this ticket should be closed. I think it's trying
 to solve a problem that in practice is not as widespread as was originally
 expected, and which is not solved by a version of `apply_filters()` that
 enforces only a single primitive type. If a filter passes and expects a
 value of only one type, and a plugin filters it and returns a different
 type, then either the plugin is entirely broken or the resulting type
 coercion is actually not causing a problem.

 [https://wordpress.org/plugins/user-switching/ My User Switching plugin]
 uses type declarations on several of its methods which are hooked into
 filters such as `user_has_cap` and `map_meta_cap` and has done so for the
 last 11 years. In that time I've had around three or four reports of
 breakage where another plugin returns an incorrect type from its own
 callback on one of those filters and User Switching gets the blame, and
 more than one was due to a filter returning no value at all and therefore
 being entirely broken. I don't recall it taking long to debug any of the
 cases. Granted this is my own personal experience from a moderately
 popular plugin, but it's one data point more than the theoretical problem
 posed by this ticket.

 [https://wordpress.org/about/stats/ Over 55% of WordPress sites run PHP
 8]. I don't know how reliable the result counts are for searches on
 wordpress.org but it seems there's around 60 reports of `"argument must be
 of type"` in the support forums across all of 2024, which is somewhere in
 the region of one report per week across 70,000 plugins and themes. It
 doesn't seem like there is a widespread problem.

 Ok so why not just add the proposed function to core anyway, and make use
 of it instead of opposing it? I can think of a few reasons:

 1. If a plugin returns an incorrect type from a filter and that type
 cannot be coerced, it'll trigger a PHP warning or an error. The proposed
 solution actually ''downgrades'' that to a notice via the
 `_doing_it_wrong()` system, making it less likely to be seen. Sometimes
 it's correct to allow some code to trigger a straight forward PHP warning
 rather than dance around it in userland code.
 2. If a plugin returns an incorrect type from a filter and that type is
 successfully coerced rather than triggering a warning or error, then
 perhaps it's not a problem.
 3. The type checking inside `apply_filters_typed()` does have a
 performance cost, however small. The `apply_filters()` function has been
 subject to two decades of continuous performance improvements to squeeze
 out every last millisecond. I don't think a performance degradation is
 acceptable.
 4. The error message in the proposed function is no more helpful than the
 default message reported by PHP because it still doesn't identify the
 culprit, which is one of the main points made in the description of this
 ticket. In order to make it useful would require some processing of the
 callback to construct a name for it, reducing performance further.
 5. In this whole thread not one concrete example of the problem in the
 real world has been given.

 When this ticket was opened four years ago it certainly looked like we
 might be heading toward a problem with the increased type strictness of
 PHP 8. In practice I don't believe this has materialised, and if it has
 then it's died down already.

 Recommending that this ticket is closed.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/51525#comment:37>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform