Repository navigation
PHPStan: Resolve always-true type checks - #86104
Conversation
- Drop the `method_exists()` check for `WP_Interactivity_API::add_client_navigation_support_to_script_module()` in `lib/client-assets.php`. The method was added in WordPress 6.9, and the plugin requires 7.0. - Document the Navigation block's `$layout` parameter as `mixed`. It comes from block attributes, so the existing `is_array()` guards are needed. - Ignore the `method_exists()` check for `WP_Theme_JSON_Gutenberg::get_viewport_media_queries()` in the Navigation block, which is needed when the block library runs in WordPress Core, where that class is not defined. Remove the 4 resolved entries from the PHPStan baseline. See WordPress#86088.
🤖 PR meta 🤖🎉 PropsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. 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:
Updated as activity occurs, without notifying anyone named here. Add the |
…hecks # Conflicts: # tools/phpstan/baseline.neon
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the PHPStan cleanup.
No open issues 🎉
What changed in this PR
Addresses four baselined PHPStan always-true type-check errors as part of #86088, preserving intended runtime behavior.
Changes:
- Removes an obsolete Interactivity API method-existence check.
- Corrects the Navigation layout parameter’s PHPDoc type while retaining runtime guards.
- Moves an intentional compatibility check to a targeted PHPStan ignore and removes the corresponding baseline entries.
| File | Description |
|---|---|
tools/phpstan/baseline.neon |
Removes entries covering four addressed errors. |
phpstan.neon.dist |
Adds a targeted ignore for the Navigation compatibility check. |
packages/block-library/src/navigation/index.php |
Changes the layout parameter’s documented type from array to mixed. |
lib/client-assets.php |
Removes the obsolete method-existence guard for client navigation support. |
Review effort: Balanced
| // Add client navigation support to block library modules. | ||
| foreach ( $modules as $module ) { | ||
| if ( str_starts_with( $module['id'], '@wordpress/block-library' ) && method_exists( 'WP_Interactivity_API', 'add_client_navigation_support_to_script_module' ) ) { | ||
| if ( str_starts_with( $module['id'], '@wordpress/block-library' ) ) { |
There was a problem hiding this comment.
This makes sense because Gutenberg requires WP 7.0, and WP_Interactivity_API::add_client_navigation_support_to_script_module() was introduced in WP 6.9.
The only caller of `block_core_navigation_get_layout_custom_property_declarations()` passes the `array_replace()` of two values it has already checked with `is_array()`, so `$layout` is always an array. Document it as `array<string, mixed>` rather than `mixed`, and remove the three redundant `is_array( $layout )` guards. Each offset is still read with `??`, and `justifyContent` is still checked with `is_string()`, so unexpected values fall back to the defaults as before. The return type is narrowed to `array<string, string>`. At the call site, the merged layout gets an inline `@var array<string, mixed>`, since PHPStan types an array narrowed from an attribute as `array<mixed>`, while the layout attribute is a JSON object with string keys. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
Every key `block_core_navigation_get_layout_custom_property_declarations()` returns is fixed, and each value comes from a small set of literals, so document them in a `@phpstan-return` array shape. For PHPStan to verify the `--navigation-layout-align` values, the vertical alignment is now looked up in its own map rather than chosen with `in_array()`. PHPStan does not tie the `in_array()` check to the `$justification` it picks, so it inferred that `space-between` could be returned. The behavior is unchanged: `center` and `right` map to `center` and `flex-end`, and everything else falls back to `flex-start`. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
Annotate the `$modules` loaded in `gutenberg_define_interactivity_modules_support()` with the shape that wp-build's `generateModuleRegistrationPhp()` writes for each entry: an `id`, `path`, and `asset`, plus `min_only` set to `true` for modules that only ship minified, such as WASM workers. This resolves the `mixed` offset access errors at higher PHPStan levels. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
`WP_Theme_JSON_Gutenberg::get_viewport_media_queries()` only ever returns the `@mobile`, `@tablet`, and `@desktop` keys, each holding a non-empty `@media` query string, so document that in a `@phpstan-return` array shape. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
The media queries come from `WP_Theme_JSON_Gutenberg::get_viewport_media_queries()` or, when that is unavailable, from Core's `WP_Theme_JSON::get_viewport_media_queries()`. Core's method only documents a plain `array` return, so the union of the two leaves each `$media_query` as `mixed`. That value is passed as the `rules_group` of the style engine CSS rules, which expects a string. Skip any query that is not a string, so `rules_group` is narrowed to `string` whichever class supplies the queries. Both implementations only return strings, so there is no change in behavior. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
This reverts commit aed1887. The guard was only needed because Core's `WP_Theme_JSON::get_viewport_media_queries()` documented a plain `array` return. That method now has an array shape in WordPress/wordpress-develop#15644, so with stubs generated from it, each media query is already a `non-falsy-string` and `rules_group` is narrowed without the check. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
…the commit
The stubs generator already read a commit hash from the wordpress-develop checkout that Gutenberg is in, when the checkout has it. A pull request (`--pr`) or a branch (`--commit=trunk`), though, was always fetched from GitHub, since a ref is only resolved to a commit once fetched. That is slow even when the pull request's branch is the one checked out.
Resolve the ref to its commit first, so the checkout serves as a cache for these too. The lookup uses the GitHub REST API (`commits/{ref}` with the `vnd.github.sha` media type), which answers in under a second, and handles a branch, a tag, and `refs/pull/N/head` alike. `git ls-remote` was tried first, but for a repository with as many refs as wordpress-develop, it took around 50 seconds. When the lookup fails, or the checkout lacks the commit, the ref is fetched as before.
PHP's HTTP stream wrapper ignores the proxy environment variables that Git and curl read, so they are applied to the request, honoring `no_proxy`. Without that, the lookup fails wherever GitHub is only reachable through a proxy, falling back to the slow fetch. A `GITHUB_TOKEN` or `GH_TOKEN` is sent when set, for the higher rate limit of authenticated requests.
Generating the stubs from a pull request whose head is checked out drops from about 60 seconds to 6.
Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com>
Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
…p#15644 That pull request adds the array shape for `WP_Theme_JSON::get_viewport_media_queries()`, which the Navigation block falls back to, so that its media queries are known to be strings when passed to the style engine as a `rules_group`. Regenerate the stubs with `--commit=trunk` once it is committed to Core, before this is merged. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
… it has the commit" This reverts commit 07fd94e. No network request is needed to reuse the local checkout for a pull request. With `--checkout --pr=<number>` and no uncommitted changes, the generator already records the checkout's `HEAD` commit together with the pull request, with the same description and source in composer.json as `--pr` gives, and without fetching anything. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
…kout The usage and the coding guidelines only said that `--checkout` stubs cannot be generated again in CI, which holds for uncommitted changes alone. Without them, `--checkout --pr=<number>` records the checkout's commit along with the pull request, exactly as `--pr` does, so when the pull request's branch is checked out and pushed, it generates the same stubs without fetching the pull request from GitHub. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
The whole PHPStan guidance was one paragraph, covering what is analyzed, how to run it, the baseline, the WordPress stubs, and local configuration. Give it a subsection of its own with a heading for each topic, put the commands in code blocks, and list the stub generator's options with when to use each. The content is unchanged. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
The `function.alreadyNarrowedType` error for the Navigation block's `method_exists()` check is reported from rule level 4, but the entry was listed under the level 3 heading, after the `WP_REST_Request` offset entries. Give it a `Level 4:` heading, explain the check in the comment, and list `message` before `identifier`, as the other entries do. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
The commands to run the analysis and to regenerate the baseline were each shown together in one code block, which reads as if both should be run, when either one does. List them instead, each with the setup it is for. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
Link each file and directory that the PHPStan subsection of the coding guidelines refers to, and name the stubs generator script that the Composer command runs. Per the documentation guidelines, a package README is linked with a root-relative path, which the Block Editor Handbook rewrites. Other files and directories are linked by their full GitHub URLs, as `docs/manifest.json` is, since root-relative links to them would not resolve in the handbook. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
…ring keys The layouts merged for each viewport are narrowed from attribute values with `is_array()`, so PHPStan only knows them as `array<mixed>`. Passing their merge to a parameter typed `array<string, mixed>` needed an inline `@var` asserting string keys, which nothing checks. `block_core_navigation_get_layout_custom_property_declarations()` does not depend on the keys being strings, since it only reads named offsets with `??`. Document `$layout` as `array<mixed>` instead, which is what it is given, and remove the `@var`, so that the call is the same as before. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
Resolve the conflicts in the generated files by generating them again. The WordPress stubs are generated from wordpress-develop trunk at r65320, which includes the shapes from WordPress/wordpress-develop#15644, and the PHPStan baseline is regenerated with them. Co-Authored-By: Claude Opus 5.5 (medium effort) <noreply@anthropic.com> Claude-Code-Session-Id: 32212707-a7f1-4bca-bef2-7f3b4982950b
What?
See #86088
Resolves the remaining
function.alreadyNarrowedTypeerrors, where PHPStan reports a type check as always true: one check is removed, redundant guards are removed after narrowing a parameter type, and one intentional check is ignored in the configuration. Related types are narrowed along the way so that the values involved are checked at higher rule levels too.Builds on WordPress/wordpress-develop#15644, which was committed to Core in r65318.
Why?
Each of these needs a different fix, depending on whether the check is still needed. This resolves 4 of the 226 errors initially baselined in #66693.
How?
lib/client-assets.php:method_exists( 'WP_Interactivity_API', 'add_client_navigation_support_to_script_module' ). The method was added in WordPress 6.9, and the plugin requires WordPress 7.0, so the check is always true.block_core_navigation_get_layout_custom_property_declarations()had threeis_array( $layout )checks, but its only caller checks both inputs withis_array()before merging them witharray_replace(), so$layoutis always an array. It is now documented asarray<mixed>, and the checks are removed. The keys are not narrowed to strings, since nothing validates the layout attribute's keys and the function does not depend on them. Each offset is still read with??, andjustifyContentis still checked withis_string(), so unexpected values fall back to the defaults as before. (2 errors)phpstan.neon.dist: the Navigation block checksmethod_exists( 'WP_Theme_JSON_Gutenberg', 'get_viewport_media_queries' )before falling back toWP_Theme_JSON. PHPStan sees the Gutenberg class as always defined, but the block library also runs in WordPress Core, where it is not. Following the guidance in Baselined PHPStan errors need to be addressed #86088, this is a targeted ignore with a comment explaining why, rather than a baseline entry or an inline@phpstan-ignore.tools/phpstan/baseline.neon.Narrowed types, which resolve further errors at higher rule levels:
@phpstan-returnarray shape listing each custom property and its possible values. For PHPStan to verify the--navigation-layout-alignvalues, the vertical alignment is looked up in a map of its own rather than chosen within_array(), with the same results.WP_Theme_JSON_Gutenberg::get_viewport_media_queries()now has a@phpstan-returnarray shape, as doesWP_Theme_JSON::get_viewport_media_queries()in Code Quality: Define array shapes for block type selectors and viewport media queries wordpress-develop#15644. With both, the media queries that the Navigation block passes to the style engine as arules_groupare known to be strings.gutenberg_define_interactivity_modules_support()are typed with the shape that wp-build writes for each entry.WordPress stubs and documentation:
--checkout --pr=<number>records the same source as--prwhen the pull request's branch is checked out with no uncommitted changes and pushed, without fetching anything.There is no change in behavior.
Testing Instructions
composer phpstanand confirm it passes with no unmatched ignored errors.composer phpstan:baselinesand confirmtools/phpstan/baseline.neonhas no further changes.Testing Instructions for Keyboard
Not applicable. There are no UI changes.
Screenshots or screencast
Not applicable.
Use of AI Tools
I used Claude Code to identify the affected occurrences and draft the change. I reviewed the diff and take responsibility for it.
The follow-up commits that narrow the types, generate the stubs, and update the documentation were written with Claude Code using Claude Opus 5.5 (medium effort), and reviewed by a maintainer.