Skip to content

Fix PHPStan level 0 issues in themes#10951

Open
westonruter wants to merge 7 commits intoWordPress:trunkfrom
westonruter:fix/phpstan-themes
Open

Fix PHPStan level 0 issues in themes#10951
westonruter wants to merge 7 commits intoWordPress:trunkfrom
westonruter:fix/phpstan-themes

Conversation

@westonruter
Copy link
Member

The commits in this PR were cherry-picked from #10419 for Core-61175.

Trac ticket: https://core.trac.wordpress.org/ticket/64238

Use of AI Tools

n/a


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.

@github-actions
Copy link

github-actions bot commented Feb 17, 2026

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props westonruter, sabernhardt.

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

@westonruter
Copy link
Member Author

@sabernhardt Would you review when you get a chance?

@github-actions
Copy link

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.

*
* @param WP_Query $query WP_Query object.
* @return WP_Query Possibly-modified WP_Query.
* @return void

Choose a reason for hiding this comment

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

This seems right. For history, this was already in r25808, when the Featured_Content class was added.

* @param int $post_id The ID of the post.
* @param string $location The location where the meta is shown.
* @return string Post meta HTML.
* @return string|void Post meta HTML.
Copy link

@sabernhardt sabernhardt Feb 17, 2026

Choose a reason for hiding this comment

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

This function can return void in two situations, but I think returning an empty string could be better when there is no HTML to return. Maybe it's unwise to change that now, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, for this it seems best to document the existing behavior.

$repl = sprintf( '<svg class="svg-icon" width="%d" height="%d" aria-hidden="true" role="img" focusable="false" ', $size, $size );

$svg = preg_replace( '/^<svg /', $repl, trim( $arr[ $icon ] ) ); // Add extra attributes to SVG code.
$svg = (string) preg_replace( '/^<svg /', $repl, trim( $arr[ $icon ] ) ); // Add extra attributes to SVG code.

Choose a reason for hiding this comment

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

The pattern and $repl are both strings. If $arr[ $icon ] is somehow not a string, then it will have errors with the trim() function.

Suggested change
$svg = (string) preg_replace( '/^<svg /', $repl, trim( $arr[ $icon ] ) ); // Add extra attributes to SVG code.
$svg = preg_replace( '/^<svg /', $repl, trim( (string) $arr[ $icon ] ) ); // Add extra attributes to SVG code.

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue that PHPStan is warning about is that preg_replace() can return false. So then we'd need to add two casts. However, what about cfb02ce? This ensures the proper type up front.

Choose a reason for hiding this comment

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

Yes, I prefer checking is_string( $arr[ $icon ] ) instead of casting it.

Comment on lines +869 to +876
*
* @phpstan-return (
* $header is 'Tags'
* ? string[]|false
* : ( $header is 'Name'|'ThemeURI'|'Description'|'Author'|'AuthorURI'|'Version'|'Template'|'Status'|'TextDomain'|'DomainPath'|'RequiresWP'|'RequiresPHP'|'UpdateURI'
* ? string|false
* : false )
* )

Choose a reason for hiding this comment

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

Does class-wp-theme.php belong in this PR?

Copy link
Member Author

@westonruter westonruter Feb 17, 2026

Choose a reason for hiding this comment

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

Yes, because it fixes issues with calls to wp_get_theme()->get( 'Version' ).

Copy link
Member Author

Choose a reason for hiding this comment

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

For example:

wp_enqueue_style( 'twenty-twenty-one-style', get_template_directory_uri() . '/style.css', array(), wp_get_theme()->get( 'Version' ) );

PHPStan is flagging this:

Parameter #4 $ver of function wp_enqueue_style expects bool|string|null, array|string|false given.

With the inclusion of the @phpstan-return, the issue is resolved.

@westonruter
Copy link
Member Author

@sabernhardt anything else you'd like revised?

@sabernhardt
Copy link

I have nothing further.

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.

2 participants