Rebuild post type meta capabilities when unregistering a post type - #13342
Rebuild post type meta capabilities when unregistering a post type#13342westonruter wants to merge 11 commits into
Conversation
The global `$post_type_meta_caps` registry is keyed by custom capability name, so a single entry may be owed to any number of registered post types. `WP_Post_Type::remove_rewrite_rules()` removed entries by subtracting every value of the unregistered post type's own `$cap` object, which: * deleted mappings that other, still-registered post types sharing a capability type continue to depend on; * deleted entries for post types registered with `map_meta_cap` set to `false`, which never stored any to begin with; * treated primitive capabilities as meta capabilities, since only the read, delete and edit capabilities are ever stored. Once a mapping is gone, `map_meta_cap()` falls through to its `default:` branch and returns the meta capability verbatim rather than mapping it down to primitives, so a post's own author can lose edit access to it. Subtraction cannot work against a shared registry, as nothing records which post types an entry is owed to. Replace it with `_rebuild_post_type_meta_capabilities()`, which rebuilds the registry from the post types that remain, and call that from `unregister_post_type()` once the post type has been removed from `$wp_post_types`. Removing the loop also empties the `foreach.nonIterable` PHPStan baseline, so that file and its `includes` entry are deleted. The `@ticket` annotations on the new tests are placeholders pending the Trac ticket number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When introduced in 3.1 the function held its list in a `static $meta_caps`
and returned it when called with no arguments:
function _post_type_meta_capabilities( $capabilities = null ) {
static $meta_caps = array();
if ( null === $capabilities )
return $meta_caps;
That branch went away when the list moved to the `$post_type_meta_caps`
global, but the `null` default and the "Stores or returns" summary were
left behind. Calling the function with no arguments has since only produced
a `foreach()` warning, so default the parameter to an empty array, drop the
stale half of the summary, and declare the `void` return.
Also give the `$post_type_meta_caps` global a value type, matching how it is
documented on `_rebuild_post_type_meta_capabilities()`. Its keys are custom
capability names and its values core meta capability names, so it is an
`array<string, string>` rather than a bare `array`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The function's `foreach ( $capabilities as $core => $custom )` shows what it expects: a map of core meta capability name to the custom capability name it is registered under. `get_post_type_capabilities()` passes exactly that, a merge of `$default_capabilities` and, optionally, `$default_capabilities_for_mapping`, both of which are string to string. The `string[]` annotation left the keys untyped and so said none of this. Document the parameter as `array<string, string>`, matching the `$post_type_meta_caps` global it populates. `_rebuild_post_type_meta_capabilities()` sources its map from `WP_Post_Type::$cap`. That is a `stdClass`, which carries no property types, and PHPStan types `get_object_vars()` on any object as `array<mixed>` regardless of how the object itself is documented. The call is therefore reported as an `argument.type` error at higher rule levels even though the values are always strings at runtime. Accept that rather than narrowing the map at runtime or relaxing what the function documents; typing `$cap` well enough for the analyser to follow is a separate piece of work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e of get_post_type_object() to fix PHPStan errors Fixes: Cannot access property on WP_Post_Type|null.
`_rebuild_post_type_meta_capabilities()` had a single caller and existed only to hold a few lines that read no state the caller did not already have. Move its body into `unregister_post_type()`, directly after the post type is removed from `$wp_post_types`, and drop the function. The loop variable is named `$registered_post_type` because `$post_type_object` is already bound to the post type being unregistered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
r36316 moved the list of meta capabilities from a `static $meta_caps` into
the `$post_type_meta_caps` global, which removed the branch that returned
the list when the function was called without arguments:
static $meta_caps = array();
if ( null === $capabilities )
return $meta_caps;
The docblock was never updated to say so, and until this branch changed it
the summary still described the function as returning that list. Add the
missing `@since 4.5.0` entry for that, along with one for the parameter
default changing from `null` to an empty array.
Also reference `map_meta_cap()` with an inline `@see`, since that is what
consumes the stored list.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The method no longer touches the meta capability list, so record that with a `@since` entry and point at `unregister_post_type()`, which now rebuilds the list from the post types that remain. Described as a rebuild rather than a removal, since the work did not simply move to the caller. Nothing unsets entries any more, and a reader sent looking for one would not find it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The four tests covering the meta capability rebuild carried a placeholder `@ticket` number while the ticket was still being drafted. Point them at the ticket that was filed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| if ( $registered_post_type->map_meta_cap ) { | ||
| _post_type_meta_capabilities( get_object_vars( $registered_post_type->cap ) ); | ||
| } | ||
| } |
There was a problem hiding this comment.
what are the implications here for registering a post type? does this go both-ways, where something gets potentially overwritten that shouldn’t be?
There was a problem hiding this comment.
Looking at how this works…
When register_post_type() is called during init, it constructs a WP_Post_Type. The constructor will call WP_Post_Type::set_props() which in turn calls get_post_type_capabilities(). This get_post_type_capabilities() function will also call _post_type_meta_capabilities() if map_meta_cap is true, which results in $post_type_meta_caps being populated. So the end result with just register_post_type() being called multiple times is the $post_type_meta_caps being rebuilt from scratch in the same way as this is being done here when one of the post types is unregistered.
unregister_post_type()removes entries from the global$post_type_meta_capslist by subtracting every value of the post type's own$capobject. That list is keyed by custom capability name and is shared by every post type resolving to the same name, so the subtraction deletes mappings that other, still-registered post types depend on.Once a mapping is gone,
map_meta_cap()stops translating the meta capability into contextual primitives and returns it verbatim. The ownership and post-status checks (edit_others_*,edit_published_*,edit_private_*) do not become stricter, they disappear from the result entirely, so a post's own author can lose edit access to it.The full analysis, including three distinct defects and a reproduction script, is in Core-66008.
The fix
Subtraction cannot work against a shared list: an entry may be owed to any number of registered post types, and nothing records which ones. Rather than reference-count the entries, the list is rebuilt from the post types that remain.
The cleanup is removed from
WP_Post_Type::remove_rewrite_rules()entirely, leaving that method to do only what its name says, and the rebuild happens inunregister_post_type()once the post type has been removed from$wp_post_types. Placement matters:remove_rewrite_rules()is called several lines before thatunset(), so a rebuild performed from inside it would still see the post type being removed.Rebuilding rather than subtracting fixes all three defects at once, because it inherits the rules of
_post_type_meta_capabilities()instead of trying to invert them. Entries are written only for post types withmap_meta_cap => true, only for the three meta capabilities, and an entry shared by several post types survives as long as any one of them is still registered.Removing the loop also empties
tests/phpstan/baselines/foreach.nonIterable.neon, so that baseline and itsincludesentry inphpstan.neon.distare deleted here too. This supersedes #13079, which proposed silencing that baseline entry with a type cast.Tests
Four tests in
tests/phpunit/tests/post/types.php, one per defect plus a functional one. Each was confirmed to fail before the fix, on an assertion after the unregistration rather than during setup:--group post(951 tests) and--group capabilities(789 tests) both pass.Notes for review
The
$capabilitiesmap handed to_post_type_meta_capabilities()comes fromWP_Post_Type::$cap, a barestdClassthat carries no property types. PHPStan typesget_object_vars()on any object asarray<mixed>regardless of how the object is documented, so the call reports anargument.typeerror at higher rule levels even though the values are always strings at runtime. Narrowing the map at runtime or relaxing what the function documents would both be worse; typing$capwell enough for the analyzer to follow is separate work, tracked in Core-64898.WP_Post_Type::remove_rewrite_rules()is public, so moving the cleanup out of it is technically a behavior change for any caller invoking it directly. A search of public GitHub forremove_rewrite_rules()returns only vendored copies of core's ownpost.php,taxonomy.phpand the two class files, with no third-party call sites at all.Some accompanying documentation fixes to
_post_type_meta_capabilities(), each in its own commit: r36316 removed the branch that returned the list when the function was called with no arguments, but left behind thenulldefault and a summary describing that return value.Trac ticket: Core-66008
Trac ticket: Core-65817
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Investigating the defect and its history, drafting the fix and its tests, and drafting the Trac ticket. The approach, the final implementation and the tests were reviewed, revised and are owned by me.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.