#64896 Fix remaining PHPStan level 10 errors in WP_Hook

[westonruter]

This is stacked on #12441 and includes all of its commits. The only new commit here is b0d8b59, which resolves the remaining PHPStan level 10 errors in WP_Hook on top of the type tightening done in that PR. @johnbillion: feel free to merge this into your branch if you want to fold it in; otherwise I'll rebase onto trunk once your PR lands.

The new commit:

• Adds @return void to the methods lacking return values.
• Documents the callback $args arrays as list<mixed> and narrows the $callback params of remove_filter()/has_filter() to callable, matching add_filter().
• Passes 0 instead of false to _wp_filter_build_unique_id() in has_filter(), matching its int $priority param. (The value is unused for the key, so this is behavior-neutral.)
• Restructures the apply_filters() loop to assign the current priority to a local variable before storing it in $current_priority, bailing from the loop in the (impossible) case that current() returns false. This keeps $current_priority honestly typed as array<int, int>; previously, PHPStan saw int|false being stored.

With this, composer phpstan src/wp-includes/class-wp-hook.php reports no errors at level 10.

Local phpstan.neon overrides

includes:
	- phpstan.neon.dist
	- vendor/phpstan/phpstan/conf/bleedingEdge.neon

parameters:
	# https://phpstan.org/user-guide/rule-levels
	level: 10

	editorUrl: 'phpstorm://open?file=%%file%%&line=%%line%%'

	# Narrow scanDirectories to source subdirs only. Pointing at plugin roots causes
	# PHPStan to traverse vendor/ and node_modules/ (~9k files), which dominates
	# startup time — excludePaths does not prune the traversal.
	scanDirectories:
		- tests/phpunit

	ignoreErrors:
		- '#^Cannot cast mixed to int\.$#'
		# FTP\Connection is PHP 8.1+; the @var is correct for WP's 7.4+ range, but
		# bleedingEdge's version-aware check flags it against the PHP 7.4 floor.
		-
			identifier: class.notFound
			path: src/wp-admin/includes/class-wp-filesystem-ftpext.php
		-
			message: '#expects resource, FTP\\Connection#'
			path: src/wp-admin/includes/class-wp-filesystem-ftpext.php
		-
			identifier: function.internal

Trac ticket: https://core.trac.wordpress.org/ticket/64896
Trac ticket: https://core.trac.wordpress.org/ticket/64898

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Fable 5
Used for: Diagnosing some of the PHPStan level 10 errors and drafting some of the fixes in the final commit; I reviewed, adjusted, and take responsibility for the changes.

---

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

https://claude.ai/code/session_01Y1qB3ZWwGV2nBtdb9NgcJa

[github-actions]

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 johnbillion, westonruter, sergeybiryukov.

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

[github-actions]

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.

[johnbillion]

This is intentionally wider than callable: 7819ca4

[westonruter]

How about bf773c3?

[westonruter]

I had to widen the $callback type in _wp_filter_build_unique_id() as well.

[westonruter]

I chose array{ 0: string|object, 1: string, ... } as the array shape for the $callback because this is the array shape required by _wp_filter_build_unique_id() or else it will return null.

[johnbillion]

How about encapsulating callable|string|array{ 0: string|object, 1: string, ... } into a PHPStan type on the class with a name that makes it clear what it represents? eg:

@phpstan-type Maybe_Callable callable|string|array{ 0: string|object, 1: string }

I don't believe the trailing ... is necessary on the array either. A callable array will only every have two elements, the class name or object and the method name.

[SergeyBiryukov]

The new commit:

* Adds `@return void` to the methods lacking return values.

As per the documentation standards, this should generally be avoided:

Note: @return void should not be used outside the default bundled themes and the PHP compatibility shims included in WordPress Core.

That said, it is quite possible that this point is now outdated and should perhaps be reconsidered.

[westonruter]

@SergeyBiryukov:

That said, it is quite possible that this point is now outdated and should perhaps be reconsidered.

I think it is obsolete, yes. PHPStan identifies the lack of a return type as an error. New void-returning functions being introduced in core have been using the native void type hint. That said, we could configure PHPStan to ignore the missing return type, but why? Better to be explicit.

Related: There has been cases of void being used in core already which have been replaced with null in unions via Core-64704.

[johnbillion]

I agree that refraining from using @return void is outdated, but let's handle that separately from these changes.

[johnbillion]

The $priority argument is no longer used in _wp_filter_build_unique_id(). Perhaps it should be removed instead.

[westonruter]

I was debating whether to do this when working through this. The $hook_name parameter isn't used either, but we can't remove it so easily. My hesitation about removing it is that static analysis with older versions of WP would then flag the lack of the arg as being an error when supplied, or otherwise on newer versions it would flag when it is supplied. I think it's just as well to leave it as-is for now.

