Add is_post_type_viewable filter

[powerbuoy]

It would be great if we could hook into is_post_type_viewable() and override the check.

We've created a new config for post types; has_single, which works very much like has_archive only it disables single pages for a post-type (useful for post-types such as office or employee which usually in our cases only have archive pages).

However, even though the post-type shows a 404 on single (using template_redirect), inside the admin the "View"-links and "Permalink" settings still show up. It seems the easiest way to hide everything related to single pages is to simply return false from is_post_type_viewable().

[powerbuoy]

Thinking about it, an even better solution would probably be to add official support for has_single :)

[powerbuoy]

Found a duplicate of that: #43347 but seems abandoned. Perhaps adding an is_post_type_viewable filter will be easier?

[donmhico]

Hi @powerbuoy,

Thank you for the ticket. 49628.diff introduces a new filter post_type_viewable which exposes $post_type to allow users to return either true or false depending on their needs.

[peterwilsoncc]

Related #49380

[peterwilsoncc]

@donmhico Minor note for the docs, when the filter is called $post_type will always be a WP_Post_Type object. If the function was passed a string it will be converted by that point.

[deepaklalwani]

Update filter and $post_type parameter description

[SergeyBiryukov]

Thanks for the patch! Moving to a future release for now to handle with #49380.

[audrasjb]

Posts, Post Types: Introduce post_type_viewable hook to filter the result of is_post_type_viewable() function.

[audrasjb]

49628.3.diff refreshes the previous patch against trunk, rephrases a bit the docblocks wording and update the @since info.

Moving for 5.9 consideration.

This will also need a small note in the "Miscellaneous changes" dev note.

[audrasjb]

Posts, Post Types: Introduce post_type_viewable hook to filter the result of is_post_type_viewable() function.

[audrasjb]

In 49628.4.diff: Previous patch refreshed against trunk.

[hellofromTonya]

There's a risk of a backwards-compatiblity break in directly returning the filtered value. Why? There are currently no checks to ensure a boolean value is returned through the filter before returning from this function.

I suggest:

$is_viewable = apply_filters( 'post_type_viewable', $is_viewable, $post_type );

// Make sure the filtered value is a boolean type before returning it.
if ( ! is_bool( $is_viewable ) ) {
    return false;
}

return $is_viewable;

Notice that if it's not a boolean data type, it returns false. This is a safer approach than attempting to type cast to bool when the return value could be of any data type.

[audrasjb]

Posts, Post Types: Introduce is_post_type_viewable hook to filter the result of the is_post_type_viewable() function.

[audrasjb]

Patch refreshed to take into account @hellofromTonya’s comment and also to rename the hook.

[hellofromTonya]

In 52024:

Posts/Post Types: Add filter to is_post_type_viewable().

Introduces a new filter 'is_post_type_viewable' which allows overriding the check. The expected filtered value is a boolean. As filtered values can change, including the data type, this commit includes a is_bool() check, thus ensuring backwards-compatibility.

Follow-up to [33666], [36402].

Props audrasjb, deepaklalwani, hellofromTonya, peterwilsoncc, powerbuoy, sergeybiryukov.
Fixes #49628.

[hellofromTonya]

Thank you everyone for contributing! The new filter 'is_post_type_viewable' is committed and will ship with 5.9.

[Cybr]

Thank you for the patch! I have a comment on it, however.

There's a risk of a backwards-compatiblity break in directly returning the filtered value.

I'm not sure where this backward-compatibility issue lies. Isn't this a newly introduced filter that augments an already boolean value? I believe the defensive programming can be removed:

1. The documentation clearly states it should be a boolean.
2. The return value can be cast to a boolean, which should suffice: return (bool) $is_viewable;

Currently, this defense adds 3 extra opcodes, of which 1 jump -- roughly 17 times per request on a clean install.

If we want to focus on performance, we need to be reticent with introducing defensive code and expect developers to abide by the types. I think this is especially true (pun intended) for newly introduced and substantially better-documented functions/methods/filters/actions.

[hellofromTonya]

Replying to Cybr:

1. The documentation clearly states it should be a boolean.
2. The return value can be cast to a boolean, which should suffice: return (bool) $is_viewable;

Type casting to boolean (and any type) is not the same nor is it as predictable as checking that the expected data type is actually returned. For example, if a callback in the wild returns an object or an array with an element in it (array( false )), type casting returns true. What happens if nothing is returned when the non-filtered state is true? Type casting would set it to false.

PHP is going in the direction of throwing notices and later fatal errors for type mismatches. PHP 8.1 shows us the trend where passing the wrong data type to a native PHP function will result in a deprecation notice and then later in PHP 9 a fatal error. If this trend continues, it's reasonable to leap forward to where type casting non-scalars could also follow that trend.

