Repository navigation
HTML API: Improve PHPStan types for tag queries and tag names - #15305
westonruter wants to merge 7 commits into
Conversation
The `next_tag()` docs say `tag_name`, `match_offset`, `class_name`, and `tag_closers` may each be `null`, where a `null` `tag_name` means "any tag". `parse_query()` handles that with `isset()`. Without `|null` in the shape, PHPStan rejected documented calls such as the one in `WP_Interactivity_API_Directives_Processor::next_balanced_tag_closer_tag()`, which passes `get_tag()` (`string|null`) as `tag_name`. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 2c14c65e-32a4-4332-8e55-c2d31a16a529
This mirrors the `Query` shape in `WP_HTML_Tag_Processor` and adds the `breadcrumbs` key, which only the HTML Processor supports. Type aliases are not inherited by subclasses, so the shape is defined again here. It is a superset of the parent's shape, so the parameter stays contravariant. `matches_breadcrumbs()` gets the same narrowed element type for its crumbs. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 2c14c65e-32a4-4332-8e55-c2d31a16a529
Shape keys are indented four spaces after the asterisk, and the closing brace goes right after the asterisk, as in the `ScriptModule` and `Connector` aliases. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 2c14c65e-32a4-4332-8e55-c2d31a16a529
A tag name can never be `'0'`, but requiring `non-falsy-string` for query input asks callers to prove more than they usually can. A `'' !== $tag` check narrows to `non-empty-string`, not to `non-falsy-string`. A `'0'` query is harmless: like any unknown tag name, it simply matches nothing. This also matches `class_name` and the attribute names elsewhere in the HTML API, which are `non-empty-string`. The `tag_name` key, the bare-string query form, the `breadcrumbs` key, `$sought_tag_name`, and the `matches_breadcrumbs()` parameter all change. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 2c14c65e-32a4-4332-8e55-c2d31a16a529
Every name the parser reports starts with a letter, `_`, `:`, or `#`, so it is never empty and never `'0'`. PHPStan cannot prove this from the `substr()` in `WP_HTML_Tag_Processor::get_tag()`, so an inline `@var` asserts it there, with the reason. Everything else follows from that: * `get_token_name()` returns either `get_tag()` or a fixed `#…` name, in both processors. * `WP_HTML_Processor::get_tag()` returns a virtual token's `node_name`, so `WP_HTML_Token::$node_name` and its constructor parameter are narrowed too, as is `insert_virtual_node()`, whose callers pass literal tag names. A `next_tag()` call can now pass `get_tag()` as its `tag_name`, as `WP_Interactivity_API_Directives_Processor::next_balanced_tag_closer_tag()` does. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 2c14c65e-32a4-4332-8e55-c2d31a16a529
`get_qualified_tag_name()` returns `get_tag()`, its lowercase form, or a literal SVG or MathML name, so it is never empty and never `'0'`. The breadcrumbs are only ever built from `WP_HTML_Token::$node_name` values and `'HTML'`, and are only changed with `[]`, `array_pop()`, and `array_slice()`, so they form a list of non-falsy names. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 2c14c65e-32a4-4332-8e55-c2d31a16a529
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. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. 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. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the PHPDoc annotation refinements supporting HTML API query typing.
No open issues 🎉
What changed in this PR
This PR refines PHPStan annotations in the HTML API so generated stubs preserve bare tag-name queries, supporting Gutenberg’s type checking. All changes are PHPDoc-only; runtime behavior is unchanged.
Changes:
- Add query type aliases covering array, bare tag-name, and
nullinputs. - Narrow query properties and tag-name outputs with more precise string types.
- Annotate processor breadcrumbs and token node names consistently.
| File | Description |
|---|---|
src/wp-includes/html-api/class-wp-html-token.php |
Annotates token node names as non-falsy strings. |
src/wp-includes/html-api/class-wp-html-tag-processor.php |
Defines query types and refines query properties and tag-name return annotations. |
src/wp-includes/html-api/class-wp-html-processor.php |
Adds breadcrumb-aware query types and refines breadcrumb and tag-name annotations. |
Review effort: Balanced
The `$query` parameter of `next_tag()` is documented as `array|string|null`, but its hash notation only describes the array form, so stubs generated from it (as used by Gutenberg) turned it into an array shape and dropped the bare tag-name form, reporting calls like `next_tag( 'FIGURE' )` as errors. A `Query` type alias now covers the array, bare tag-name, and `null` forms, with each key allowed to be `null` as documented. The HTML Processor defines its own alias adding the `breadcrumbs` key, since type aliases are not inherited by subclasses. Tag names are returned as `non-falsy-string`, since a parsed name is never empty or `'0'`, but accepted as `non-empty-string`, since that is all a `'' !== $tag` check proves and a `'0'` query simply matches nothing. The changes are to PHPDoc only, so there is no runtime change. Developed in #15305. Follow-up to r55203. Props westonruter, dmsnell. See #65817. git-svn-id: https://develop.svn.wordpress.org/trunk@65220 602fd350-edb4-49c9-b593-d223f7449a82
The `$query` parameter of `next_tag()` is documented as `array|string|null`, but its hash notation only describes the array form, so stubs generated from it (as used by Gutenberg) turned it into an array shape and dropped the bare tag-name form, reporting calls like `next_tag( 'FIGURE' )` as errors. A `Query` type alias now covers the array, bare tag-name, and `null` forms, with each key allowed to be `null` as documented. The HTML Processor defines its own alias adding the `breadcrumbs` key, since type aliases are not inherited by subclasses. Tag names are returned as `non-falsy-string`, since a parsed name is never empty or `'0'`, but accepted as `non-empty-string`, since that is all a `'' !== $tag` check proves and a `'0'` query simply matches nothing. The changes are to PHPDoc only, so there is no runtime change. Developed in WordPress/wordpress-develop#15305. Follow-up to r55203. Props westonruter, dmsnell. See #65817. Built from https://develop.svn.wordpress.org/trunk@65220 git-svn-id: http://core.svn.wordpress.org/trunk@64369 1a063a9b-81f0-0310-95a4-ce76da25c4cd
…stubs generator (#86109) * Tools: Report when the WordPress stubs are already generated When the stubs are up to date with the recorded source, `composer phpstan:generate-wordpress-stubs` exited without any output, which looked like it did nothing. It now says which source the stubs were generated from, and that `--force` generates them again. * Regenerate wordpress-stubs from WordPress/wordpress-develop#15305 * Regenerate baselines * Tools: Add a --quiet option to the WordPress stubs generator `composer phpstan` runs the generator before every analysis, so the notice that the stubs are already generated was printed on every run, where it is only noise. It is meant for running `composer phpstan:generate-wordpress-stubs` directly. With `--quiet`, which `composer phpstan` now passes, the notice is left out. Generating the stubs is still reported, since that takes a while and changes what gets analyzed. * Tools: Generate the WordPress stubs from core trunk The typing of HTML API tag queries and tag names is now committed to core in r65220, so the stubs are generated from that trunk commit instead of from the pull request. The baseline no longer has the `wp_tag_cloud()` argument error, which the newer stubs resolve. * Add missing space in comment --------- Co-authored-by: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Co-authored-by: westonruter <westonruter@git.wordpress.org> Co-authored-by: huzaifaalmesbah <huzaifaalmesbah@git.wordpress.org> Co-authored-by: sirreal <jonsurrell@git.wordpress.org>
…stubs generator (#86109) * Tools: Report when the WordPress stubs are already generated When the stubs are up to date with the recorded source, `composer phpstan:generate-wordpress-stubs` exited without any output, which looked like it did nothing. It now says which source the stubs were generated from, and that `--force` generates them again. * Regenerate wordpress-stubs from WordPress/wordpress-develop#15305 * Regenerate baselines * Tools: Add a --quiet option to the WordPress stubs generator `composer phpstan` runs the generator before every analysis, so the notice that the stubs are already generated was printed on every run, where it is only noise. It is meant for running `composer phpstan:generate-wordpress-stubs` directly. With `--quiet`, which `composer phpstan` now passes, the notice is left out. Generating the stubs is still reported, since that takes a while and changes what gets analyzed. * Tools: Generate the WordPress stubs from core trunk The typing of HTML API tag queries and tag names is now committed to core in r65220, so the stubs are generated from that trunk commit instead of from the pull request. The baseline no longer has the `wp_tag_cloud()` argument error, which the newer stubs resolve. * Add missing space in comment --------- Co-authored-by: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Co-authored-by: westonruter <westonruter@git.wordpress.org> Co-authored-by: huzaifaalmesbah <huzaifaalmesbah@git.wordpress.org> Co-authored-by: sirreal <jonsurrell@git.wordpress.org> Source: WordPress/gutenberg@299c5ec
✅ Committed in r65220 (140b182).
This is a dependency for:
next_tag()string query false positives and quiet the stubs generator gutenberg#86109This adds PHPStan types for tag queries and tag names in the HTML API. The changes are to PHPDoc only; runtime behavior is unchanged.
$queryinnext_tag()is documented asarray|string|null, but the hash notation only describes the array form. Generated stubs, such as those Gutenberg uses, turn that hash into an array shape and drop thestringform, sonext_tag( 'FIGURE' )gets reported as an error. See WordPress/gutenberg#86088 (comment). Adding an explicit@phpstan-paramkeeps the bare tag-name form.Changes:
Querytype alias inWP_HTML_Tag_Processorcoverstag_name,match_offset,class_name, andtag_closers, or a bare tag name, ornull. It is used fornext_tag(),parse_query(), and$last_query. Each key may benull, as documented.WP_HTML_Processorgets its ownQueryalias with the extrabreadcrumbskey, since type aliases are not inherited.matches_breadcrumbs()uses the same element type.$sought_tag_name,$sought_class_name, and$sought_match_offsetproperties are narrowed to match.non-empty-string, outputs promisenon-falsy-string. A tag name can never be'0', but a'' !== $tagcheck only narrows tonon-empty-string, and a'0'query is harmless, as it simply matches nothing. This also matchesclass_nameand attribute names, which can legitimately be'0'.get_tag(),get_token_name(), andget_qualified_tag_name()returnnon-falsy-string|null, andget_breadcrumbs()returnslist<non-falsy-string>. One inline@varinWP_HTML_Tag_Processor::get_tag()states why the name read from the HTML is never empty or'0', since PHPStan cannot infer that fromsubstr(). Everything else follows from it, includingWP_HTML_Token::$node_name.With this,
WP_Interactivity_API_Directives_Processor::next_balanced_tag_closer_tag()type-checks when it passesget_tag()as thetag_name. A full level 10 PHPStan run reports no new errors.Related: #15121 (discussion)
Trac ticket: https://core.trac.wordpress.org/ticket/65817
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5 (medium effort)
Used for: Reviewing the initial commit, extending the narrowing to
WP_HTML_Processorand the tag name getters, and checking the results with PHPStan. The direction and type choices were reviewed and decided 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.