Add new functions apply_filters_single_type() and apply_filters_ref_array_single_type()

[jrf]

This is a proposal to add two new functions apply_filters_typesafe() apply_filters_single_type() and apply_filters_ref_array_typesafe() apply_filters_ref_array_single_type().

Underlying reasons

PHP 8 is a lot more strict about data types and depending on what is done with the data will start throwing more notices/warnings, but more concerning exceptions, which will result in fatal errors (= white screen of death) when uncaught.

Problem outline

Now, say you have a filter hook foo which passes an integer as its second argument and expects an integer to be returned.

<?php
$int = apply_filters( 'foo', $int );

Also say that there are two functions hooked into this filter, function plugin_a() and function plugin_b().

apply_filters() will pass the original integer to plugin_a() and subsequently will pass the returned value from plugin_a() as the parameter to plugin_b().

Now take a situation where plugin_a() returns something other than an integer, while plugin_b() rightfully expects to receive an integer and doesn't do additional type checking (which it shouldn't have to do) and this results in an error.

Plugin B will be identified in the error backtrace as the place where the error occurred, so plugin B will be blamed and get a bad reputation, while plugin A is actually at fault.

Code example: ​https://3v4l.org/uqT9G

Proposed solution

To combat this situation I propose to add two new functions apply_filters_single_type() and apply_filters_ref_array_single_type().

These functions would basically have the same underlying logic as the existing apply_filters() and apply_filters_ref_array() functions, with one big difference:

• At the start of the function, the variable type of the second parameter (the value being filtered) received is checked and remembered.
• In the loop which calls the hooked in functions, a check is done on the variable type received back from each function and if it doesn't match the expected variable type, the previous value is used for the call to the next hooked-in function and a __doing_it_wrong() error is thrown identifying the function which returned the incorrectly typed variable. This includes doing this check after the last hooked-in function is called, before returning the value to the original caller of apply_filters_single_type().

For apply_filters_ref_array(), this would apply to the first array item in $args.

Practical advantages

1. Using these functions will prevent potential fatal errors, diminishing the impact of these type of developer errors for end-users.
2. The "Doing it wrong" notice will make identifying the function/plugin/theme which caused the problem a lot easier.
3. Plugins/Themes and WP core using the new functions will be able to count on the parameter/return type of the call to these new functions and can write their code based on that, without excessive defensive coding to defend against the errors made by others.
4. Using these functions will help in the efforts to identify more PHP 8 related incompatibilities, both in WP Core as well as in plugins and themes.

Technical considerations

1. I considered adding a new $typesafe parameter to the existing apply_filters() function, but as it can be passed a variable number of arguments to be passed along to the hooked-in functions, this is not an option. This doesn't apply to apply_filters_ref_array(), but for consistency I think introducing a new sister-function for both would be the better choice.
2. I've also considered type juggling an incorrectly type value within a try/catch, using that value to pass on to the next function if the juggling succeeded and only using the previous value if the juggling did not succeed, but I think that could cause even more problems, so I'd rather opt for ignoring the incorrectly typed returned value altogether.

Future scope

Once these functions are in place, a review should be done of all WP Core calls to apply_filters() and apply_filters_ref_array() and where the filtered value is expected to be a singular type, the function call should be switched out for a call to one of the new functions.

The WP version in which this review is done will need a good dev-note about these changes as they could be considered a BC-break, but then again, they are only a BC_break for hooked-in functions already doing it wrong and likely causing problems already.

Other

I'm tentatively milestoning this for WP 5.6, even though I realize this is late in the game, but as this proposal could help with the PHP 8 efforts, I believe it would be good to address this sooner rather than later.

/cc @omarreiss

[TimothyBlynJacobs]

This is super necessary, I'd love to see it happen. I had a couple of questions about the implementation.

At the start of the function, the variable type of the second parameter (the value being filtered) received is checked and remembered. In the loop which calls the hooked in functions, a check is done on the variable type received back from each function and if it doesn't match the expected variable type

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?

What about places where we want union types?

What if a plugin allows for null to disable the object? How will we notate that?

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

I'm sure we can probably have some reasonable defaults. But I think this API will really need to support accepting a check parameter. This would still be a benefit over checking the return type manually because it means we won't throwaway the entire filter stack, just the plugin that is broken, and of course is simpler DUX wise.