[johnbillion]

Frustratingly, $args isn't necessarily a list here (although I can see that WP_Hook:: do_action() already documents it as such). The top-level apply_filters() function can be called with the associative array unpacking syntax:

$args = [
    'one' => 1,
    'two' => 2,
];
$value = apply_filters( 'foo', 'bar', ...$args );

This causes named parameters to be passed to apply_filters() which results in an associative array of those named parameters being passed to WP_Hook::apply_filters().

I doubt anyone is doing this in real life, but for type safety we need to account for it. Not sure the best approach though. Perhaps $args = array_values( $args )?

[westonruter]

Interestingly, in PHP 8 passing an associative array to call_user_func_array() maps the parameters to the corresponding named param in the callback function: https://3v4l.org/GXnqP#veol

When passing the array through apply_filters(), then the mapping is lost, resulting in (apparent) incorrect param: https://3v4l.org/lTFHM#veol

This doesn't work in PHP 8.0 at all. It only starts to work in PHP 8.1. In PHP 7.x, the associative nature of the array is ignored.

Since this didn't work in PHP 7.x and you couldn't pass them in PHP 8.0 either, it seems like now is the time to make sure that $args gets turned into a list to make sure to preserve the expected behavior from PHP 7.

As for how to type apply_filters() so that static analysis flags attempting to spread named parameters into an invocation, I just learned about the @no-named-arguments annotation which does the trick:

apply_filters( 'foo', 'bar', ...array( 'asdasd' => 1 ) );

Function apply_filters invoked with named argument $asdasd, but it's not allowed because of @no-named-arguments.

I've pushed this as bd5a548.

We could add something like this in a function like apply_filters(), do_action(), et al:

/** @var mixed[] $args */
if ( ! array_is_list( $args ) ) {
	_doing_it_wrong( __FUNCTION__, __( 'Expected args as list.' ), '7.1.0' );
	$args = array_values( $args );
}

This might be overkill and it adds another function call in the critical path for WP.

[westonruter]

With the latest changes, given the following PHP file:

<?php
apply_filters( 'foo', 'bar', ...array( 'baz' => 1 ) );
apply_filters_ref_array(
	'foo',
	array(
		'bar',
		'baz' => 1,
	)
);
apply_filters_deprecated(
	'foo',
	array(
		'bar',
		'baz' => 1,
	),
	'1.0'
);

do_action( 'foo', ...array( 'bar' => 1 ) );
do_action_ref_array(
	'foo',
	array(
		'bar' => 1,
	)
);
do_action_deprecated(
	'foo',
	array(
		'bar' => 1,
	),
	'1.0'
);

PHPStan (rule level 10) is flagging each invocation as having an error:

 ------ ----------------------------------------------------------------------- 
  Line   try-bad-hook-params.php                                                
 ------ ----------------------------------------------------------------------- 
  2      Function apply_filters invoked with named argument $baz, but it's not  
         allowed because of @no-named-arguments.                                
         🪪  argument.named                                                     
         at src/wp-content/mu-plugins/try-bad-hook-params.php:2                 
  5      Parameter #2 $args of function apply_filters_ref_array expects list<m  
         ixed>, array{0: 'bar', baz: 1} given.                                  
         🪪  argument.type                                                      
         💡  array{0: 'bar', baz: 1} is not a list.                             
         at src/wp-content/mu-plugins/try-bad-hook-params.php:5                 
  12     Parameter #2 $args of function apply_filters_deprecated expects list<  
         mixed>, array{0: 'bar', baz: 1} given.                                 
         🪪  argument.type                                                      
         💡  array{0: 'bar', baz: 1} is not a list.                             
         at src/wp-content/mu-plugins/try-bad-hook-params.php:12                
  19     Function do_action invoked with named argument $bar, but it's not      
         allowed because of @no-named-arguments.                                
         🪪  argument.named                                                     
         at src/wp-content/mu-plugins/try-bad-hook-params.php:19                
  22     Parameter #2 $args of function do_action_ref_array expects list<mixed  
         >, array{bar: 1} given.                                                
         🪪  argument.type                                                      
         💡  array{bar: 1} is not a list.                                       
         at src/wp-content/mu-plugins/try-bad-hook-params.php:22                
  28     Parameter #2 $args of function do_action_deprecated expects list<mixe  
         d>, array{bar: 1} given.                                               
         🪪  argument.type                                                      
         💡  array{bar: 1} is not a list.                                       
         at src/wp-content/mu-plugins/try-bad-hook-params.php:28                
 ------ ----------------------------------------------------------------------- 


 [ERROR] Found 6 errors                                                         

[westonruter]

I agree that refraining from using @return void is outdated, but let's handle that separately from these changes.

Reverted in 199b9ce