#64896 Fix remaining PHPStan level 10 errors in WP_Hook
#12443
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -201,6 +201,7 @@ private function resort_active_iterations( $new_priority = false, $priority_exis | |
| * a callback that may or may not exist. | ||
| * @param int $priority The exact priority used when adding the original filter callback. | ||
| * @return bool Whether the callback existed before it was removed. | ||
| * @phpstan-param Maybe_Callable $callback | ||
| */ | ||
| public function remove_filter( $hook_name, $callback, $priority ) { | ||
| if ( null === $priority ) { | ||
|
|
@@ -248,13 +249,14 @@ public function remove_filter( $hook_name, $callback, $priority ) { | |
| * of that hook is returned, or false if the function is not attached. | ||
| * If `$callback` and `$priority` are both provided, a boolean is returned | ||
| * for whether the specific function is registered at that priority. | ||
| * @phpstan-param Maybe_Callable|false $callback | ||
| */ | ||
| public function has_filter( $hook_name = '', $callback = false, $priority = false ) { | ||
| if ( false === $callback ) { | ||
| return $this->has_filters(); | ||
| } | ||
|
|
||
| $function_key = _wp_filter_build_unique_id( $hook_name, $callback, is_int( $priority ) ? $priority : 10 ); | ||
|
|
||
| if ( ! $function_key ) { | ||
| return false; | ||
|
|
@@ -320,9 +322,11 @@ public function remove_all_filters( $priority = false ) { | |
| * | ||
| * @since 4.7.0 | ||
| * | ||
| * @param mixed $value The value to filter. | ||
| * @param list<mixed> $args Additional parameters to pass to the callback functions. | ||
|
Member
There was a problem hiding this comment. Frustratingly, $args = [
'one' => 1,
'two' => 2,
];
$value = apply_filters( 'foo', 'bar', ...$args );This causes named parameters to be passed to 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
Member
Author
There was a problem hiding this comment. Interestingly, in PHP 8 passing an associative array to When passing the array through 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 As for how to type apply_filters( 'foo', 'bar', ...array( 'asdasd' => 1 ) );
I've pushed this as bd5a548. We could add something like this in a function like /** @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.
Member
Author
There was a problem hiding this comment. 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: |
||
| * When filtering, this array is expected to include $value | ||
| * at index 0. When called for an action by WP_Hook::do_action(), | ||
| * it contains only the action arguments. | ||
| * @return mixed The filtered value after all hooked functions are applied to it. | ||
| */ | ||
| public function apply_filters( $value, $args ) { | ||
|
|
@@ -337,9 +341,14 @@ public function apply_filters( $value, $args ) { | |
| $num_args = count( $args ); | ||
|
|
||
| do { | ||
| $priority = current( $this->iterations[ $nesting_level ] ); | ||
|
|
||
| if ( false === $priority ) { | ||
| // This is not expected to occur since the hook is known to have callbacks at one or more priorities. | ||
| break; | ||
| } | ||
|
|
||
| $this->current_priority[ $nesting_level ] = $priority; | ||
|
|
||
| foreach ( $this->callbacks[ $priority ] as $the_ ) { | ||
| if ( ! $this->doing_action ) { | ||
|
|
@@ -370,7 +379,7 @@ public function apply_filters( $value, $args ) { | |
| * | ||
| * @since 4.7.0 | ||
| * | ||
| * @param list<mixed> $args Parameters to pass to the callback functions. | ||
| */ | ||
| public function do_action( $args ) { | ||
| $this->doing_action = true; | ||
|
|
@@ -387,7 +396,7 @@ public function do_action( $args ) { | |
| * | ||
| * @since 4.7.0 | ||
| * | ||
| * @param list<mixed> $args Arguments to pass to the hook callbacks. Passed by reference. | ||
| */ | ||
| public function do_all_hook( &$args ) { | ||
| $nesting_level = $this->nesting_level++; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
$priorityargument is no longer used in_wp_filter_build_unique_id(). Perhaps it should be removed instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was debating whether to do this when working through this. The
$hook_nameparameter 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.