[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 its 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.

[giuseppe.mazzapica]

Thanks a lot for this @jrf

A few notes:

• plugin.php can't have external dependencies, so before doing any _doing_it_wrong it is necessary to check that function is defined.
• Automatically determine the expected type might not always possible. For example, I can pass a string but what I actually want is a callable. Same with all the other pseudo-types. E.g. an array is passed but any iterable could do.
• Things become even more complex with objects. If I pass an object to apply_filters_typesafe and then I call a method on the result, I want the object I retrieve to have that method. So checking the result to be an object will not be enough, and checking all parent classes can be wrong either. Let's imagine an object that extends/implements A and B, but on the result of the filter, I call a method of B. If the filter returns an instance of A that is no good.
• Regarding null there are two cases: the first is that null is the value to be filtered, the second is that null is the value returned by the filters. In the first case, there's basically nothing to do, passing null to apply_filters_typesafe would equal to apply_filters. In the second case, null could be acceptable by the caller.
• Regarding strictness about int and float, I think the default behavior should be fine with it. Even PHP core does that and there's no need to be stricter than PHP.

To take into consideration all the points above, I think that apply_filters_typesafe should accept a set of args, as in the best WordPress tradition, to fine-tune its behavior.

I can think of accepted_types and nullable options.

That said, because I'm better at PHP than words, I'll paste here what my idea would be.

Note this is meant to only be a discussion point, and I have not tested this at all.

<?php
function apply_filters_typesafe( $tag, $arguments = array(), $value = null, ...$values ) {

    if ( ! has_filter( $tag ) ) {
        return $value;
    }

    static $types_map;
    if (!$types_map) {
        $types_map = [
            'boolean' => 'boolean',
            'integer' => 'numeric',
            'double' => 'numeric',
            'string' => 'string',
            'array' => 'array',
            'resource' => 'resource',
            'resource (closed)' => 'resource',
            'NULL' => 'mixed',
        ];
    }

    $type = gettype( $value );
    $is_object = is_object( $value );
    $accepted_types = isset( $types_map[ $type ] ) ? array( $types_map[ $type ] ) : array();

    // Do not calculate multiple times for same class.
    static $classes_types = [];
    // Skip calculation of accepted types if they are are explicitly passed.
    if ( $is_object && empty ( $arguments['accepted_types'] ) ) {
        $class = get_class( $value );
        if ( isset( $classes_types[ $class ] ) ) {
            $accepted_types = $classes_types[ $class ];
        } else {
            $accepted_types = array( $class );
            $parent = get_parent_class( $class );
            while ( $parent ) {
                $accepted_types[] = $parent;
                $parent = get_parent_class( $parent );
            }

            $accepted_types = array_merge( $accepted_types, class_implements( $class ) );
        }

        $classes_types[ $class ] = $accepted_types;
    }

    $arguments = array_replace(
        array(
            'nullable' => false,
            'accepted_types' => $accepted_types
        ),
        $arguments
    );

    $original = $value;
    // Objects are passed by ref, clone to return original unchanged in case of errors.
    $to_filter = $is_object ? clone $value : $value;

    // `next_filter` function doesn't exist, but you get the point
    $filter = next_filter( $tag );
    $filtered = $filter( $to_filter, ...$values );

    // 'mixed' is a valid PHP 8 pseudo-type so we support for consistency.
    // That said, if mixed is fine then just use apply_filters.
    if ( in_array( 'mixed', (array)$arguments['accepted_types'] ) ) {
        // `has_next_filter` function doesn't exist, but you get the point
        return has_next_filter( $tag )
            ? apply_filters_typesafe( $tag, $arguments, $filtered, ...$values )
            : $filtered;
    }

    static $can_do_it_wrong = false;
    if ( ! $can_do_it_wrong && function_exists( '_doing_it_wrong' ) ) {
        $can_do_it_wrong = true;
    }

    if ( null === $filtered ) {
        if ( !$arguments['nullable'] ) {
            $filtered = $original;

            if ( $can_do_it_wrong ) {
                _doing_it_wrong(
                    __FUNCTION__,
                    "Filters for '$tag' where not expected to return null.",
                    '5.6'
                );
            }
        }

        // `has_next_filter` function doesn't exist, but you get the point
        return has_next_filter( $tag )
            ? apply_filters_typesafe( $tag, $arguments, $filtered, ...$values )
            : $filtered;
    }

    static $functions;
    if ( ! $functions ) {
        $functions = array(
            'int' => 'is_int',
            'integer' => 'is_int',
            'double' => 'is_float',
            'float' => 'is_float',
            'numeric' => 'is_numeric',
            'number' => 'is_numeric',
            'bool' => 'is_bool',
            'boolean' => 'is_boolean',
            'string' => 'is_string',
            'array' => 'is_array',
            'callable' => 'is_callable',
            'function' => 'is_callable',
            'resource' => 'is_resource',
            'iterable' => 'is_iterable',
            'countable' => 'is_countable',
        );
    }

    foreach ( (array)$arguments['accepted_types'] as $type ) {
        if ( isset( $functions[ $type ] ) && call_user_func( $functions[ $type ], $filtered ) ) {
            // `has_next_filter` function doesn't exist, but you get the point
            return has_next_filter( $tag )
                ? apply_filters_typesafe( $tag, $arguments, $filtered, ...$values )
                : $filtered;
        }

        if ( $is_object && is_string ( $type ) && is_a( $filtered, $type ) ) {
            // `has_next_filter` function doesn't exist, but you get the point
            return has_next_filter( $tag )
                ? apply_filters_typesafe( $tag, $arguments, $filtered, ...$values )
                : $filtered;
        }
    }

    if ( $can_do_it_wrong ) {
        $expected = implode( "', '", $arguments['accepted_types'] );
        $actual = is_object( $filtered ) ? 'instance of ' . get_class($filtered) : gettype( $filtered );
        _doing_it_wrong(
            __FUNCTION__,
            "Filters for '$tag' where expected to return a value of one of types: '$expected'. Got '$actual' instead.",
            '5.6'
        );
    }
    
    // `has_next_filter` function doesn't exist, but you get the point
    return has_next_filter( $tag )
        ? apply_filters_typesafe( $tag, $arguments, $original, ...$values )
        : $original;
}

I know it's complex, but there are complex issues to tackle.

Just to leave here a few examples, you could use the above function like this:

<?php
$callable = apply_filters_typesafe('foo', ['accepted_types' => ['callable']], '__return_true');
$callable();

Or:

<?php
$fs = apply_filters_typesafe('foo', ['accepted_types' => ['WP_Filesystem_Base']], $fs);
$fs->find_folder('bar');

Or even:

<?php
/** @var numeric|null $num */
$num = apply_filters_typesafe('foo', ['nullable' => true], 0);
$int = (int)($num ?? 42);

[giuseppe.mazzapica]

Please also note that, of course, in the real implementation I would not expect to call apply_filters or this whole thing would make no sense, so when you see apply_filters in the function above what I mean is "Run the login that runs the next filter".

A proof of concept: ​https://3v4l.org/pbMST

(edited several times the link because I found bugs.)

[TimothyBlynJacobs]

@jrf I agree Core shouldn't try and actually implement LSP out of the box.

I think instead of a complex args array, we should simplify this to allowing users to pass a callable( $original, $filtered ) : bool. We could maybe create a predefined version that uses the simplest of type mechanics. And create custom ones for things like accepting a callable.

IMO a developer creating a wrong object or callable is not significantly less unlikely than providing the incorrect arg type.

[jrf]

Hi @giuseppe.mazzapica, thank you for sharing your thoughts on this. You bring up some good points, though unfortunately I disagree with nearly all.

As the title of the issue seems to leave the impression that these functions should support full type declarations as supported by PHP, I'm going to change the _typesafe in the suggested function names to _single_type.

Let's step through the points you bring up one by one:

Regarding null

Regarding null there are two cases: the first is that null is the value to be filtered, the second is that null is the value returned by the filters. In the first case, there's basically nothing to do, passing null to apply_filters_typesafe would equal to apply_filters. In the second case, null could be acceptable by the caller.

null should never be acceptable. As stated before, passing null as the original value, would mean that running the filters is useless as null would be the only acceptable outcome.
Regarding filter functions returning null, that is also not acceptable as it breaks the principle of this function being typesafe. Filters should always return a value and in the case of filters called via these functions, a value of the same type as originally received, so null would never be an acceptable return.

You are basically suggesting to support "nullable types" in this function. This goes against the principle of one type and one type only and would significantly reduce the advantages of having these functions in the first place.

Regarding callable

I can't think of a single reason for a callable being filterable. To be honest, that just sounds like a security issue waiting to be exploited, which I don't think is a good case to support.

If people still want to do so, let them use apply_filters(), but I'd really wouldn't want to support it in a function with safe in the function name.

Regarding iterable and countable

These I'd need to have a think about, but I'm leaning against a "no", if for no other reason than that it would still require the code in the callback functions hooked into the "typesafe" filter to allow for several different types and several different way to add or remove something from the iterable/countable.

Again, that situation can be handled by using apply_filters().

Regarding number and numeric

First of all, this violates the single type principle again. Second of all, too few people actually understand that is_numeric() will also accept NAN, INF, '1e1' (floats in exponent annotation), ' 1' and more.

Also see: ​https://phpcheatsheets.com/test/numeric_tests/

Supporting number/numeric would also negate some of the advantages regarding PHP 8, which introduces "saner numeric comparisons" and "saner numeric strings".
In other words, the behaviour of whatever is passed through would become unpredicatable PHP cross-version, which goes against the whole point of these new functions and would make identifying the callbacks doing it wrong again more difficult.

Regarding mixed

No. Just plain no. Use apply_filters(), that's the behaviour you expect there. Just like union types, mixed should not be supported by this function.

Regarding adding an extra argument, let's call it $type_spec.

I tried to avoid adding an extra argument to make switching existing calls from apply_filters() to the new functions as easy as possible, but I can see your point about objects.

If we'd go that way, we would basically need to have a "type" key and an "instanceof" key. If "instanceof" is set, "type" would not need to be set, though setting it to object would be acceptable.

But then, why have two array keys ? We know what the supported basic types are and if whatever is passed (as a string) is not one of the basic types, doing an instanceof could be done automatically.

Also, and again if we'd go the way of adding a "type" argument, then I'd advocate for the original value being passed to apply_filters_single_type() to also be evaluated for validity before any callback functions are called, a doing it wrong being thrown if it is not of the expected type and the function short circuiting (not calling any callbacks) if the wrong type was passed.

This function should never fallback to calling apply_filters() as in that case, it negates the whole point of having this function in place, as callbacks would still not be able to trust the input received from via "typesafe" hooks.

plugin.php can't have external dependencies, so before doing any _doing_it_wrong it is necessary to check that function is defined.

Good point :+1:

// Objects are passed by ref, clone to return original unchanged in case of errors.

Good point, this should definitely be taken into account.

[giuseppe.mazzapica]

Thanks @jrf, instead of answering your points one by one, let me try to put in words what I tried to put in code because I think there was some misunderstanding.

To be honest, I'm afraid the direction this ticket is taking lands to now where, and that is for the "single type" principle that is being self-imposed.

First: this does not play well with PHP: a variable can be more than one type. An array or a string both can also be callable. And the concept of "single type" is completely not applicable to objects: object is pretty much useless as it guarantees exactly zero safety.

Second: it does not play well with WordPress. If I understand correctly, the idea is to use of these functions you propose in the core. I'm fairly certain that there's no way it is possible to do that in many places if a "single type" is accepted. There are a huge amount of filter hooks in WordPress that depending on several factors can pass null or a string or an integer. The first example into my mind: a post ID that comes from the DB will be a string, if it comes from cache might very well be an integer. Sometimes a $post variable passed by a core filter is a WP_Post, sometimes is an integer and sometimes a string. To make it short: if you want to use these functions in a relevant number of places in WP core, that will just not possible without giving up on the "single type" thing.

Third: it makes the whole thing much less useful and powerful. As a developer who advocates type-safe code for years, I know very well the "single type" approach that PHP itself had for a long time only resulted in much more untyped code. There's no possibility to develop easily with a type system that only allows a single and invariant type and so the alternative to that is use no type at all. What I foresee if the "single type" approach sticks is that not only will not be possible to use it in the core besides very few exceptions, but it will be much less useful for plugin developers.

I'll try to expand on these topics below, and also answer some of the things you posted.

Typed hooks

When I first saw this ticket I thought "too good to be true", because I thought about the possibility to finally have typed hooks in WordPress.

At Inpsyde, we are very strict about types. We have a couple of PHPCS rules that force us to add a type declaration (argument and return) to every function and method. We automatically exclude from those rules functions attached to hooks for the simple reason that hooks are not type-safe.
If we do something like this:

<?php
add_filter('hook', function (string $x): string { /*...*/ });

We can be sure that this will explode because the core itself of some other plugin will pass something that is not a string.

If we analyze the code above, the piece of code that calls apply_filters has (in theory for now) the possibility to define the type of the hook.

In fact, there's a single place where the result of apply_filters is consumed. That means the code that calls apply_filters is the code that decides the type of the hook, the various places where add_filter is called should stick with that type.

Unfortunately, this does not happen right now: apply_filters declares a type, but code that uses add_filter returns whatever, and that is happily accepted by WordPress.

I thought that this could be a chance to stop this.

The way I imagined this was the same mechanism of a function having the so-called "type hints" for parameters. When that happens, whoever calls the function has to pass a type that is compatible with the type the function declared to accept.

Similarly to that, whoever calls apply_filters_typesafe (let me stick with this naming for now) would be able to declare the type of the hook, and then callbacks added to it would be required to return compatible types.

<?php
/** @var string $my_string */
$my_string = apply_filters_typesafe( 'hook', /* ... */ );

Doing so, I would finally be able to safely things like this:

<?php
add_filter('hook', function (string $x): string { /*...*/ });

This would be safe because core would refuse to pass to the next hooking callback something that does not respect the declared hook type. This would be a huge improvement to WordPress, besides the support of PHP 8.

Determining the type

Now that is (I hope) clear what for me is the purpose of an apply_filters_typesafe, that is having typed hooks, we can surely proceed in saying that we could enforce the rule that a typed hook must have a single type.

I'll come back to this later, but for now, let's assume we decide to stick with that.

There would still be two problems:

• it will not always be possible to infer the type from the given filter "subject"
• it will not work in a multitude of existing WordPress hooks

The callable issue

You said:

I can't think of a single reason for a callable being filterable.
To be honest, that just sounds like a security issue

The security issue argument is not existent. A callable to be executed must be loaded, that is the file that declares it must be loaded by WordPress. And if a file is loaded, it could do anything, including wiping the entire database, insert new super-admin users, and so on. The fact that it could also filter a callback is not the problem there. If unchecked PHP files can be included in the application that is a huge security issue, but that's a different story and has nothing to do with the fact that a callable is filterable.
WordPress even allows in several places to re-write declared functions (drop-ins, child-themes), there's no security issue there.

Regarding "a single reason for a callable being filterable", I don't have to think. I can show you some examples in the WP core: ​https://developer.wordpress.org/?s=_handler&post_type%5B%5D=wp-parser-hook

Any of those "handler" hooks accepts a callable. The link above was the one I find more effective to show a list of functions that expect callbacks with a single link, but I think there are other examples in core. Moreover, I can tell you that the "filterable callback" is a pattern that I myself use very often, and because I often review my colleague's code, I can assure you I'm not the only one. Finally, I'm very sure that extremely popular plugins do it here and there.

Now, considering that WordPress passes a string to those functions if WP would auto-determine the type that would be string. Which of course is not valid because an anonymous function or an object method that is correct would be refused and a string that is not callable would be accepted.

Objects

How do we determine the "single type" for objects? You said to stick with the object type.

How this is useful?

When using object-oriented programming the piece of code that declares the type of the hook (that is the piece of code that calls apply_filters_single_type) can decide that anything implementing an interface will be fine. For example:

<?php
/** @var Iterator $my_iterator */
$my_iterator = apply_filters_single_type( /* ... */ );

while( $my_iterator->valid() ) {
  $my_iterator->next();
}

If I pass an ArrayIterator, the code that tries to determine the "single accepted type" will have a few choices:

• accept with object: this will obviously break the snippet above because the hook type is not respected. Moreover, the problem this issue tries to solve is not solved being the "typed hook" not typed anymore: if I add a filter using a function that declares Iterator as param type a fatal error will happen, and my plugin will be blamed even if it was another plugin to return an object that is not an iterator as desired.
• accept only instances of ArrayIterator. Could work, but it is extremely limiting. As a dev who has practiced OOP in the everyday job for a few years, the most common reason why you pass an object to a filter is to allow consumers to replace that object with a different implementation of the same interface. In fact, if you want consumers to only interact with the object you use an action, not a filter. (What, for example, WordPress does with pre_get_posts passing a WP_Query instance).
• accept any of the interfaces/parent classes of ArrayIterator. Again not useful at all. A class having only the Serializable interface would be accepted, but the "hook single type" here is Iterator not Serializable, and the code snippet above would end up in a fatal error.

Other types

For other types, e.g. the auto-casting from int to float and vice-versa, that is something PHP happily does even in PHP 8 (​https://3v4l.org/alj77) and I really don't see any reason to be stricter than PHP, but if that lands in core... I would not complain.

The only reason why in my proof-of-concept I accepted any numeric value when an int or a float was given to the filter was that this is how WordPress currently handles those values.

When a post ID is passed around in hooks it might be very well a numeric string or an integer. So I thought that supporting is_numeric instead of is_int would have made the integration in existing WordPress hooks much easier.

After all, if I know that the type is numeric, I can safely do a casting downstream:

<?php
/** @var int $post_id */
$post_id = (int)apply_filters_single_type( 'hook', /* ... */ );

Cast to int will never fail if I have the guarantee that the filter returns a value that passed the is_numeric check.

Regarding functions hooking this filter, in PHP 8 they could even have type-declarations:

<?php
/** @var int $post_id */
add_filter( 'hook', fn(int|string|float $id) => related_id( $id ) );

That said, once again, if the desired direction is to be very strict about this I will not complain but suspect that this will reduce the possibility of this ticket to be merged.

About null

I'll directly start with an example from core: ​https://developer.wordpress.org/reference/hooks/posts_pre_query/

That filter passes null because it allows overriding the results of the query before the query runs and null, which is passed as the default value, is a signal that it has not being overridden. The reason is simple: if a filter returns an empty array what it means is that the query is forced to have no results at all. So null allow distinguishing "not filtered" from "no results".

This kind of "pre" filtering is very common in WordPress: ​https://developer.wordpress.org/?s=pre_&post_type%5B%5D=wp-parser-hook

Sometimes false is used as the discriminant in place of null but the idea is the same. How are we going to handle this very common case sticking with "the single type principle"?

If we think in terms of "typed hook" it makes total sense that WordPress declares a hook to have the type array|null. This is something that for PHP supports since PHP 5.1 (​https://3v4l.org/fVVnX).

If we stick with the "single type" approach, there will be no possibility to leverage the new apply_filters_single_type for such cases, which means that WP would stick with the apply_filters: again, when the type system is not flexible the only possibility is having no types. That means that all the filters like posts_pre_query would be "condemned" to be untyped and thus break WP with a fatal error if they receive an unexpected type.

Configurable type

All the examples above try to prove that having sensitive defaults to automatically infer the hook type is very fine, but there should be a way to support the explicit declaration of the hook type.

Let's call that explicit declaration $type_spec, as @jrf proposed, so we don't have yet to define the form it takes.

How to do that? Because defaulting to the type of the filter subject is a sensitive choice it would make totally sense to have this as the last parameter of the functions, making it possible to have a default value.

Unfortunately, apply_filters is a variadic function so if we want to have a signature that resembles that function it is not possible to append a new parameter.

And if we agree that having a different approach for the two functions discussed in this ticket is not desirable, I see two possibilities:

• Prepend the argument, having something like apply_filters_single_type($tag, $type_spec, $subject, ...$args) and apply_filters_ref_array_single_type($tag, $type_spec, $args)
• Only have the ref_array variant: apply_filters_ref_array_single_type($tag, $args, $type_spec). Which at that point could be renamed apply_filters_typed($tag, $args, $type_spec). That means that if one wants a typed filter they has to use this function, which is not anymore a variant of the existing apply_filters / apply_filters_ref_array but it is a new function whose happen to accept in its simplest form the same two parameters of apply_filters_ref_array: the hook name and an array of args (using the default for $type_spec). Considering that any occurrence of apply_filters could be replaced with this function, and considering that any code doing add_filter could be left unchanged, I don't see this as an issue, actually, it would reduce the additional API introduced which can only be good.

The examples about null above IMO proved there's the necessity to, at the very least, allow for nullable types to make this typed hook function anywhere useful.

If we assume there's a single type, and so $type_spec can be a string, we could still support nullable types. We don't have to think up a syntax, PHP did that for us:

<?php
$this->posts = apply_filters_typed( 'posts_pre_query', array( null, $this ), 'array|null' );

or an example with a filter currently using apply_filters instead of apply_filters_ref_array:

<?php
// was: $check = apply_filters( 'pre_delete_post', null, $post, $force_delete );
// see: https://github.com/WordPress/WordPress/blob/master/wp-includes/post.php#L3013

$check = apply_filters_typed( 'pre_delete_post', array( null, $post, $force_delete ), 'bool|null' );

This would be IMO a very effective solution:

• Reduced new API: one functions instead of two
• Sensitive defaults: the third parameter could default to null, and when that's the case, the accepted type would be determined by WordPress.

Regarding how to calculate the default, e.g. if passing an int strictly accept int only, or int|float or numeric that is an implementation detail and, once again, I would be very fine with any of the choices but I suspect that the more permissive choice is what will make this more easily integrated into the core codebase.

Single type?

The question now is: if we allow nullable types which are a form of union types, why don't accept the full union types specification, considering that WordPress has many places where the filtered types can assume several types and also considering that this ticket happens in the context of support for PHP 8, a version that introduces union types?

In other words, if we have an implementation that allows for:

<?php
apply_filters_typed( 'hook', array( null, $value ), 'bool|null' );

there would be close to zero complications to also making it allow for:

<?php
apply_filters_typed( 'hook', array( null, $value ), 'bool|int' );

but that would provide huge benefits to plugin developers and at the same time will, once again, ease the process of using this kind approach in the core, when there are filters whose type is int|string|\WP_Post|null and moving away from that would require an immense refactoring that will never happen (at least not in the next decade).

Regarding using the string form like bool|int or the array form array('bool', 'int'), or yet something else, is an implementation detail that I think is not really relevant now.

Permit me to say that the "single type principle" is a self-imposed principle that not only overlook the current state of WP code, but also the evolution of PHP that in recent versions has put a lot of effort in moving away from the "single type" approach, introducing pseudo-types like callable (PHP 5.4) and iterable (PHP 7.1), covariant and contravariant type declarations (PHP 7.4), and union types (PHP 8.0).
All these things were done for the only reason that "single type" is just not flexible enough and that developers to overcome this lack of flexibility were forced to have no-type. If that teaches us anything, that is that the only consequence of having a not flexible type system is having a no-type system.

About pseudo-types

I personally see a lot of value in having a "type spec" that allows for things like numeric, that is types that are not valid PHP type declarations but have a core way to be checked (is_numeric), but if the decision is made to don't support them, no strong feelings from me.

Only consider that allowing for numeric will provide the possibility to be much more tolerant with existing code. I'm very sure out there, besides core, exist thousands of plugins that return a numeric string where a post ID is expected. Making "typed hook function(s)" very strict about the type will be such a huge breaking change that I hardly see it even thinkable in WordPress, so either "typed hook function(s)" would be used much less in the core (and by plugins), or they would not see the light at all.

That said, considering that to make "typed hooks" anywhere useful the support for pseudo-type like callable is kind-of mandatory, I don't really see the reason why a pseudo-type like iterable, that is also a valid type declaration in PHP, should not be allowed.

[overclokk]

I would like to add some sugar to this ticket because I'd like to have more "typed code".

I thought at two alternative:

1 - Have as many functions as many type we want to support, one function per type:

<?php
\apply_filters_type_string( 'hook', 'string', ...$args );
\apply_filters_type_int( 'hook', 1, ...$args );
\apply_filters_type_array( 'hook', ['val1', 'val2'], ...$args );
// ... and so on

Why? Because I think of having more declarative function reduce the amount of logic inside the function itself, separation of concern, and I don't see any issue with having more API functions in core if they can help to write better code, and we don't have to write functions for all type, only for some and let the developers to write its own if they need them.
With this we could simply switch from \apply_filters() to \apply_filters_type_*() simpler.

2 - A system more similar to PSR-14 and only for one type, object:

<?php
function do_action_psr( 'hook', object $obj ) {
    // Do some stuff
}

//...

$obj = new My_Object( /* With arguments */ );

\do_action_psr( 'hook', $obj );

Why? Because with this I could use the Value Object pattern and if I want to do stuff more strictly I can write a decorator function like this:

<?php
function do_action_my_interface( 'hook', My_Interface $obj ) {
    // Do some stuff
    \do_action_psr( 'hook', object $obj );
}

//...

$obj = new My_Object( /* With arguments */ ) // extends My_Interface;

\do_action_my_interface( 'hook', object $obj );

With objects I don't even need to use apply_filters() because they are passed by reference.

Beetween the two I prefer the second even if I can see that there's one issue with this, if we want more types we have to write more decorators.

Obviously these are only my two cents.

[noisysocks]

Bumping this from 5.6 as we're past the cut-off for enhancements.

[hellofromTonya]

Summarizing discussions during 5.7 Core Scrub:

From @ jrf:

To be fair, the responses so far were IMO too ambitious. We need to learn to crawl before we can walk. Take it one step at a time, start with single type and have a potential roadmap for the future for making it more complex, but we do need to start with this as it will save a lot of plugins a world of pain.

I agree: baby steps. Start small and take one step forward. This approach gets us started while giving us the ability to learn and iterate.

What is the next step?

From @ jrf:

I think we need to have some more people vocalize that one step at a time is a good idea and get some visible consensus on that before creating a patch.

⭐️Next Step Proposal ⭐️

Baby step approach adding one type at a time. Start with one single type.

1. Start with single type filtering.
2. Add a lot of unit tests for this single type.
3. Then create/update roadmap for the future steps to add more types.
4. Repeat.

📣Seeking your feedback 📣

Do you agree with the one step at a time approach?

[hellofromTonya]

It's too late in the 5.7 alpha cycle for this ticket as we are still working to gain consensus on the proposed baby step approach. Raising the ticket in Dev Chat. Let's see if we can get consensus and then pull it into 5.8 early.

[audrasjb]

The approach proposed by @hellofromTonya makes sense to me.

1. Start with single type filtering.
2. Add a lot of unit tests for this single type.
3. Then create/update roadmap for the future steps to add more types.
4. Repeat.

[herregroen]

I would suggest two functions

apply_filters_typesafe( $tag, $value, ...$args ) which uses gettype to retrieve the type of the $value and the uses the appropriate is_ function to validate that type.

apply_filters_typed( $type, $tag, $value, ...$args ) in which $type is either a primitive or a class name. Primitives are validated using the appropriate is_ function and class names are validated using is_a || is_subclass_of. In the future we could allow for arrays as $type to allow multiple return types but that can be added later or even more complex objects to, for example, describe the shape of arrays. But the first implementation should remain simple and focus on just a string $type argument.

The mixed type should also be supported for the simple reason that this will allow apply_filters, apply_filters_typesafe and apply_filters_typed to share one single implementation.

apply_filters( ... ) is simply apply_filters_typed( 'mixed', ... ).
apply_filters_typesafe( ... ) is simply apply_filters_typed( gettype( $value ), ... ).

This means we have just one function to maintain and test as opposed to multiple.

Type-checking would be the following function:

<?php
/**
 * Checks whether the given variable is a certain type.
 *
 * Returns whether `$value` is certain type.
 *
 * @since 5.8.0
 *
 * @param string $type  The type to check.
 * @param mixed  $value The variable to check.
 * @return bool Whether the variable is of the type.
 */
function is_type( $type, $value ) {
        switch( $type ) {
                case 'boolean':
                        return is_bool( $value );
                case 'integer':
                        return is_int( $value );
                case 'double':
                        return is_float( $value );
                case 'string':
                        return is_string( $value );
                case 'array':
                        return is_array( $value );
                case 'object':
                        return is_object( $value );
                case 'resource':
                case 'resource (closed)':
                        return is_resource( $value );
                case 'NULL':
                        return is_null( $value );
                case 'unknown_type':
                        return false;
                case 'mixed':
                        return true;
                default:
                        return is_a( $value, $type ) || is_subclass_of( $value, $type );
        }
}

The main apply_filters loop in WP_Hook would have to be as follows

<?php
// Avoid the array_slice() if possible.
if ( 0 == $the_['accepted_args'] ) {
        $next_value = call_user_func( $the_['function'] );
} elseif ( $the_['accepted_args'] >= $num_args ) {
        $next_value = call_user_func_array( $the_['function'], $args );
} else {
        $next_value = call_user_func_array( $the_['function'], array_slice( $args, 0, (int) $the_['accepted_args'] ) );
}
if ( ! is_type( $type, $next_value ) ) {
        _doing_it_wrong(
                $the_['function'],
                sprintf(
                        __( 'Invalid type returned in filter. Expected %1$s but received %2$s' ),
                        $type,
                        gettype( $next_value )
                ),
                '5.8'
        );
} else {
        $value = $next_value;
}

[herregroen]

I've added a very quick and dirty patch of what I propose for reference. Code itself likely still needs work but should help to illustrate what I intend.

Introduce type checking to the filters system by:

• Adding new methods to the hook class apply_filters_typesafe() and apply_filters_typed()
• Add new function wp_is_type() to check if a value is of the corresponding type
• Backward compatibility with apply_filters by passing a mixed type
• New functions apply_filters_typesafe() and apply_filters_typed()

New methods and functions have tests coming with them to validate their behaviour.

Trac ticket: https://core.trac.wordpress.org/ticket/51525

[tabrisrp]

Refreshed this as part of contributors day at WordCamp Canada. Proposed PR takes after @herregroen proposal and adds tests for the newly introduced methods and functions.

[dmsnell]

Counts the type of initial filter values and the types of returned filtered values when filtered.

[dmsnell]

Before forming many personal thoughts, I wonder if it would be helpful for us to measure the breadth of this particular issue at large. The reason is that I wonder about the underlying warrant.

PHP 8 is a lot more strict about data types and depending on what is done with the data will start throwing more notices/warnings, but more concerning exceptions, which will result in fatal errors (= white screen of death) when uncaught.

This is true, but hopefully also WordPress developers are being educated that filter inputs are always mixed. Okay, well maybe this isn't true, but I see many errors in practice that are related to people assuming that a given input is in an expected form, particularly in server code for blocks. However, in almost all of those cases, the types are correct, so of the frequent cases I've already seen, type enforcement would leave the bugs unseen.

they are only a BC_break for hooked-in functions already doing it wrong and likely causing problems already.

With a quick scan of Core code, I found that determine_current_user already returns string|boolean, and more may exist. Maybe this is broken, but I think that the prevalence of null|other_type is really extremely common. In fact, maybe it's more like null|false|other_type.

In the case of crawling before walking, I think some empirical evidence could still help, because maybe single-typed filters don't provide the value we'd hope for, aren't applicable as frequently as we suspect, and only introduce breakage where existing code worked fine (sadly the primary result I've seen of adding strict types into existing PHP code).

---

Perhaps we could start collecting this data with the Playground and the plugin previews in the Directory, or setup a test trying to run through various flows and code paths to see the extent of how types vary and how those are used. Without plugins installed most filters don't have attached functions, so it'd only really be possible to judge by integrating with plugins, even many plugins at once. Maybe we could use something like this to sample real-world sites and get more realistic data.

In summary, I wonder what the situation in practice looks like, and if it's prevalent that plugins are returning the wrong type and that the wrong type causes defects then it should be easier to consider introducing a new primitive as fundamental and exposed as the proposed ones would be.

My script, which took only a few minutes to write, prints a text file for visual inspection and produces a JSON file with actual call counts. For each filter name, the type of the initial $value is an array key, and then under each of those initial value types is a key/value array of filtered value types and the number of times a filter returned that type.

[jrf]

@dmsnell While I appreciate the idea, I don't think empirical data has any value in this case.

It's been four years since PHP 8.0 has been released, so most commonly used plugins will have put guard code in to prevent acting on incorrect/unexpected input data to prevent their reputation being damaged by other plugins/themes doing it wrong.
And that includes Core too in a number of places.

Where the value of the proposed functions is now, is mostly for new filters being added, whether it is Core or in plugins/themes. If these functions would be available, anything hooking into filters run via the new functions would not have to worry about type safety of the input anymore as they can be guaranteed that the type will be correct, which would allow for cleaner code inside the hooked in functions.

[mathieulamiotwpmedia]

Thank you for your ideas! I would tend to agree with @jrf, especially "commonly used plugins will have put guard code in to prevent acting on incorrect/unexpected input data to prevent their reputation being damaged by other plugins/themes doing it wrong."
This is one of the main motivations behind this enhancement. Maybe this point of view differs depending if you are a Core maintainer or a 3rd party developer, but as a 3rd party developer, even if cases of filter misuse are rare, this is already too much. Unfortunately, we have frequent reports like that on code that we did not safeguard properly. Hence our need for an off-the-shelf safe-type mechanism. I assume this is a general issue in the ecosystem, that would push developers away from actively providing customization with filters
.
I am saying safe-type, not strict type: the idea here is resiliency, not failing early.

As this ticket has been discussed at yesterday's weekly of WP Core devchat, here are some feedbacks and next steps:

• It would be good to handle, or at least anticipate union types and nullable types, based on PHP syntax (for instance ?int|string). @tabrisrp, I and our team will look into this great idea and circle back with a refined proposal.
• "changes to src/wp-includes/class-wp-hook.php shouldn't rely on other files. There are situations where it is loaded extremely early." We'll look into that as well!
• Should we use doing_it_wrong or something else, maybe failing early to enforce types? In the spirit of resiliency, doing_it_wrong allows to preserve safety while informing. A mechanism that would fail early would not fit the need as a 3rd party developer, and would reduce the interest in this feature. Let's gather opinions on this point.
• "For a major change to such a foundational part of the Core API, I would also think the next steps would be to have a committer willing to "sponsor" this effort (for lack of a better term). Someone who would help support the creation of a Proposal, collect feedback, and help define when the feature is "merge" ready." I do agree with that! So if any committer would like to sponsor this initiative, we'd be happy to continue the effort!

Thanks, we will try to circle back here with a refined proposal about the 2 first points in a few weeks.

[jrf]

It would be good to handle, or at least anticipate union types and nullable types, based on PHP syntax (for instance ?int|string).

I'm not sure this actually has merit as in the case of a multi-type input, the hooked in functions would need type checks anyhow, so what would be the benefits of using the new functions in that case ?

[mathieulamiotwpmedia]

It came up in the discussion as apparently, a majority of filters in the core could not use a single type. I don't know the Core well enough to have an opinion on that though.

I can see how nullable types could be used, with code abusing a bit booleans and being permissive with null cases for instance.
About union types, we would probably not use it as plugin or theme developers, because as you mention, it's a bit missing the goal of simplifying safe checks (and that's probably why we did not even think about it in the first place). But if it is not too complex to add and it can help adoption for already existing filters in Core, we would give it a go...

[tabrisrp]

Before going further with the changes in core itself, we have made an update on the library we created to mimick this concept of typed filters. The new version adds support for:

• union types (i.e. int|string)
• nullable types (i.e. (?string)
• typed array (i.e. string[])

The code is fully covered by unit/integration tests. ​https://github.com/wp-media/apply-filters-typed/

Hopefully that would cover the concerns raised in the previous slack discussion, and help the solution go further for an adoption in core.

[johnbillion]

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.

​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.

​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. _doing_it_wrong() itself calls apply_filters(), as does the proposed wp_is_type(). This has the potential to turn what would be a PHP notice or warning into a fatal out of memory error.
6. 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.

[tabrisrp]

While I understand where you're coming from and your reasons, we made the choice to invest time into working on this ticket because it is solving a problem that was recurring often enough in our case to we felt it would be worth it.

For context, we have one of our plugins installed on more than 4 million websites, which gives us a good sample.

Errors related to types happened pretty much anytime we tried to be strict about it for action/filter callbacks. The most problematic thing is that the reported error will point out to our plugin, not the plugin misusing the filter values to begin with, so the burden ends up being on our support and development team. It's not hard to debug, but with our volume it's an unnecessary waste of time.

If I were to search in our support history about it, I could probably bring a good dozen of examples, the latest happening last month.

Related to your point 3, since any return typehint related to filters can't be trusted, we have to add necessary type checking inside the plugin methods, so the performance impact ends up being delayed to a difference place anyway.

In any case, we believe the idea of stricter filters is beneficial, and using our library in our plugins is already helping us. That's why we wanted to share this with core.

If the final decision ends up to closing it without going further, it will be disappointing, but it would be good to have a clear voice on this, as the feedback I got yesterday on Slack was to have someone from core sponsor this for 6.8?