These types of strict checks guard Core, users, and extenders.

Yes, this does add 3 extra opcodes. Benchmarking the added type check would yield a tiny microsecond or less as it uses inexpensive native PHP logic.

In the future, type safe filters will come to Core (maybe in 6.0). When it does, this code can be safely replaced with it.

What do you think @Cybr?

[hellofromTonya]

Inspiration hit me tonight inspired by @Cybr. This is a boolean return. Check it like this:

<?php
return true === apply_filters( 'post_type_viewable', $is_viewable, $post_type );

No type casting.
No is_bool()conditional.

Patch coming.

[hellofromTonya]

Reopening to simplify the strict type check to conserve memory or processing (performance).

[hellofromTonya]

Simplifies the strict boolean type check to conserve memory and processing. Follow-up to [52024].

[hellofromTonya]

Marking follow-up patch for commit.

[peterwilsoncc]

The functional change in 49628.6.diff​ looks good to me.

In 49628.7.diff​ I've added a note to the filter description indicating that for a post to be considered viewable the filter must return a boolean true with an explanation as to why.

@hellofromTonya if you don't think the explanation is required, then I'm happy if your version of the patch goes in.

[hellofromTonya]

@peterwilsoncc The explanation is good as it provides context for why it's doing a strict type comparison. Should add that to the other filter too.

[Cybr]

Thank you for following up. I believe the welcome change from comment:18 helps with performance greatly. However, I am still not convinced it's the right way forward. I know WP 5.9 is imminent, so I ask for forgiveness for taking time away from other matters in need of attention.

First, I'd like to ask: "Should we make WordPress strictly typed? And if so, could we do it gradually, thus inconsistently?"
I think "no" and "please do not."

My goal is to prevent this tactic from becoming a strategy: PHP is not a strictly typed language, and it shall never become one. Whoever is echoing that postulation should stop.

If this trend continues, it's reasonable to leap forward to where type casting non-scalars could also follow that trend.

I doubt it will. PHP's ​RFC for 8.1 is only about nullable, which I believe will resolve internally as void, in turn removing a strict check ("is a parameter not inputted or is it null?"), improving performance. Even if the trend continues, we're not there yet; let's work with the facts we know: We're computer scientists, after all.

PHP went from a weakly typed, slow, and buggy language to a dynamically typed, fast, and reliable language. Facing HHVM/Hack, the PHP Group began its quest for performance, making them rethink how to model PHP's internal structure. In this, they found that the overhead of type juggling is inefficient and it made both their and userland's debugging process difficult.

1. ​Why is strict type checking problematic?
2. ​Background and Rationale (with introducing scalar type checking).

For example, implode() could take its parameters either way -- now, its order is strict. In PHP 7.4, they started warning userland; from PHP 8.0 thenceforth, they block its mixed usage with a fatal error (​see it in action). ​Their RFC states this change was necessary for the function was inconsistent, and it didn't help their move towards strict type checking internally that improves the aforementioned inefficiencies and debugging processes.

However, with this ticket's proposed patch, there is no warning nor error to the filter's user: It simply casts an incorrect return type of the filter to a different type and value, but then inconsistent with how PHP would juggle. PHP developers expect type juggling to go in ​a specific direction, but with this patch, when the developer doesn't abide by the strictly typed documentation they ought to follow (which we shouldn't expect from juniors), they'll find (after hours of debugging) that their weakly inputted 1|'1'|['1'] == true unexpectedly became a strictly returned false.

Please keep in mind that PHP internally (them) is not PHP userland (us). So, just because PHP is trending towards becoming strict-typed for native functions, it doesn't mean PHP will start bulldozing dynamically typed code that utilizes it. In fact, they are proud of managing a dynamically typed language, as shown in the first two links I enumerated above.

I believe it is not Core's duty to handhold developers who think sending an incorrectly typed value is acceptable. I beg you not to mangle unexpected variables from on-server developer APIs. It makes onboarding difficult, bloats the code to no extend, and it makes WordPress painfully slow. Microseconds quickly add up to milliseconds, and even 5 milliseconds are noticeable to humans.

I also ask you to change your perspective of PHP. PHP userland is not PHP's backend, and we can exploit that beautifully. Manifest typing is not cast upon us, and I doubt it ever will; the defenses placed in this patch act as though it did. Those are more obvious in comment:11 vs. comment:18 (ultimately, they act the same).

In the future, type safe filters will come to Core (maybe in 6.0). When it does, this code can be safely replaced with it.

#51525 is an excellent proposal for plugins and theme developers that forward filter values to strictly typed functions. However, I am well on my way to removing all type-declarations from my plugins. The reason is that Zend Engine is not optimized for this; it adds ​~5% function overhead + ~4% per variable. IIRC, OpCache was promised to check for types-declarations in the future; however, the large majority of WordPress websites do not have OpCache enabled or have it misconfigured by their host.

