Skip to content

PHPStan: Resolve always-true type checks - #86104

Merged
westonruter merged 20 commits into
WordPress:trunkfrom
huzaifaalmesbah:fix/phpstan-compat-checks
Nov 6, 2026
Merged

westonruter merged 20 commits into
WordPress:trunkfrom
huzaifaalmesbah:fix/phpstan-compat-checks

Conversation

@huzaifaalmesbah

@huzaifaalmesbah huzaifaalmesbah commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

What?

See #86088

Resolves the remaining function.alreadyNarrowedType errors, 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?

  • Removed an outdated check in 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.
  • Removed redundant guards in the Navigation block: block_core_navigation_get_layout_custom_property_declarations() had three is_array( $layout ) checks, but its only caller checks both inputs with is_array() before merging them with array_replace(), so $layout is always an array. It is now documented as array<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 ??, and justifyContent is still checked with is_string(), so unexpected values fall back to the defaults as before. (2 errors)
  • Ignored an intentional check in phpstan.neon.dist: the Navigation block checks method_exists( 'WP_Theme_JSON_Gutenberg', 'get_viewport_media_queries' ) before falling back to WP_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.
  • Removed the 4 corresponding entries from tools/phpstan/baseline.neon.

Narrowed types, which resolve further errors at higher rule levels:

  • Navigation layout custom properties: the return value now has a @phpstan-return array shape listing each custom property and its possible values. For PHPStan to verify the --navigation-layout-align values, the vertical alignment is looked up in a map of its own rather than chosen with in_array(), with the same results.
  • Viewport media queries: WP_Theme_JSON_Gutenberg::get_viewport_media_queries() now has a @phpstan-return array shape, as does WP_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 a rules_group are known to be strings.
  • Script module registry: the modules loaded from the build's registry in gutenberg_define_interactivity_modules_support() are typed with the shape that wp-build writes for each entry.

WordPress stubs and documentation:

  • The stubs are generated from wordpress-develop trunk at r65320, which includes Code Quality: Define array shapes for block type selectors and viewport media queries wordpress-develop#15644, and the baseline is regenerated with them.
  • The stubs generator's usage and the coding guidelines now say that --checkout --pr=<number> records the same source as --pr when the pull request's branch is checked out with no uncommitted changes and pushed, without fetching anything.
  • The PHPStan guidance in the coding guidelines was one long paragraph. It is now a subsection with a heading for each topic, code blocks for the commands, and a list of the stub generator's options.

There is no change in behavior.

Testing Instructions

  1. Run composer phpstan and confirm it passes with no unmatched ignored errors.
  2. Run composer phpstan:baselines and confirm tools/phpstan/baseline.neon has no further changes.
  3. Optional smoke test: on the front end, check that a Navigation block with a custom justification, a vertical orientation, and "Allow to wrap to multiple lines" disabled renders as before, and that client-side navigation still works for blocks that use it.

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.

- 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.
@huzaifaalmesbah huzaifaalmesbah added [Type] Code Quality Issues or PRs that relate to code quality No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core labels Nov 1, 2026
@github-actions github-actions Bot added the [Package] Block library /packages/block-library label Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

🤖 PR meta 🤖

Preview in WordPress Playground

🎉 Props

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: huzaifaalmesbah <huzaifaalmesbah@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.


Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

  • @propsbot add <username> to give props to a GitHub user. Requires write access.
  • @propsbot remove <username> to leave a user out of the props. Requires write access.
  • @propsbot refresh to refresh this list. Adding the props-bot label works too.

Updated as activity occurs, without notifying anyone named here. Add the props-bot label to refresh.

@huzaifaalmesbah
huzaifaalmesbah marked this pull request as ready for review November 1, 2026 03:38
…hecks

# Conflicts:
#	tools/phpstan/baseline.neon

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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

Comment thread lib/client-assets.php
// 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' ) ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

westonruter and others added 13 commits November 6, 2026 09:37
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
westonruter and others added 2 commits November 6, 2026 10:44
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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The review evidence identifies no active issues requiring changes, with the noted PHPStan and documentation updates addressing the remaining review concerns.

No open issues 🎉

Review effort: Lite

@westonruter
westonruter enabled auto-merge (squash) November 6, 2026 21:33
@westonruter
westonruter merged commit 8f625ba into WordPress:trunk Nov 6, 2026
81 checks passed
@github-actions github-actions Bot added this to the Gutenberg 24.5 milestone Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core [Package] Block library /packages/block-library [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants