Build/Test Tools: Use szepeviktor/phpstan-wordpress for the extensions core does not need to write itself - #13437
Draft
swissspidy wants to merge 6 commits into
Draft
Build/Test Tools: Use szepeviktor/phpstan-wordpress for the extensions core does not need to write itself#13437swissspidy wants to merge 6 commits into
swissspidy wants to merge 6 commits into
Conversation
…dependency. Pulls in the PHPStan extensions maintained for the WordPress ecosystem so that core can register the ones that apply to it, rather than carrying its own copies. Only the package is added here; nothing from it is loaded yet. Its `extension.neon` is deliberately not included, since that bootstraps the `php-stubs/wordpress-stubs` package, which describes the very code core analyzes. Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013QJ9RphuttTR7cS5G4mU21
`check_comment_flood_db()`, `wp_render_block_style_variation_support_styles()` and `twenty_twenty_one_post_classes()` declare fewer parameters than the `$accepted_args` they are registered with, so the extra arguments were passed and discarded. Registering them for the arguments they take is what szepeviktor/phpstan-wordpress's `HookCallbackRule` asks for, and leaves nothing for it to report once it is registered in the next commit. The block support lives in Gutenberg as well, where the same change is due. Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013QJ9RphuttTR7cS5G4mU21
…y to core. Loads three services from szepeviktor/phpstan-wordpress in `base.neon`, chosen by measuring each of the package's extensions against `src/` at rule level 10 and at the level CI enforces: * `ShortcodeAttsDynamicFunctionReturnTypeExtension`, which types `shortcode_atts()` from the defaults passed to it. A docblock cannot express that merge. * `HookCallbackRule`, for its check that `$accepted_args` agrees with the callback's signature. Its objection to an action callback that returns a value is ignored in `phpstan.neon.dist`, with the reason recorded there: core registers such functions on actions deliberately, and WordPress discards the value. * `HookDocsRule`, for its check that the type a hook docblock documents accepts the value the hook passes. That documented type is what `apply_filters()` is typed from. The package's other extensions are not loaded. Its hook docblock resolver, visitor and `apply_filters()` extension are what core's own were adapted from, and core's resolve the "This filter is documented in" reference comments. Its remaining return type extensions each do what a conditional `@phpstan-return` does, which core already carries for `wp_parse_url()`, `wp_slash()` and `stripslashes_from_strings_only()`, and loading them changed nothing measurable there. The README records the disposition of every extension and why. The baselines gain the twenty hook docblocks whose documented type does not accept the value passed, and one `get_posts()` call that the shortcode attributes now show is handed a string where it expects an array of IDs. Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013QJ9RphuttTR7cS5G4mU21
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. |
…accept the value passed.
szepeviktor/phpstan-wordpress's `HookDocsRule`, registered in `base.neon`, reports a hook docblock whose `@param` type does not accept the value the hook is fired with. That documented type is what `apply_filters()` is typed from, so each one misled every caller of the filter as well as every callback written against the documentation.
Where the value was the one the documentation promised all along, the value is corrected: `$bulk` in the Quick Edit filters is a bool rather than the loop counter, `disable_captions` is filtered on `false` rather than an empty string, and the IDs handed to `duplicate_comment_id`, `update_{$meta_type}_meta` and `delete_term_taxonomy` are cast to the documented int rather than passed as the strings the database returns. Where the value is right and the documentation was not, the docblock is corrected: `wp_audio_shortcode` and `wp_video_shortcode` pass an attachment post or null rather than a file, `{$adjacent}_image_link` passes `string|false`, `dashboard_secondary_items` an int, and `blog_details` receives either a `WP_Site` or the plain-object copy that `WP_Site::get_details()` makes deliberately.
Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013QJ9RphuttTR7cS5G4mU21
…inline. `HookCallbackRule` objects to an action callback whose return type is not void. WordPress discards an action callback's return value, and core registers functions that happen to return one on actions as a matter of course, so the objection is not actionable here. Rather than matching the message away in `phpstan.neon.dist`, each of the fifty-two registrations now carries an inline `@phpstan-ignore` saying why, which keeps the decision next to the code it is about and lets a new registration be judged on its own. Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013QJ9RphuttTR7cS5G4mU21
…to core. php-stubs/wordpress-stubs, from which every plugin's static analysis takes its view of core, applies a map of PHPStan types on top of core's docblocks when it generates the stubs: conditional return types, narrowed parameters, templates, purity and a few class-level annotations. szepeviktor/phpstan-wordpress dropped its own return type extensions in favor of that map. Each entry is a type core could carry itself, and one carried here reaches core's own analysis, the stubs, and every plugin from one place. This adds the map's entries to the docblocks of the 290 functions, methods, properties and classes it names, as `@phpstan-` tags. An entry is left out where core already carries a `@phpstan-return` or template for the symbol, where the parameter or return is documented with hash notation that `HashNotationVisitor` derives a shape from, or where the symbol lives in Gutenberg. A few entries were wrong for core and are adjusted or dropped: `add_shortcode()` callbacks receive a string rather than an array when a shortcode has no attributes, `_get_list_table()` accepts a `WP_Screen` as well as a name, `get_tag_regex()` and `wp_get_inline_script_tag()` do return an empty string, `wp_update_comment()` returns the row count `wpdb::update()` does, `size_format()` and `get_tags()` are stated in terms of what core's parameter and `get_terms()` types allow, `get_html_split_regex()`, `block_version()` and `wp_is_uuid()` are not pure, and the `WP_REST_Request` generics report every offset assignment in core as an error and are not adopted. Five calls the new types show to be passing something other than the documented value are corrected: `wp_upload_bits()` and `add_option()` are given the value their deprecated parameter documents, `iframe_header()` is no longer given a deprecated argument, `switch_to_blog()` is given the site ID as an int, and `get_term_to_edit()` documents the `WP_Term` it returns. The baselines lose 53 entries the new types resolve, including every `parameter.defaultValue` and `method.nonObject` error, and gain 8: seven reworded, and one `WP_Term_Query::populate_terms()` assignment on what `get_term()` may return that is worth a look of its own. Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013QJ9RphuttTR7cS5G4mU21
swissspidy
force-pushed
the
try/use-phpstan-ext
branch
from
September 8, 2026 10:01
88f65d1 to
8338da9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explores the question raised on #13433: rather than adapting extensions from szepeviktor/phpstan-wordpress into
tests/phpstan/one at a time, can core depend on the package and load its extensions directly, without the stubs it bundles?Short answer: yes, and this branch does it, but only three of its extensions earn a place in core. The rest are either already outdone by core's own versions, or do what a
@phpstan-returnin the function's docblock does, and a docblock is the better fix because it also types the function for every plugin through the generated stubs. The package has been moving in exactly that direction itself, and the second half of this branch follows it: the types that php-stubs/wordpress-stubs used to apply on top of core's docblocks are now in core's docblocks.Trac ticket: https://core.trac.wordpress.org/ticket/65817
The commits
szepeviktor/phpstan-wordpresstorequire-dev. Itsextension.neonis not included: that file bootstrapsphp-stubs/wordpress-stubs, a declaration of every core function and class, which would declare the code under analysis a second time. Composer still installs the stubs as a transitive dependency; they are not read.$accepted_argson three hook registrations thatHookCallbackRulefound:check_comment_flood_db(),wp_render_block_style_variation_support_styles()(also due in Gutenberg) andtwenty_twenty_one_post_classes()declare fewer parameters than they were registered for.tests/phpstan/base.neon, by class name, with a note on each; the README records what was taken, what was not, and why.HookDocsRulefound documenting a type the hook does not pass. Where the value was what the documentation promised all along, the value is corrected ($bulkbecomes a bool,disable_captionsis filtered onfalse, the IDs passed toduplicate_comment_id,update_{$meta_type}_metaanddelete_term_taxonomyare cast to the documented int). Where the value is right and the documentation was not, the docblock is (wp_audio_shortcodepasses an attachment post or null, not a file;{$adjacent}_image_linkpassesstring|false;blog_detailsreceives either aWP_Siteor the plain-object copyWP_Site::get_details()makes deliberately).HookCallbackRuleobjects to an action callback that returns a value. WordPress discards it, and core registers such functions on actions on purpose 52 times (wp_save_post_revision()onpost_updated,redirect_canonical()ontemplate_redirect, …). Each registration carries an inline@phpstan-ignore return.void (…)saying so, rather than a message pattern inphpstan.neon.dist.functionMap.phpinto core. See below.composer run phpstanon the CI configuration is green at every commit. Net effect on the baselines: 118 entries removed, 8 added (7 of those reworded, 1 new).What is loaded from the package, and what is not
Measured with PHPStan 2.2.13 and phpstan-wordpress 2.0.4 against trunk at r63526. At rule level 10 over the whole
src/tree, trunk reports 29,381 errors; each extension was registered on its own and the report compared. The CI configuration (level 5 plus baselines) was run with the rules as well.ShortcodeAttsDynamicFunctionReturnTypeExtensionmedia.php)shortcode_atts()from its defaults, the same mergewp_parse_args()performs; a docblock cannot express it. The 20 introduced are precise: the video shortcode multiplieswidth, typed360|640|string, andget_posts()is handed anincludestring where it documentsint[].HookCallbackRule$accepted_argsmismatches, 52 action callbacks that return a value)HookDocsRuleapply_filters()is typed from. All 19 were genuine documentation defects and are fixed. Only docblocks written at the call are checked; a "This filter is documented in" reference is not.WpParseArgsDynamicFunctionReturnTypeExtension(szepeviktor/phpstan-wordpress#310, not released yet)$argsas a plain array where #310 leaves the documented type, which rewords 24 messages fromarray<mixed>toarrayand changes nothing else. So once #310 ships, core can register the package's class the way it registersShortcodeAtts…rather than carry a copy.EscSqlDynamicFunctionReturnTypeExtension@phpstan-return.WpSlashDynamicFunctionReturnTypeExtensionwp_slash()docblock already carries a conditional type; the extension keeps array shapes through the call, which@phpstan-return ( T is string ? string : T ), asstripslashes_from_strings_only()is written, would also do.WpParseUrlFunctionDynamicReturnTypeExtensionfalse, and the introduced errors inpluggable.phpare real. That is a one-line docblock fix.SlashitFunctionsDynamicFunctionReturnTypeExtensionstring→non-falsy-string)trailingslashit()now carries@phpstan-return non-falsy-stringfrom the function map, which is the whole effect.NormalizeWhitespace…,StripslashesFromStringsOnly…WpConstantFetchRuleMULTISITE,WP_NETWORK_ADMINand the like where a function exists, but core is where those functions read them.HookDocsVisitor,HookDocBlock,ApplyFiltersDynamicFunctionReturnTypeExtensionAssertWpErrorTypeSpecifyingExtensiontests/phpunitis analyzed, and then@phpstan-assertonWP_UnitTestCase_Base::assertWPError()itself is the way to express it.Adopting the function map
wordpress-stubs'
functionMap.phpnames 361 symbols. Each entry was applied to the corresponding core docblock as@phpstan-param,@phpstan-return,@phpstan-template,@phpstan-pure/-impure,@phpstan-assert-if-true,@phpstan-property-read,@phpstan-typeor@finaltags, formatted the way core already writes them (multi-line conditionals, no leading backslash on global classes).@phpstan-return, template or hash-notation shape (get_posts(),get_terms(),wp_die(),wp_get_upload_dir(),the_title(), …), 20 because the function lives in a Gutenberg block (block_core_*,render_block_core_*), andcheck_admin_referer()/check_ajax_referer()/sanitize_post_field()because the map's parameter narrowing contradicts the conditional return core already has.add_shortcode()callbacks receive a string rather than an array when a shortcode has no attributes;_get_list_table()accepts aWP_Screenas well as a name;get_tag_regex()andwp_get_inline_script_tag()do return'';wp_update_comment()returns the row countwpdb::update()does, not0|1;get_html_split_regex(),block_version()andwp_is_uuid()are not pure;wp_widget_rss_form()'serrorisstring|false;WP_Dependencies::query()'s template was not referenced by a parameter; and theWP_REST_Requestgenerics report every$request['key'] = …in core as an error, so they are not adopted (that one needs rework upstream before it can be carried here).wp_upload_bits()andadd_option()are given the value their deprecated parameter documents,iframe_header()is no longer given a deprecated argument,switch_to_blog()is givenWP_Site::$blog_idas an int, andget_term_to_edit()documents theWP_Termit returns rather thanint|string.parameter.defaultValueandmethod.nonObjecterror, at the cost of one new entry, aWP_Term_Query::populate_terms()assignment on whatget_term()may return.With these in core, the corresponding entries in wordpress-stubs' map become redundant once the stubs are regenerated from a release carrying them, and a plugin analyzed with the stubs sees the same types core's own analysis does.
Two things worth knowing
The package's branches. phpstan-wordpress develops on two lines. Its
2.xbranch (v2.0.4, PHPStan^2.0) is the one core can depend on. Itsmasterbranch is the PHPStan 1.x line, and that is where szepeviktor/phpstan-wordpress#309, thewp_parse_args()extension, was first merged; szepeviktor/phpstan-wordpress#310 ports it to2.x, and the measurement above shows it does for core exactly what #13433's version does.Where the package's other extensions went. The 1.x line had extensions for
get_post(),get_terms(),current_time(),wp_die(),is_wp_error()and many more. The2.xbranch removed them in favor of the function map, which this branch in turn moves into core.What this suggests for upstream
HookCallbackRulereports under PHPStan's own identifiers (arguments.count,return.void,return.missing), andHookDocsRuleunderparameter.phpDocType. Core's baselines are split by identifier, so these share files with PHPStan's own errors. Identifiers of its own, asWpConstantFetchRulehas, would fix that; a parameter to disable the action-return check would be nicer still.HookDocBlockcould resolve "This filter is documented in" reference comments the way core's does. Core's implementation is intests/phpstan/HookDocBlock.php.WP_REST_Requestgenerics in the function map do not survive contact with code that assigns throughArrayAccess.Follow-ups in core
WpParseArgsDynamicFunctionReturnTypeExtensionfrom the package once a release carries Add a dynamic return type extension for wp_parse_args() (2.x) szepeviktor/phpstan-wordpress#310, in place of the copy proposed in Type wp_parse_args() from the arguments it was called with #13433.falseto the component form ofwp_parse_url()'s@phpstan-return.get_posts()call in the gallery shortcode passesincludeas a string.WP_Term_Query::populate_terms()assigns a property on whateverget_term()returns,WP_Errorincluded.$accepted_argschange toblock-style-variations.phpis also due in Gutenberg, where that file lives, as are the map entries forblock_core_*functions.Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Fable 5.1
Used for: The survey of both branches of phpstan-wordpress and of the wordpress-stubs function map, the per-extension measurements above, the script that applied the function map to core's docblocks and the review of its output, the hook docblock fixes, the configuration and README changes, and drafting this description. Directed and reviewed by me.
Session transcript: https://claude.ai/code/session_013QJ9RphuttTR7cS5G4mU21
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.
🤖 Generated with Claude Code
https://claude.ai/code/session_013QJ9RphuttTR7cS5G4mU21