Build/Test Tools: Statically verify that hook documentation and arguments stay in sync#12022
Conversation
…cks in PHPStan. Add a PHPStan dynamic return type extension, ported and adapted from szepeviktor/phpstan-wordpress, that types the return value of apply_filters() and its variants (apply_filters_deprecated(), apply_filters_ref_array()) from the `@param` type documented in the docblock preceding the call, rather than falling back to `mixed`. Also supports WordPress core's "documented elsewhere" convention: when a hook is invoked under a `/** This filter is documented in <file> */` reference comment, the canonical docblock is looked up from the referenced file by matching the hook name and used as if written at the call site. Wired into tests/phpstan/base.neon so it applies to both the level-0 dist configuration and the local level-10 override. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document the hook invocations that were missing inline documentation, using the standard "This filter is documented in <file>" reference comments per the WordPress inline documentation standards: - the_permalink in Twenty Thirteen and Twenty Fifteen link-format helpers. - widget_title in the Twenty Fourteen ephemera widget. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- In WP_XMLRPC_Server::set_is_enabled(), the back-compat shim applies the pre_option_enable_xmlrpc and option_enable_xmlrpc filters directly. Add the "This filter is documented in wp-includes/option.php" references and pass the option name (and default value) so the calls match the documented signatures of those dynamic option filters. - Move the wp_pre_execute_ability docblock in WP_Ability so it immediately precedes the apply_filters() call, rather than being separated from it by the sentinel assignment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…elsewhere" references in PHPStan.
Build on the apply_filters() return-type extension with hook-documentation
support:
- Add HookDocumentationRule, reporting any apply_filters()/do_action() (and
variants) invocation that is not preceded by a documenting docblock or a valid
"This filter is documented in <file>" reference. References whose target file
is missing, or that do not actually document the hook, are reported too. Calls
whose hook name carries no literal text (e.g. the generic *_ref_array( $hook,
$args ) forwarders) are exempt.
- Resolve the "documented elsewhere" convention when typing apply_filters(),
matching dynamic canonical hook names (e.g. "{$type}_template_hierarchy")
against the literal name used at the referencing call site.
- Rework HookDocsVisitor to follow the documentation standard: a docblock is
captured from any node and propagated to the hook call it documents, including
hooks inside a multi-line condition or used as an array element value, and is
cleared at statement boundaries.
- Register the rule and exclude the Gutenberg-generated src/wp-includes/build
tree from analysis.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The the_permalink filter is documented (in wp-includes/link-template.php) as receiving the permalink and the post, but the Twenty Eleven, Twenty Thirteen, and Twenty Fifteen link-format helpers fired it with only the permalink. Pass get_post() as the second argument so callbacks receive the documented post argument, matching the canonical the_permalink invocation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the missing parameter name to the `@param` tag documenting several theme-specific filters (twentyeleven_author_bio_avatar_size, twentyeleven_status_avatar, twentyeleven_attachment_size, tag_archive_meta, twentyten_header_image_width, twentyten_header_image_height, and twentytwentyone_html_classes), which previously read e.g. "@param int The..." with no `$variable`. Also correct the Twenty Eleven Ephemera widget's widget_title reference, which pointed at the obsolete wp-includes/default-widgets.php, to the canonical wp-includes/widgets/class-wp-widget-pages.php. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion. When updating an existing autosave, WP_REST_Autosaves_Controller fired wp_creating_autosave with only the autosave array, omitting the $is_update argument added in 6.4.0. This call path updates an existing autosave (it sets the existing autosave ID and calls wp_update_post()), mirroring the update branch in wp_create_post_autosave(), so pass true to match the documented two-argument signature and give callbacks the expected $is_update value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The https_local_ssl_verify filter is documented (in class-wp-http-streams.php / class-wp-http-curl.php) as receiving the SSL-verify value and the request URL (the $url parameter added in 5.1.0), but several loopback callers fired it with only the value. Pass the loopback request URL as the second argument so filters can vary SSL verification per URL, matching the canonical invocations. Affects the loopback requests in WP_Automatic_Updater, WP_Site_Health (REST availability, can_perform_loopback, check_for_page_caching), the plugin/ theme editor scrape in wp_edit_theme_plugin_file(), and spawn_cron(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… updates. The wp_update_nav_menu action is documented (in wp-includes/nav-menu.php) as receiving the menu ID and an array of menu data, but the two sites that fire it after updating only the "auto add pages" option -- wp_nav_menu_update_menu_items() and WP_REST_Menus_Controller::handle_auto_add() -- passed only the menu ID. These paths have no menu-data array, so pass an empty array() to satisfy the documented two-argument signature and avoid an ArgumentCountError for callbacks registered with two arguments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… scraping.
The error_scrape handler re-fires the activate_{$plugin} action to reproduce a
fatal that occurred during activation, but passed no arguments. The action is
documented as receiving a bool $network_wide; relying on do_action()'s empty
argument padding meant a callback type-hinting the parameter would receive an
empty string, which fails under strict types (and for an int parameter would
fatal even in coercive mode). Pass false explicitly, matching the single-site
activation this admin screen reproduces.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GlobalDocBlockVisitor previously injected @var types from @global tags only for global statements inside a function or method body. WordPress also documents file-scope globals with an @global docblock on the global statement itself (see wp-admin/edit.php and other admin entry points), but PHPStan resolved those as mixed. Extend the visitor to also read @global tags from the docblock attached directly to a file-scope global statement, merging them with any tags inherited from an enclosing function (statement-level tags take precedence). Function-scope behavior and handwritten @var precedence are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The install_plugins_{$tab} action is documented as receiving the current page
number, and the canonical invocation passes $paged. The upload-tab form
rendered on non-upload screens fired install_plugins_upload with no arguments,
so pass $paged to match the documented signature.
Document the $tab and $paged globals (populated by
WP_Plugin_Install_List_Table::prepare_items()) with an @global docblock so they
resolve throughout the file.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Several term hooks are documented as receiving an $args array (added in 6.1.0) as their final argument, but some invocations omitted it: - In wp_insert_term(), the edit_terms and edited_terms actions fired on the empty-slug path are now passed the function's $args. - In wp_update_term(), the term_id_filter (documented elsewhere in this file) is now passed $args, matching its canonical invocation. - In the _update_post_term_count() and _update_generic_term_count() callbacks, the edit_term_taxonomy and edited_term_taxonomy actions are fired while only recounting, where no wp_update_term() args exist, so an empty array() is passed to satisfy the documented three-argument signature. This gives callbacks the documented argument and avoids an ArgumentCountError for callbacks registered with three arguments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add HookParamCountRule, which reports any apply_filters()/do_action() (and
variant) invocation that passes a different number of arguments than its
documentation describes. WordPress passes a hook's arguments straight through to
its callbacks, so a documented-but-unpassed parameter is a real defect: a
callback registered for the documented argument count triggers an
ArgumentCountError (or a TypeError for a typed parameter) when the hook fires
with fewer arguments.
The provided count is read from the call: positional arguments for the variadic
apply_filters()/do_action(), or the size of the array argument for the
*_ref_array()/*_deprecated() variants, using PHPStan's inferred constant array
size so literals and typed variables are both handled. Calls whose count cannot
be determined statically are skipped.
To support this, HookDocBlock gains getDocumentedParamCount() (which resolves a
hook's documented parameters inline or via a "documented in" reference, without
the return-type extension's fallback to the bare reference comment). Reference
resolution now walks up the directory tree to locate the named file from any
location instead of assuming a wp-includes/wp-admin root, and matches dynamic
hook names (e.g. "{$type}_template_hierarchy") between the referencing site and
the canonical invocation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n network screens.
The handle_network_bulk_actions-{$screen} filter is documented as receiving a
fourth $site_id argument, which the per-site Network Admin screens (site-themes,
site-users, sites) pass. The network-wide Themes and Users bulk-action screens
omitted it. These screens act across the whole network rather than a single
site, so pass 0 -- the same int "no specific site" value site-themes.php uses
when no id is in the request -- to match the documented four-argument signature.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ent and the_excerpt. Both the_content and the_excerpt are documented (in wp-includes/post-template.php) as receiving only the content/excerpt string, but two callers passed an extra argument: - do_trackbacks() passed $post->ID to the_content. - WP_REST_Revisions_Controller::prepare_excerpt_response() passed $post to the_excerpt. Neither argument is part of the documented signature, and callbacks attached to these filters do not receive it. Drop the extra argument so the invocations match the documented one-parameter signature. The $post parameter of prepare_excerpt_response() is left in place as it is part of the method's established (overridable) signature. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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. |
There was a problem hiding this comment.
Pull request overview
This PR adds PHPStan support for validating WordPress hook documentation against hook invocations, then updates core and bundled theme call sites/docblocks to satisfy the new checks.
Changes:
- Adds PHPStan hook docblock resolution, return-type inference for filters, documentation validation, and hook argument-count validation.
- Extends global docblock handling for file-scope globals used by PHPStan.
- Fixes discovered hook argument mismatches and malformed/missing hook documentation across core and bundled themes.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/phpstan/HookParamCountRule.php |
Adds rule comparing provided hook arguments with documented parameters. |
tests/phpstan/HookDocumentationRule.php |
Adds rule requiring hook docs or valid documented-elsewhere references. |
tests/phpstan/HookDocsVisitor.php |
Adds visitor that propagates hook docblocks to nested hook calls. |
tests/phpstan/HookDocBlock.php |
Adds hook docblock/reference resolution and dynamic hook-name matching. |
tests/phpstan/GlobalDocBlockVisitor.php |
Supports file-scope @global annotations on global statements. |
tests/phpstan/base.neon |
Registers new PHPStan services/rules and excludes generated build output. |
tests/phpstan/ApplyFiltersDynamicFunctionReturnTypeExtension.php |
Infers filter return types from documented first hook parameter. |
src/wp-includes/taxonomy.php |
Passes documented taxonomy hook arguments. |
src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php |
Aligns the_excerpt filter call with documented signature. |
src/wp-includes/rest-api/endpoints/class-wp-rest-menus-controller.php |
Passes documented menu data argument. |
src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php |
Passes documented autosave update flag. |
src/wp-includes/media.php |
Fixes PHPDoc array types for image editor output format. |
src/wp-includes/cron.php |
Passes cron URL to https_local_ssl_verify. |
src/wp-includes/comment.php |
Aligns the_content filter call with documented signature. |
src/wp-includes/class-wp-xmlrpc-server.php |
Adds option hook references and documented arguments. |
src/wp-includes/abilities-api/class-wp-ability.php |
Moves sentinel creation before the hook docblock. |
src/wp-content/themes/twentytwentyone/functions.php |
Adds missing hook parameter variable name. |
src/wp-content/themes/twentythirteen/functions.php |
Documents and updates the_permalink filter call. |
src/wp-content/themes/twentyten/functions.php |
Adds missing hook parameter variable names. |
src/wp-content/themes/twentyfourteen/inc/widgets.php |
Adds documented reference for widget_title. |
src/wp-content/themes/twentyfifteen/inc/template-tags.php |
Documents and updates the_permalink filter call. |
src/wp-content/themes/twentyeleven/tag.php |
Adds missing hook parameter variable name. |
src/wp-content/themes/twentyeleven/inc/widgets.php |
Updates stale widget_title documentation reference. |
src/wp-content/themes/twentyeleven/image.php |
Adds missing hook parameter variable name. |
src/wp-content/themes/twentyeleven/functions.php |
Updates the_permalink filter call arguments. |
src/wp-content/themes/twentyeleven/content-status.php |
Adds missing hook parameter variable name. |
src/wp-content/themes/twentyeleven/author.php |
Adds missing hook parameter variable name. |
src/wp-admin/plugins.php |
Passes documented activation hook argument. |
src/wp-admin/plugin-install.php |
Adds file-scope globals and passes paged argument. |
src/wp-admin/network/users.php |
Passes documented network bulk action site ID placeholder. |
src/wp-admin/network/themes.php |
Passes documented network bulk action site ID placeholder. |
src/wp-admin/includes/nav-menu.php |
Passes documented menu data argument. |
src/wp-admin/includes/file.php |
Passes final request URL to SSL verification filter. |
src/wp-admin/includes/class-wp-site-health.php |
Passes request URLs to SSL verification filter. |
src/wp-admin/includes/class-wp-automatic-updater.php |
Passes loopback URL to SSL verification filter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
What an ocean of bugs revealed! |
HookDocsVisitor cleared the tracked docblock only at statement boundaries, so a docblock attached to an array item (or another non-statement expression) leaked to following sibling items. That could make an undocumented hook in a later array element appear documented by an earlier element's docblock, a false negative for the hook documentation and argument-count rules. Track the docblock on a stack and restore the previous value when the introducing node is left, so a docblock applies only to that node's descendants while a statement-level docblock still flows into its nested expressions. This surfaced one genuinely undocumented-looking call: in get_inline_data() the editable_slug reference comment sat inside an echo concatenation rather than before a statement. Hoist the filtered value into a documented $editable_slug variable before the echo so the reference precedes the call, with identical output. Co-Authored-By: Copilot <198982749+Copilot@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (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. |
| plugin_sandbox_scrape( $plugin ); | ||
| /** This action is documented in wp-admin/includes/plugin.php */ | ||
| do_action( "activate_{$plugin}" ); | ||
| do_action( "activate_{$plugin}", false ); |
There was a problem hiding this comment.
Good catch — you're right. The activation request passes is_network_admin() as $network_wide (activate_plugin( $plugin, …, is_network_admin() ) in the activate case), so the error-scrape re-trigger should reproduce that same network context rather than hardcode false. Fixed in b0f6210 to pass is_network_admin().
(Reply from Claude, acting on behalf of @westonruter.)
|
|
||
| /** This action is documented in wp-admin/network/site-themes.php */ | ||
| $referer = apply_filters( 'handle_network_bulk_actions-' . get_current_screen()->id, $referer, $action, $themes ); // phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores | ||
| $referer = apply_filters( 'handle_network_bulk_actions-' . get_current_screen()->id, $referer, $action, $themes, 0 ); // phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores |
There was a problem hiding this comment.
Thanks, but I don't think a separate three-argument variant is feasible here. handle_network_bulk_actions-{$screen} is a single dynamic hook with one canonical docblock (in wp-admin/network/site-themes.php), and a callback registered with accepted_args = 4 runs on every screen that fires it. If this screen fired only three arguments, such a callback would receive null/error for the fourth — exactly the ArgumentCountError/TypeError risk this rule is meant to catch. So all call sites need to pass the documented four arguments.
Since a network-wide screen has no single-site context, 0 is the appropriate value: it's the same "no specific site" sentinel that site-themes.php itself falls back to ($id = isset( $_REQUEST['id'] ) ? (int) $_REQUEST['id'] : 0), so four-argument callbacks already see 0 there in the no-site case. Keeping 0.
(Reply from Claude, acting on behalf of @westonruter.)
|
|
||
| /** This action is documented in wp-admin/network/site-themes.php */ | ||
| $sendback = apply_filters( 'handle_network_bulk_actions-' . get_current_screen()->id, $sendback, $doaction, $user_ids ); // phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores | ||
| $sendback = apply_filters( 'handle_network_bulk_actions-' . get_current_screen()->id, $sendback, $doaction, $user_ids, 0 ); // phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores |
There was a problem hiding this comment.
Same reasoning as the Network Themes screen above: this is the same single dynamic hook (handle_network_bulk_actions-{$screen}) with one shared canonical docblock, so every site that fires it must pass the documented four arguments, otherwise a callback registered with accepted_args = 4 breaks. This network-wide users screen has no single-site context, so it passes 0 — the "no specific site" sentinel also used by site-themes.php when no id is present. Keeping 0.
(Reply from Claude, acting on behalf of @westonruter.)
…n error scraping. Co-Authored-By: Copilot <198982749+Copilot@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Level 10 error impact (trunk vs. this branch)I ran the full level-10 analysis on
|
| Error identifier | Before | After | Δ |
|---|---|---|---|
argument.type |
12,279 | 11,276 | −1,003 |
return.type |
1,483 | 765 | −718 |
offsetAccess.nonOffsetAccessible |
4,839 | 4,171 | −668 |
property.nonObject |
3,752 | 3,571 | −181 |
binaryOp.invalid |
1,305 | 1,161 | −144 |
encapsedStringPart.nonString |
715 | 572 | −143 |
echo.nonString |
311 | 205 | −106 |
foreach.nonIterable |
578 | 507 | −71 |
method.nonObject |
662 | 613 | −49 |
assign.propertyType |
277 | 237 | −40 |
offsetAccess.invalidOffset |
733 | 699 | −34 |
cast.int |
830 | 801 | −29 |
missingType.return |
2,340 | 2,315 | −25 |
assignOp.invalid |
151 | 135 | −16 |
phpDoc.parseError |
5 | 0 | −5 |
cast.string |
89 | 84 | −5 |
parameterByRef.type |
15 | 12 | −3 |
property.nameNotString |
27 | 26 | −1 |
offsetAccess.nonArray |
7 | 6 | −1 |
missingType.iterableValue |
3,377 | 3,376 | −1 |
array.invalidKey |
18 | 17 | −1 |
function.alreadyNarrowedType |
17 | 18 | +1 |
offsetAccess.notFound |
120 | 122 | +2 |
variable.undefined |
540 | 547 | +7 |
return.unusedType |
9 | 16 | +7 |
property.notFound |
757 | 777 | +20 |
| Total (all identifiers) | 36,241 | 33,034 | −3,207 |
Summary
- The reduction is disproportionately large relative to the code change because the
apply_filters()return-type extension is the dominant lever: typing filtered values (instead ofmixed) cascades through every downstream consumer. The three biggest drops —argument.type(−1,003),return.type(−718), andoffsetAccess.nonOffsetAccessible(−668) — are exactly this: once a filtered value has a real type, PHPStan stops flagging it as passed to typed params, returned from typed functions, or indexed as a possible non-array. - The small regressions (+37 across five identifiers, e.g.
property.notFound+20,variable.undefined+7,return.unusedType+7) are the flip side of more precise typing: giving a value a concrete type occasionally exposes a more-specific complaint thatmixedpreviously masked. These are precision gains rather than new defects, and are far outweighed by the ~3,244 genuine reductions. - The new rules (
wordpress.hookDocMissing,wordpress.hookParamCountMismatch, and the "documented in" reference checks) contribute 0 errors in the "after" run — every violation they surfaced is fixed in this branch — so they add enforcement going forward without leaving standing errors.phpDoc.parseErroralso drops to 0 from the malformed@paramdocblock fixes.
Methodology: both runs used the same local phpstan.neon (level 10, treatPhpDocTypesAsCertain: false); the only difference is the branch's code and the registered rules. The "before" run is trunk (where the new rule files and registrations don't exist, so wordpress.* identifiers are absent by construction).
(Analysis performed and posted by Claude, on behalf of @westonruter.)
|
I don't intend to commit this all in one go. The PHPStan extensions would be in one commit, and then the other fixes would be committed generally on a per-component basis. |
|
Or rather, the other fixes would be committed first, and then the PHPStan extensions would be committed. |
|
@justlevine You may want to review this as well. |
Teaches PHPStan to read WordPress's existing hook docblocks so that hook documentation, "documented elsewhere" references, and the arguments passed to hooks are verified automatically instead of by manual review — then fixes the issues this surfaces in core.
See the Trac ticket for the full problem framing. In short:
apply_filters()was typed asmixed, discarding the type already declared in the hook's docblock./** This filter is documented in <file> */reference comments were never verified, so they could point at a missing file or a file that no longer documents the hook.ArgumentCountError(or aTypeErrorfor a typed parameter) in a callback registered for the documented count; passing too many silently drops the extra argument and misleads the documentation.@paramtags went unflagged.What's in this PR
Tooling (in
tests/phpstan/, wired in viabase.neonso it applies to both the level‑0phpstan.neon.distand the local level‑10 config):apply_filters()from the first documented@param. Adapted fromszepeviktor/phpstan-wordpress.{$type}_template_hierarchy).@globaldocblock resolution inGlobalDocBlockVisitor. This builds on top of Build/Test Tools: Honor@globaldocblock tags in PHPStan analysis #11692.Core fixes surfaced by the tooling:
edit_terms/edited_terms/edit_term_taxonomy/edited_term_taxonomy/term_id_filter,https_local_ssl_verify,wp_update_nav_menu,wp_creating_autosave,install_plugins_upload,activate_{$plugin}, the networkhandle_network_bulk_actions-{$screen}screens, and the XML‑RPCpre_option_enable_xmlrpc/option_enable_xmlrpcshim.the_contentindo_trackbacks()andthe_excerptinWP_REST_Revisions_Controller.@paramtags and a stale reference comment in bundled themes, andthe_permalinkcalls now passing the documentedget_post()argument.All 38 PHPStan errors identified by the new extensions and fixed in this PR
(Note: there is an existing PHPStan error in
trunkwhich is being fixed separately in #12020.)Notes for reviewers
0is passed to satisfy the signature.src/wp-includes/buildtree is excluded from analysis; the corresponding fix lives in the Gutenbergwp-buildpage templates and is handled separately._mb_ord()incompat.php).Trac ticket: https://core.trac.wordpress.org/ticket/65376
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.8
Used for: Drafting the PHPStan extension and rules, locating and fixing the hook issues across core, and ecosystem impact research. All changes were reviewed and directed by me.