_get_list_table issues

[IanDelMar]

The _get_list_tables() function has some issues. See #256.

Also, see https://phpstan.org/r/1a049be1-3251-46b0-9e5a-2688de476073. Note the error on line 59, which occurs because WP_List_Table itself will never be returned.

This can be partially resolved by updating the functionMap entry as follows:

[
    '_get_list_table' => ['($class_name is T ? new<T> : false)', '@phpstan-template T of' => "'WP_Posts_List_Table'|'WP_Media_List_Table'|'WP_Terms_List_Table'|'WP_Users_List_Table'|'WP_Comments_List_Table'|'WP_Post_Comments_List_Table'|'WP_Links_List_Table'|'WP_Plugin_Install_List_Table'|'WP_Themes_List_Table'|'WP_Theme_Install_List_Table'|'WP_Plugins_List_Table'|'WP_Application_Passwords_List_Table'|'WP_MS_Sites_List_Table'|'WP_MS_Users_List_Table'|'WP_MS_Themes_List_Table'|'WP_Privacy_Data_Export_Requests_List_Table'|'WP_Privacy_Data_Removal_Requests_List_Table'", 'class_name' => 'class-string', 'args' => 'array{screen?: string}'],
]

This results in the following stub:

    /**
     * Fetches an instance of a WP_List_Table class.
     *
     * @since 3.1.0
     *
     * @global string $hook_suffix
     *
     * @param string $class_name The type of the list table, which is the class name.
     * @param array  $args       Optional. Arguments to pass to the class. Accepts 'screen'.
     * @return WP_List_Table|false List table object on success, false if the class does not exist.
     * @phpstan-template T of 'WP_Posts_List_Table'|'WP_Media_List_Table'|'WP_Terms_List_Table'|'WP_Users_List_Table'|'WP_Comments_List_Table'|'WP_Post_Comments_List_Table'|'WP_Links_List_Table'|'WP_Plugin_Install_List_Table'|'WP_Themes_List_Table'|'WP_Theme_Install_List_Table'|'WP_Plugins_List_Table'|'WP_Application_Passwords_List_Table'|'WP_MS_Sites_List_Table'|'WP_MS_Users_List_Table'|'WP_MS_Themes_List_Table'|'WP_Privacy_Data_Export_Requests_List_Table'|'WP_Privacy_Data_Removal_Requests_List_Table'
     * @phpstan-param class-string $class_name
     * @phpstan-param array{screen?: string} $args
     * @phpstan-return ($class_name is T ? new<T> : false)
     */
    function _get_list_table($class_name, $args = array())
    {
    }

new<T> may be used from PHPStan 1.11 onwards.

But this assertion still fails:

/** @var 'WP_Posts_List_Table'|'WP_Post' $className */
assertType('WP_Posts_List_Table|false', _get_list_table($className));

Additionally adding @phpstan-param class-string<WP_List_Table> $class_name would eliminate all incorrect return types (PHPStan now raises errors if a type is passed that results in an incorrect return type). This might be too restrictive. However, I would interpret the documentation on $class_name (which states "The type of the list table, which is the class name.") to imply that _get_list_table() actually requires a WP_List_Table class string.

Here is a solution that works without further restricting $class_name, though it requires PHPStan 2.0: https://phpstan.org/r/a36a822d-bf1f-4ce7-bd79-c83c817fb032.

Last resort for now: remove _get_list_table() and use the return type extension exclusively.

@johnbillion Do you have any idea? What do you think about @phpstan-param class-string<WP_List_Table> $class_name? Would this be too restrictive?

[johnbillion]

The return value in core is WP_List_Table|false. That doesn't strictly mean that $class_name must be class-string<WP_List_Table>, but I agree that it's implied. And if you're doing something else then you should fix it with an @var above your call to _get_list_table().

[szepeviktor]

Why don't we go for @phpstan-template T of \WP_List_Table?

[IanDelMar]

The return value in core is WP_List_Table|false. That doesn't strictly mean that $class_name must be class-string<WP_List_Table>

It doesn’t. For me it’s "the type of the list table, which is the class name," that equates to class-string<WP_List_Table>. The function still returns WP_List_Table|false.

Why don't we go for @phpstan-template T of \WP_List_Table?

This doesn’t resolve the error on line 59 and also restricts $class_name to class-string<WP_List_Table>.

We have to agree upon:

1. Whether it’s okay to restrict $class_name to class-string<WP_List_Table>, or if we need to allow a general class-string, and
2. Whether we should increase the required PHPStan version to 1.11 in order to resolve the Type #1 from the union: Type X is not always the same as T. It breaks the contract for some argument types, typically subtypes. issue using new<T>.

I’d say both restricting $class_name (why would you call _get_list_table on a class-string of a class that doesn’t extend WP_List_Table, especially given that no one should be calling _get_list_table anyway) and bumping the PHPStan version (as it’s not a breaking change) are acceptable.