I think my story boils down to that we should:

1. handle variables strictly when the type is implied ('hello' ==== $user_var);
2. handle variables weakly of boolean type ($act = ! $user_var or if ( $user_var ));
3. cast when it makes sense (42 === (int) $user_var) (PHP can emit conversion warnings, good! Less work for us.);
4. cast filters ($thing = (string) apply_filters()) (PHP can emit conversion warnings, good! Less work for us.);
5. cast return types return (bool) $user_var.
6. only emit deprecation notices (doing it wrong) when behavior changes from earlier WP versions.

I also believe that only if WordPress should ever become strictly typed should we check filters and functions' types strictly. We're already facing issues with array vs. iterable; let's not pile up.

Please reconsider. I apologize for this long post.

[hellofromTonya]

Excellent write up @Cybr! Thanks for sharing

Let's back up a moment.

This legacy function returns a boolean. This ticket adds a filter to allow extenders to override that boolean, i.e. flip the state.

The goal of this change is to ensure this function does not change what it returns, i.e. remains backwards-compatible.

What about type casting?

Type casting other boolean representations is fine (1, 0, '1', '0') as these directly correlate to true and false. However, other values and/or data types do not. Though PHP will type juggle, the boolean state after juggling does not directly correlate (i.e. they don't directly mean boolean true or false). For example, an object does not directly correlate to a boolean, nor does an array with elements, etc. ​https://3v4l.org/m5fBl

There are many times when type casting is the right approach when the data type/value is known. For example stringified numbers.

But filters are different. Why? Any callback can return anything. It's unpredictable.

Just like with performance efforts, predictability, robustness, and stability are also important.

The goal in this ticket is not to introduce strict typing to core as a standard. That would require a separate ticket, a Make post, and a lot of discussion. The goal is to ensure the functions retain their expected behavior and return type.

Is this a new design pattern in core (i.e. boolean strict comparison for a filter)?

No. Here are other examples:

• ​https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/class-wp.php#L671
• ​https://github.com/WordPress/wordpress-develop/blob/master/src/wp-admin/post.php#L182
• ​https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/canonical.php#L894
• ​https://github.com/WordPress/wordpress-develop/blob/master/src/wp-admin/includes/class-wp-posts-list-table.php#L464

[hellofromTonya]

Improves the explanation in 49628.9.diff to be consistent with the similar change in #54375.

[hellofromTonya]

In 52045:

Posts/Post Types: Improves the 'is_post_type_viewable' filter boolean return.

As a follow-up to [52024], simplifies the strict boolean type check to conserve memory and processing. Also includes an explanation of why a strict boolean is required as the returned filtered value. This commit is consistent with the implementation in [52043].

Follow-up to [33666], [36402], [52024].

Props hellofromTonya, peterwilsoncc, cybr, jrf.
Fixes #49628.

[Cybr]

Sorry for barging in again. I'll provide my counter-arguments briefly this time.

1. I agree that the return values of is_post_type_viewable()/is_post_type_viewable() should remain boolean.
2. I disagree filters is_post_type_viewable/is_post_status_viewable should be typed strictly.

Those are mutually exclusive: One does not affect the other.

1. The function return value doesn't change when you typecast the filter with (bool). This is in line with the goal.
2. We should not fix PHP's "quirks" for type juggling. PHP gives [] == false and [false] == true. Those quirks apply to everyone on every line of comparative code they write.

Although the strict checks are not a new design pattern in Core, I consider them all incorrect.

To repeat, I believe we should only strictly type filter return values when applicable PHP code requires a strict type and would error out without proper typing.

function test( string $string ) {}
// This would error if not a stringable type is given. (int) 1 is still acceptable. [] is not.
test( apply_filters( 'example', '' ) );

Filters is_post_type_viewable/is_post_status_viewable apply to no such case; their container functions do, but those can typecast safely.

I'm asking for a second opinion, as well.

[hellofromTonya]

Removing the commit keyword as the patch has been committed.

It's okay for this ticket to stay open for more feedback of 2nd-opinion until Beta 1.

@Cybr another note: strict comparison is more performant than type casting/juggling.

[hellofromTonya]

Beta 1 is happening in less than 30 minutes. I have discussed with @jrf who has been incredibly busy elsewhere. I suspect she will circle back at some point to share more information as to why type casting is not a good solution and why a strict check is preferred here.

Need to reclose this ticket to prepare for Beta 1 release party.

[audrasjb]

Dev note published: ​https://make.wordpress.org/core/2021/12/13/posts-post-types-and-taxonomy-changes-in-wordpress-5-9/