Skip to content

Bundled Themes: Fix PHPStan level 10 errors in Ipsum - #14701

Open
westonruter wants to merge 11 commits into
WordPress:trunkfrom
westonruter:fix/ipsum-phpstan-level-10
Open

westonruter wants to merge 11 commits into
WordPress:trunkfrom
westonruter:fix/ipsum-phpstan-level-10

Conversation

@westonruter

Copy link
Copy Markdown
Member

Ipsum passes PHPStan at the level 5 the repository configures, but has 69 errors at level 10. This fixes all of them, so Ipsum is clean at level 10 with bleedingEdge, as Twenty Twenty-Four and Twenty Twenty-Five already are.

  • Pattern image URLs. Thirteen patterns appended the theme version as a cache busting ?ver= (added in Rename the theme functions and add the theme version to pattern images ipsum#163 in response to review of Add Ipsum, the new default theme for 7.2 #14558), reading it with WP_Theme::get( 'Version' ), which returns string|false. That accounted for 60 of the errors. A new ipsum_get_asset_url() helper takes a path relative to the theme, resolves it with get_parent_theme_file_uri(), and adds the parent theme version with add_query_arg() when it can be read. The patterns now call it. Going through get_parent_theme_file_uri() also makes each asset URL filterable with parent_theme_file_uri, which the patterns' plain get_template_directory_uri() concatenation did not allow, and the ver is added after that filter runs, so a filtered URL keeps the cache busting.
  • Stylesheet version. ipsum_enqueue_styles() loads the parent theme's style.css but versioned it with the active theme, so with a child theme active, an Ipsum update did not change the stylesheet URL. It now uses get_template(), matching the pattern images and Twenty Twenty-Four.
  • Comments call to action binding. The postId context is untyped and its get_the_ID() fallback can be false. The post is now resolved with get_post(), which falls back to the current post as before, and the callback returns null when there is none, so the block keeps its original content. The unused $source_args, which must still be accepted to reach the block instance, is unset.
  • Search block ARIA label support. The block supports are narrowed to an array before ariaLabel is added.
  • Iterable value types for the block binding args, the template types and the block metadata.

Native types were left off the hook callbacks' parameters, since a plugin filtering in a non-array would otherwise cause a fatal error.

The theme's assets/images/ (and assets/fonts/) directories are missing from the import, so pattern images are currently broken regardless of this change. That is being addressed separately.

Trac ticket: Core-66352

Testing Instructions

  1. Run vendor/bin/phpstan analyse -c phpstan.neon.dist --level=10 src/wp-content/themes/ipsum. No errors are reported, where trunk reports 68 (69 with bleedingEdge, which also reports the unused $source_args).
  2. Activate Ipsum and insert the Gallery Offset and CTA Membership patterns in a post. Image URLs are built from the theme directory and end in ?ver=1.0.
  3. With a child theme of Ipsum active, the ipsum-style stylesheet URL ends in Ipsum's version, not the child theme's.
  4. View a single post using a template with the comments call to action. The link reads "Be the first to comment" or "Join the conversation" depending on the comment count.
  5. In the Site Editor, a Search block offers an ARIA label setting.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Running PHPStan at level 10, tracing the history of the ?ver= query string in the Ipsum repository, implementing the fixes under direction, and checking the changed callbacks with WP-CLI.


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

westonruter and others added 11 commits October 21, 2026 23:05
Thirteen Ipsum patterns append the theme version to image URLs as a
cache busting query string, read with `WP_Theme::get( 'Version' )`. That
returns `string|false`, which `esc_attr()` does not accept, so each of the
60 call sites was a PHPStan level 7 error.

`ipsum_get_version()` narrows the header to a string, falling back to an
empty string when it cannot be read, and the patterns now call it. As
before, the version comes from the parent theme.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ion.

`ipsum_enqueue_styles()` loads the parent theme's `style.css` but versioned
it with `wp_get_theme()`, which is the active theme. With a child theme
active, that is the child's version, so updating Ipsum left the stylesheet
URL unchanged, and sites with long static file caches could keep serving
the old file.

Pass `get_template()` so the version is Ipsum's, as the pattern images
already do and as Twenty Twenty-Four does for its parent theme styles.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace `ipsum_get_version()` with `ipsum_get_asset_url()`, which takes a
path relative to the theme directory, resolves it with
`get_parent_theme_file_uri()`, and adds the parent theme version as the
`ver` query argument with `add_query_arg()`, skipping it when the version
cannot be read.

The patterns now pass the image path to the helper instead of joining the
template directory URI, the path, and a hand-built query string, which
also stops them emitting an empty `?ver=` when there is no version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`get_parent_theme_file_uri()` returns the theme directory URL rather than
a file URL when the path is empty, which includes `'0'`, so neither is a
valid asset path for `ipsum_get_asset_url()`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The block binding source arguments are an object decoded from the block's
`metadata.bindings` attribute, so they are keyed by string with values of
any type.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The `postId` block context is untyped, and its fallback, `get_the_ID()`,
returns `false` when there is no current post, so neither was a valid
argument for `get_comments_number()` or `get_comments_link()`.

Resolve the post with `get_post()` instead, which falls back to the
current post just as `get_the_ID()` did, and return null when there is
none, so the block keeps its original content rather than linking to
comments on no post.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The `default_template_types` filter passes template types keyed by slug,
each an array with a string title and description.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The `block_type_metadata` filter passes the decoded `block.json`, keyed by
string with values of any type.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…abel.

The block metadata values are untyped, so writing `ariaLabel` straight
into `$metadata['supports']` wrote an offset on a mixed value. Read the
supports into a local array first, starting from an empty one when it is
missing or not an array, then add the ARIA label support and assign it
back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Block bindings pass the source arguments before the block instance, so
`ipsum_comments_cta_binding()` must accept them to reach the block, even
though it does not use them. Unset them to mark them as deliberately
unused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@westonruter
westonruter marked this pull request as ready for review October 22, 2026 06:33
@github-actions

Copy link
Copy Markdown

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:

Props westonruter.

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.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The 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

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

get_parent_theme_file_uri( 'style.css' ),
array(),
wp_get_theme()->get( 'Version' )
wp_get_theme( get_template() )->get( 'Version' )

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is important because get_parent_theme_file_uri( 'style.css' ) only ever refers to the parent theme's style.css, but without passing get_template() here, then it defaults to the child theme.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant