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

[WordPress Trac]

#51525: Add new functions apply_filters_typesafe() and
apply_filters_ref_array_typesafe()
------------------------------------------+---------------------
 Reporter:  jrf                           |       Owner:  (none)
     Type:  feature request               |      Status:  new
 Priority:  normal                        |   Milestone:  5.6
Component:  General                       |     Version:  trunk
 Severity:  normal                        |  Resolution:
 Keywords:  2nd-opinion needs-patch php8  |     Focuses:
------------------------------------------+---------------------

Comment (by jrf):

 @TimothyBlynJacobs Thanks for your response and yes, those are all very
 good questions.

 > How is the type check going to work?
 > Are we going to allow subclasses?
 > If the requirement is that a class implement an interface, how will that
 work they might not share a common base class?

 My initial line of thinking on this is to just identify the type based on
 the basic types: `boolean`, `integer`, `float`, `string`, `array`,
 `object` or `resource`. Not listing `null` as in that case with a typesafe
 filter, there is nothing to filter.

 I can imagine that for objects we could add ''some'' additional logic to
 make sure that the returned object is still an `instanceof` the original
 object or its parent. Similarly, with resources, we could check that it is
 still a resource of the same type, but that's as far as I'd go.

 Determining full covariance would be taking things too far IMO and could
 also easily become a big roadblock for this ticket to land.

 Along the same lines, I wouldn't want to start checking whether an array
 still has values of the same type and/or a minimum of the same keys as the
 original array. That kind of logic has no place in a filter calling
 mechanism.


 > What about places where we want union types?
 > What if a plugin allows for null to disable the object? How will we
 notate that?

 IMO for that the original `apply_filters()` function should still be used.

 From my point of view, the typesafe variants are intended for **''one type
 and one type only''**.

 Allowing more than one type would negate some of the advantages of the new
 functions as type checking logic within the functions calling
 `apply_filters_typesafe()` and the functions hooking in, can still not be
 removed.

 Once PHP 8 becomes the minimum PHP version for WordPress (in twenty years
 or so), union types could be used in the hooked-in functions and a `try-
 catch` be added to `apply_filters()` to catch mismatches, but that's
 another matter.

 Actually, adding the `try-catch` in `apply_filters()` already with a
 ''doing it wrong'' would probably not be a bad idea anyway as it would
 prevent potential fatal errors when plugins already choose to drop PHP 5.6
 support and add PHP 7 type declarations, but to be fair, that is outside
 the scope of this ticket and deserves it own discussion.


 > How strict will the type checking be? If I filtered a float, will a
 plugin be able to provide an integer?

 That is the one case which I do think the typesafe variant should have
 tolerance for, though with a float cast when an integer is detected and
 only when a the actual value would not be affected.
 This is similar to how that is handled in PHP itself.

 So, when an integer is expected, but a float is returned and `$float ==
 (int) $float`, the value is accepted and an `(int)` cast will be applied
 before passing the value on.
 Similarly, if a float is expected, but an integer is returned, the value
 is accepted as well and a `(float)` cast is applied before passing the
 value on.


 > I'm sure we can probably have some reasonable defaults. But I think this
 API will really need to support accepting a check parameter.

 The only use case I can see for adding that, is if full covariance and
 union types should be supported, but as I said above, I don't think that's
 the way to go.

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