WordPress 7.1 stubs

[IanDelMar]

WordPress 7.1 includes numerous type-related DocBlock improvements following core's adoption of PHPStan.

Obsolete function map entries

This PR removes obsolete function map entries and updates others where the relevant types are now documented in core:

• _get_cron_array() (identical)
• dirlist() in WP_Filesystem_Base, WP_Filesystem_Direct, WP_Filesystem_FTPext, WP_Filesystem_ftpsockets, and WP_Filesystem_SSH2
• get_approved_comments() (narrower in core)
• get_comment() (core correctly allows null even if $comment = \WP_Comment and narrower)
• get_page_by_path() (correct and narrower in core)
• get_post_ancestors() (correct and narrower in core)
• get_post() (core correctly allows null even if $post = \WP_Post)
• sanitize_post()
• stripslashes_deep() (related Issues with PHPStan templates and constant types #333)
• wp_get_speculation_rules_configuration()
• wp_is_numeric_array() (assert tag only; gives same result)
• wp_slash() (return type only) and wp_unslash() (related Issues with PHPStan templates and constant types #333)
• wpdb::get_results()
• wpdb::get_row() (return type & 1 param type only; see WordPress 7.1 stubs #476 (comment) for details)

It also removes obsolete function map entries for functions that are no longer present in core:

• block_core_home_link_build_css_font_sizes()
• block_core_navigation_render_submenu_icon()
• block_core_navigation_submenu_build_css_font_sizes()
• block_core_page_list_build_css_font_sizes()

Kept/improved overrides

Some of our types remain (partly) narrower than those documented in core, despite core's improved type documentation.

• check_admin_referer(): merge WP's return type into ours (ours narrower than core's)
• check_ajax_referer(): merge WP's return type into ours (ours narrower than core's)
• edit_term_link(): update to new implementation (related x|void is not a valid type #250)
• get_post_types(): keep our return type (we provide array key types)
• get_posts(): keep our return type (core misses id=>parent)
• sanitize_post_field(): keep our return type ($value is documented as mixed, but core conditionally returns int|array|string, probably assuming only possible post field values are passed - needs further investigation whether narrowing $value would be the better approach)
• stripslashes_from_strings_only(): keep our narrower return type (preserves information about empty strings; can be dropped if '' is not considered worth maintaining an override)
• term_exists(): keep our return type (incorrect array key type in core)
• wp_die(): keep our return type (core uses array{exit: false} which is treated as sealed by Psalm, wp_die return type is wrong #248, and will be treated as sealed by PHPStan in the next major)
• wp_insert_attachment(): keep our return type which is narrower (assuming IDs > 0)
• wp_insert_post(): keep our return type which is narrower (assuming IDs > 0)
• wp_update_post(): keep our type which is narrower (assuming IDs > 0)

Updated overrides

• WP_Widget_Factory::$widgets: core changed implementation of WP_Widget_Factory::register()

New overrides

Core documents the return type for wp_upload_dir() as:

 * @phpstan-return array{
 *     path: non-empty-string,
 *     url: non-empty-string,
 *     subdir: non-empty-string,
 *     basedir: non-empty-string,
 *     baseurl: non-empty-string,
 * }
 * |array{error: non-empty-string}

If the creation of year- and month-based upload directories is disabled, subdir can be an empty string. Its documented type non-empty-string is therefore incorrect. Furthermore, error is always false when no error has occurred, so documenting it only as non-empty-string does not accurately describe the return value. Expressing the return type as a union also appears unnecessary and may cause problems if PHPStan does not collapse the two members into a single array shape, which I believe it does not. Assuming that relevant global constants are set to reasonable values, a more accurate return type would be:

array{
    path: non-falsy-string,
    url: non-falsy-string,
    subdir: string,
    basedir: non-falsy-string,
    baseurl: non-falsy-string, 
    error: string|false
}

which is implemented by this PR. The safest approach would be to widen the non-falsy strings to string.

Inferred types

The PR suppresses the generation of @phpstan-param tags for array shapes whose parameter types are already documented using PHPStan syntax in core.

The following table summarises the effect of suppressing these generated tags as of v7.1.

Symbol	Parameter	Comparison
WP_Filesystem_*::__construct()	$opt	Core's type is more precise than ours. It correctly distinguishes between required and optional array keys, narrows the array value types, and permits the default value null.
WP_Connector_Registry::register()	$args	Core's type is more precise than ours. It correctly distinguishes between required and optional array keys and narrows the array value types.
wp_get_typography_value_and_unit()	$options	Core's type is more precise than ours because it narrows the array value types.
wp_register_script()	$args	Core's type is more precise than ours because it narrows the array value types.
wp_enqueue_script()	$args	Core's type is more precise than ours because it narrows the array value types.
add_image_size()	$crop	Core's type is more precise than ours because it narrows the array value types.
wp_get_image_encode_quality()	$size	Core's type is more precise than ours because it narrows the array value types.
get_post()	$filter	Core's type is identical to ours.
get_post_field()	$context	Core's type is identical to ours.
sanitize_post()	$context	Core's type is identical to ours.
sanitize_post_field()	$context	Our type incorrectly omits 'sample'.
wp_delete_term()	$args	Core's type is more precise than ours because it narrows the array value types. However, it also permits string, which was deliberately excluded from our type.
wp_insert_term()	$args	Core's type narrows the type of parent, widens slug and description from string to string|null, and permits arbitrary additional array keys and values.
wp_update_term()	$args	Core's type narrows the type of parent, widens slug—but not description—from string to string|null, and permits arbitrary additional array keys and values.
WP_Comment::get_children()	$args	Core omits some array keys but assigns narrower types to some of the keys it documents and permits arbitrary additional array keys and values.

The accuracy of the narrowed types in core has not been verified.

Overall:

• There are divergences between the standard PHPDoc types and the PHPStan-specific types. For example, wp_insert_term() has the standard PHPDoc type array{description?: string, ...}, while its PHPStan-specific type is array{description?: string|null, ...}. The PHPStan-specific type for $opt in WP_Filesystem_*::__construct() has likewise been widened to include the default value null.
• The PHPStan-specific types appear to describe what the current implementation accepts rather than what the intended contract should permit. This is reasonable when analysing WordPress itself. Including the default value null in a PHPStan-specific parameter type, as in WP_Filesystem_*::__construct(), does not affect the analysis result in this case: passing null is accepted either way. However, when a value of an incorrect type is passed, PHPStan reports a union containing null as the expected type, even though callers should not pass the default value explicitly. Including null in an array value, may change the analysis result as these types are effectively widened.
• Some array shapes are sealed, while others are unsealed. It is unclear (to me) whether WordPress core has an established convention governing this distinction.
• Dropping auto-generated tags, as proposed in this PR, has the benefit of potentially more accurate types (analyses of the WP codebase test this types against the implementation - I think) and less need to improve parsing WP DocBlocks at the cost of types that are sometimes wider than the intended contract.

The PR prevents the inference of void and never return types when the corresponding return type is already documented. Core has started adding @return void and @return never annotations, and inferring these types again would duplicate the existing information. This issue is also addressed by #458, but that PR is not yet ready to be merged (I will work on 458 after this PR has been merged).

Related test data

At @szepevikto's request (#476 (comment)), legacy test data for the removed function map entries is retained in tests/monitoring-data to allow continued monitoring of the corresponding types in WordPress core.

Test data for functions that are no longer part of WP is dropped.

Other type improvements

• wp_unique_id(): narrowed return type
• wp_unique_prefixed_id(): narrowed return type
• wp_get_elements_class_name(): narrowed return type

Other changes

• bump PHPStan to ^2.2 for unsealed array shape support
• use ... for unsealed array shapes, as used by WordPress core and supported by PHPStan and Psalm

Commit strategy

Each change is contained in a separate commit. The stubs file is regenerated in each commit so that the effect of every change can be reviewed in a small, self-contained step.

[szepeviktor]

following core’s adoption of PHPStan.

Tell them: leave PHPStan to us!

[szepeviktor]

Are these really obsolete?
https://developer.wordpress.org/reference/classes/wpdb/get_row/#parameters

[szepeviktor]

I would keep tests for types moved to core.
Guys, what do you think?

[szepeviktor]

please bump szepeviktor/phpcs-psr-12-neutron-hybrid-ruleset to ^1.2

[IanDelMar]

Are these really obsolete? https://developer.wordpress.org/reference/classes/wpdb/get_row/#parameters

Core documents:

 * @phpstan-param 'OBJECT'|'ARRAY_A'|'ARRAY_N' $output
 * @phpstan-return (
 *     $query is non-falsy-string
 *         ? (
 *             $output is 'OBJECT'
 *                 ? stdClass|null
 *                 : (
 *                     $output is 'ARRAY_A'
 *                         ? array<array-key, mixed>|null
 *                         : (
 *                             $output is 'ARRAY_N'
 *                                 ? list<mixed>|null
 *                                 : null
 *                         )
 *                 )
 *         )
 *         : null
 * )

The following annotation was removed in 2a3020e

 * @phpstan-param 'OBJECT'|'ARRAY_A'|'ARRAY_N' $output
 * @phpstan-return null|void|($output is 'ARRAY_A' ? array<array-key, mixed> : ($output is 'ARRAY_N' ? list<mixed> : \stdClass))

The parameter type for $output is identical.

wpdb::get_row() no longer returns void, removing it is correct.

Conditioning the return type on whether $query is a non-falsy string is also correct because the implementation contains:

	if ( $query ) {
		...
	} else {
		return null;
	}

The remaining logic is unchanged, it is merely expressed differently:

• 'OBJECT' -> stdClass|null
• 'ARRAY_A' -> array<array-key, mixed>|null
• 'ARRAY_N' -> list<mixed>|null

In the removed annotation, each of these return types was already nullable because null was included in the top-level union: null|void|.....

Therefore, core’s annotation fully replaces the mapped return type and is more precise about the effect of $query.

@szepeviktor, is there a particular difference that makes you think the function map entry may not be obsolete?

[szepeviktor]

@IanDelMar I've realized how you used the word obsolete.

Now pls revert tests. I do not trust core.

[IanDelMar]

I would keep tests for types moved to core. Guys, what do you think?

I would not keep them. Testing annotations maintained in WordPress core is beyond the scope of this project.

[szepeviktor]

Testing annotations maintained in WordPress core is beyond the scope of this project.

Yes, I know, but I want to keep a laser eye on these new core types.

[IanDelMar]

@szepeviktor I am getting mixed signals here. You previously raised concerns about the number of tests and argued for keeping the codebase small and tidy. Now you are suggesting that we retain tests for types that have moved to core. Have you checked whether core already tests these annotations?

An alternative: https://github.com/szepeviktor/viktors-observatory 😉

Could we at least reorganise the tests or somehow mark the tests so that it is immediately clear which ones test this project's own functionality and which ones exist to keep Viktor’s laser eye on WordPress types? Ideally, these WordPress-monitoring tests would run only on demand. I do not think contributors should have to deal with tests that are unrelated to the changes they are contributing to this project.

[szepeviktor]

I do not think contributors should have to deal with tests that are unrelated to the changes they are contributing to this project.

You are absolutely right.
Please move removed tests to a "laser-eyes" directory, and I'll create a manual workflow to keep that eye open.
Thank you!

[szepeviktor]

This is the new, super-strict stuff!

    <rule ref="PSR12NeutronRuleset.Security.SuperglobalKeys">
        <properties>
            <property name="allowedSuperglobalKeys" type="array">
                <element value="_GET[taxonomy]"/>
            </property>
        </properties>
    </rule>

[szepeviktor]

We are read to FLY!

[szepeviktor]

Ian is in autocannon mode.

[IanDelMar]

@szepeviktor almost done :-)

[szepeviktor]

You should receive a voucher to replace your worn-out keyboard.

[IanDelMar]

@szepeviktor, before this is merged, I think someone should take a closer look. For each case, I have tried to indicate why I think we should retain our existing type or replace it with core’s.

[szepeviktor]

Do not click here: https://github.com/szepeviktor/return-type-oracle

[IanDelMar]

Haha, I had something similar once. If I remember correctly, I used an ErrorFormatter, though. I found it pretty tedious to review all those “suggestions”, even after filtering out those that were identical to types already known from the DocBlocks.

[szepeviktor]

Recent results of the oracle: https://gist.github.com/szepeviktor/347b362b6935071f298579507d77962d

[szepeviktor]

Recent results of the oracle: https://gist.github.com/szepeviktor/347b362b6935071f298579507d77962d

Just got much SHORTER: ~1000 lines, now anyone can handle it.

(the oracle is only 254 lines long)

[IanDelMar]

What is 'do_shortcode' => ['non-falsy-string|string'], supposed to represent? This simplifies to string.

[IanDelMar]

'get_blog_list' => ['array|array{}'],
'get_blogaddress_by_id' => ['\'\'|string'],

[IanDelMar]

Should we move this discussion to https://github.com/szepeviktor/return-type-oracle to avoid cluttering the comments on this PR?