PHPStan code quality improvements for 6.9

[justlevine]

This ticket is for various code quality issues and improvements surfaced via PHPStan.

Implementing PHPStan is tracked separately in #61175.

Previously (latest first)

• #52217 (6.8)
• #51423 (6.2)

[justlevine]

Crossposted stats from #61175 using ​https://github.com/WordPress/wordpress-develop/pull/7619, so there's a quick reference for comparison when this ticket gets closed for WP7.0

(even if today's trunk isn't 100% synonymous with 6.9)

As of 12 April 2025 (based on ​https://github.com/WordPress/wordpress-develop/commit/4ce25a5b915ce4409fb6339b8b1ce46fc10aeb0a )

(Previous rebase: 21 Feb 25 ​https://github.com/WordPress/wordpress-develop/commit/165d803380c561f39caeb0cbdc44f5c06c988cba )

PHPStan Level	Error Count	New	Remediated
0	14	-	-
1	65	-	5
2	326	1	2
3	195	5	-
4	985	6	1
5	557	8	5
6	2134	3	0

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

This PR fixes the parameter doc-type on WP_REST_Server::get_index()'s $request param to be a WP_REST_Request instead of an array.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates _wp_filter_build_unique_id() to explicitly return null if the function isn't passed a valid $callback. This is instead of the current behavior where the function can return void, but is typed as only returning a string.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

This PR fixes the default $post ID param values used in several functions in wp-includes/link-template.php, to bring them in line with their int doctypes (and the doctypes of their internal function calls).

More specifically,

• get_page_link(): $post = false changed to $post = 0
• _get_page_link(): $post = false changed to $post = 0
• post_comments_feed_link(): $post_id = '' changed to $post_id = 0

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates the @param types on get_comments_number_text() to correctly reflect the default false parameter.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes the doc-block for rest_menu_read_access, where the third param name is incorrectly named $this, which is an illegal parameter name.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes the default $site_id parameter value used throughout WP_User to correctly match the (int) doctypes, and the doctypes of the functions they use internally.

Specifically:

• __construct(): $site_id = '' is now $site_id = 0
• init(): $site_id = '' is now $site_id = 0
• for_blog(): $blog_id = '' is now $blog_id = 0
• for_site(): $site_id = '' is now $site_id = 0

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes an issue in wp_xmlrpc_server::wp_deleteTerm() doesn't correctly return the WP_Error message from the wp_delete_term() attempt.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes the WP_Meta_Query constructor arg, to take a default value of array(), as specified by the doc-block @param .

Since the function is gated with a loose, falsy ! $meta_query check, there is no difference to function behavior.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates the @return type for _wp_get_current_user() to correctly reflect that it can return a null value if no user is logged in.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

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

This PR fixes an issue in get_taxonomy_labels(), where the template_name was accessed as an object property on $tax->labels instead of as an array property.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

[SergeyBiryukov]

In 60152:

Coding Standards: Correct $post parameter default values in link-template.php.

This commit corrects the default post ID parameter values used in several functions to bring them in line with their int doctypes (and the doctypes of their internal function calls).

More specifically,

• get_page_link(): $post = false changed to $post = 0
• _get_page_link(): $post = false changed to $post = 0
• post_comments_feed_link(): $post_id = '' changed to $post_id = 0

Follow-up to [1752], [4475], [6365], [9136], [9274], [21735], [24490], [32606], [37252].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60152.

This PR fixes a bug in WP_Debug_Data::get_wp_themes_inactive(), where the update_themes site $transient was not defined before being checked in several isset() checks later on in the function (L1359).

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates wp_get_nav_menu_to_edit() to explicitly return null and adds null to the function's return type. This is instead of the existing behavior where the implict void return is not typehinted.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

[SergeyBiryukov]

In 60166:

Docs: Correct parameter types for get_comments_number_text().

This updates the @param types to correctly reflect the default false parameter.

Follow-up to [6495], [25567], [28912], [49936].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60166.

[SergeyBiryukov]

In 60172:

Docs: Correct parameter name for rest_menu_read_access filter.

The filter's third parameter was incorrectly named $this, which is not a valid parameter name.

Follow-up to [59718], [59734].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60172.

[SergeyBiryukov]

In 60174:

Coding Standards: Correct $site_id parameter default values in WP_User.

This commit corrects the default $site_id parameter values used throughout the WP_User class to bring them in line with their int doctypes (and the doctypes of the functions used internally by the affected class methods).

More specifically,

• __construct(): $site_id = '' changed to $site_id = 0
• init(): $site_id = '' changed to $site_id = 0
• for_blog(): $blog_id = '' changed to $blog_id = 0
• for_site(): $site_id = '' changed to $site_id = 0

Follow-up to [12796], [15566], [18597], [41624].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60174.

[SergeyBiryukov]

In 60175:

XML-RPC: Correctly return deletion error message in wp_xmlrpc_server::wp_deleteTerm().

This ensures that the correct variable is used to return the WP_Error message from the wp_delete_term() attempt.

Follow-up to [20137].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60175.

[SergeyBiryukov]

In 60176:

Coding Standards: Correct default parameter type in WP_Meta_Query::__construct().

This adjusts the default $meta_query parameter value to match the documented type of array.

Since the function is gated with a loose, falsey ! $meta_query check, there is no difference to function behavior.

Follow-up to [17699].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60176.

[SergeyBiryukov]

In 60177:

Docs: Correct $request parameter type in WP_REST_Server::get_index().

Follow-up to [34928], [52796].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60177.

[SergeyBiryukov]

In 60179:

Coding Standards: Explicitly return null in _wp_filter_build_unique_id().

This commit updates the function to explicitly return null if a valid $callback is not passed. Previously, the function could return void, but was typed as only returning a string.

Follow-up to [5936], [11409], [12090], [46220], [46801], [50807], [52300].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60179.

[SergeyBiryukov]

In 60180:

Coding Standards: Explicitly return null in wp_get_nav_menu_to_edit().

This commit updates the function to explicitly return null if a valid $menu_id is not passed or the term does not exist. Previously, the function could return void, but was typed as only returning a string or a WP_Error object.

Follow-up to [14248].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60180.

[SergeyBiryukov]

In 60181:

Site Health: Set missing $transient in WP_Debug_Data:get_wp_themes_inactive().

This commit addresses a bug where the update_themes site transient was not defined before being checked in several isset() checks later on in the method.

Follow-up to [59176].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60181.

On further investigation, this is invalid, and labels is indeed reconverted back to an object.

@justlevine Are you sure this function can return null? When a user isn't logged in, this function calls wp_set_current_user(0) which sets the $current_user global to an instance of WP_User with an ID of 0. This is what gets returned by _wp_set_current_user().

@justlevine Are you sure this function can return null? When a user isn't logged in, this function calls wp_set_current_user(0) which sets the $current_user global to an instance of WP_User with an ID of 0. This is what gets returned by _wp_set_current_user().

Nope, you're right 😅 Thanks for catching that @johnbillion !

This PR removes a needless empty( $object_terms ) check from inside is_object_in_term(). If $object_terms is empty, the immediately-preceding code-block causes the function to return early.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR improves the type safety around WP_Screen in the following ways:

• Correctly hint that WP_Screen::$_show_screen_options is null before instantiated.
• Correctly hint that ::get_option(), get_help_tab() and get_screen_reader_text() can return null.
• Ensure $this->columns is an int, by casting $this->get_option( 'layout_columns', 'default' ) from its numeric string.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This is needed because category_exists() returns a numeric string

This PR fixes wp_create_category() to return an int as defined in the DocBlock, instead of the numeric-string returned by category_exists().

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates the PHPDoc type for WP_List_Table::$_column_headers to correctly indicate it's null until ::get_column_info() is called.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes the casing of the get_current_user_id() call in script-loader.php. Previously it was being incorrectly referenced as get_current_user_ID() (last letters uppercased).

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

$old_user_data is set if ! update on L2213

This PR removes an unnecessary empty( $old_user_data ) check from wp_insert_user(). The $old_user_data variable is defined alongside $update, so by checking for one, we've already narrowed the type for the other.

• ​https://github.com/justlevine/wordpress-develop/blob/dd5e2bccaa6f145a10d25283c75ac39846e2046b/src/wp-includes/user.php#L2208-L2226

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes an issue in WP_REST_Attachments_Controller::update_item(), where the $schema['properties'] are checked for featured media, but is only defined in the parent method and not where it is currently used.

Follow up to: https://core.trac.wordpress.org/ticket/41692

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an unnecessary empty( $modes_str ) check from wpdb::set_sql_mode(), as the previous code block already checks and returns early:

• ​https://github.com/justlevine/wordpress-develop/blob/09faccb0971511c3dba6f65f9effa185ab66e741/src/wp-includes/class-wpdb.php#L960

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an unncessary empty( $status_clauses ) from WP_Comment_Query::get_comment_ids(). The preceeding foreach loop ensures that $status_clauses is a non-empty array.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

Im not sure I understand the failing test case . _Shouldnt_ a fake attachment with an ID of 0 throw a 400?

This PR removes two unnecessary empty( $object_subtype ) checks from map_meta_cap(), as it causes the currently case to break in the conditional preceding this one:

• ​https://github.com/justlevine/wordpress-develop/blob/dbad7732e342f1795a536817e5c5c86c802f84b2/src/wp-includes/capabilities.php#L461-L464

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes two calls to WP_Theme::get() that were previously incorrectly called as ::Get() (with a capital G).
In:

• WP_Language_Pack_Upgrader::get_name_for_update()
• WP_Automatic_Updater::update()

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes two calls to ftp_base::SetTimeout() that were incorrectly called as ::setTimeout() (lowercase s), in WP_Filesystem_ftpsockets::connect()

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes the casing of calls to PHPMailer::addCC (from ::addCc - lowercase c ) and PHPMailer::addBCC (from ::addBcc, - lowercase cc ) in wp_mail().

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes two attempts to use the results of base_convert() in a binaryOp without first converting the results to an integer.

In

• add_menu_page()
• WP_Duotone::colord_parse_hex()

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates WP_REST_Comments_Controller::get_items() to cast the values of $total_comments and $max_pages to string types before they are set on the response headers.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes the @return annotation for the get_registered() methods in the following classes to correctly indicate they can return null if no item is registered:

• WP_Block_Pattern_Categories_Registry::get_registered()
• WP_Block_Patterns_Registry::get_registered()
• WP_Block_Styles_Registry::get_registered()

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates get_the_block_template_html() to return an empty string ('') if no template is found and the user is not logged in. This is instead of the current behavior of returning void (coerced to null ).

This is inline with both the function DocBlock and how the function is used throughout the codebase, and thus less disruptive than explicitly returning null and then casting the results back to an empty string wherever it's used.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates default_topic_count_scale() to return an int (per documentation and usage) instead of the type float returned by ​`round()`

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

…gories()`

This PR updates the $post_id param's default value in wp_create_categories from an empty string to a 0 - inline with both the expected doc-type and how it's used.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates the $post_id param default value in redirect_post() from an empty string to 0, inline with both the doc-type and the usage of that default value internally.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes the doc-block on wp_update_attachment_metadata() to correctly return type bool.

The type seems to have been accidentally narrowed in ​https://github.com/WordPress/wordpress-develop/commit/679ccc35e63e62a90c768afcc16f46bd154dfb65, but both update_post_meta() and delete_post_meta() return true|false in this case.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes get_the_author_posts() so it correctly returns an int instead of a numeric string, per its doc-block and how it's used.

Additional the doc-block for the get_usernumposts filter has been updated to correctly indicate that $count is indeed a string.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

update_post_meta() returns an int if the meta key didn't previously exist. So I think the return type needs to be bool|int.

[johnbillion]

In 60275:

General: Various fixes to the correctness of code and documentation reported by PHPStan.

Props justlevine

See #63268

r60275

r60275

r60275

r60275

r60275

r60275

r60275

r60275

update_post_meta() returns an int if the meta key didn't previously exist. So I think the return type needs to be bool|int.

[SergeyBiryukov]

In 60282:

Coding Standards: Remove redundant ! empty() check in is_object_in_term().

If $object_terms is empty, the immediately preceding code block causes the function to return early.

Follow-up to [8131], [10159].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60282.

Thanks for the PR! This was merged in r60275.

[SergeyBiryukov]

In 60291:

Coding Standards: Remove extra check in WP_Comment_Query::get_comment_ids().

The preceding foreach loop ensures that $status_clauses is a non-empty array.

Follow-up to [30084].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60291.

[SergeyBiryukov]

In 60292:

Docs: Correct the type for WP_List_Table::$_column_headers.

The value is null until ::get_column_info() is called.

Follow-up to [31127], [35021].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60292.

[SergeyBiryukov]

In 60294:

Coding Standards: Remove redundant empty() checks in map_meta_cap().

There is already a check for empty( $object_subtype ) that exits the current case branch a few lines earlier.

Follow-up to [39179], [43378].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60294.

Thanks @johnbillion ! Updated the type and the description so it's clear just how unhelpful the return value is

[SergeyBiryukov]

In 60296:

Docs: Correct the type of the $count parameter in get_usernumposts filter.

Follow-up to [8873], [26901], [36085].

Props justlevine.
See #63268.

This PR adds a doc-block to the _() function in compat.php.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes the return type on WP_HTML_Decoder::read_character_reference() to correctly indicate that it returns a _nullable_ string, and not string|false.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes the return type on WP_Speculation_Rules::jsonSerialize() to show that it returns an array<string, mixed[]> instead of the internal array having a string key. This is due to the method's use of array_values() to intentionally strip any keys.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

Not sure if this warrants a 6.8.x commit, but I definitely don't think it's worth bothering to create a Trac ticket just to make that happen.

This PR fixes the PHPDoc property type of WP_Tax_Query::$no_results to correctly indicate that it's an array, not a string.

As a private property, changing this doc-type should cause no doctype conflicts with classes extending WP_Tax_Query.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates WP_Term_Query::get_terms() to return a numeric-string '0' instead of an in 0 when a 'count' is requested in $args['fields'] and no terms exist.

This aligns the behavior with the existing PHPDoc return type (and what's used by phpstub's map), and also aligns better with expectations. The risk of somebody "building around the quirk" and strict-checking 0 === WP_Term_Query::get_terms() in their code should be minimal.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR fixes some param/return type issues in comment.php concerning arrays. More specifically:

• separate_comments() now correctly indicates that it returns an array of WP_Comment arrays, keyed by their string types.
• _close_comments_for_old_posts now correctly indicates that it takes an array of WP_Post, and returns an array of WP_Post.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates the docs for wp_filter_oembed_result() to correctly indicate that it both take and return a string|false $result. The $result param hint for the oembed_dataparse filter has been similarly corrected.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

…eprecated`

This PR updated WP_Image_Editor_Imagick::set_imagick_time_limit() to use the @deprecated tag, instead of hiding it in a @since tag. This allows tooling and IDEs to warn developers if they are using the deprecated method.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes several unnecessary isset() calls from the codebase:

• In _get_block_template_files(): $area is proven set by the left side of the conditional.
• In Custom_Image_Header::step_2(): The only path where $oitar isnt set, returns early on L898 ​https://github.com/justlevine/wordpress-develop/blob/c043bcf2bb34a4e16ac8021a3de0f27de4670f91/src/wp-admin/includes/class-custom-image-header.php#L898
• In get_calendar(): $newrow is defined right before the for() loop, and the loop only toggles its value between true|false.
• In wp-admin/post.php: wp_die() is called if the $post_type_object is falsy on ​https://github.com/justlevine/wordpress-develop/blob/c043bcf2bb34a4e16ac8021a3de0f27de4670f91/src/wp-admin/post.php#L131
• In wp_delete_term(): the top of the foreach() loop already continues early if $default isnt set on ​https://github.com/justlevine/wordpress-develop/blob/c043bcf2bb34a4e16ac8021a3de0f27de4670f91/src/wp-includes/taxonomy.php#L2132-L2135

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates various @var tags on class properties to correctly indicate that they props may be null|unset:

• WP_Dependencies::$all_queued_deps is nullable by both ::enqueue() and ::dequeue.
• WP_Duotone::$global_styles_presets and ::$global_styles_block_names start off unset and are only initialized by static classes.
• WP_Query::init() and WP_Rewrite::init() are public methods that unset()s many class props.
• WP_Theme::cache_delete() sets many props to null.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates several functions to explicitly return null in accordance to their existing doc-types, instead of using a void return;.

Affected functions:

• get_comment_reply_link()
• get_page_of_comment()
• get_edit_term_link()
• get_preview_post_link()
• get_edit_post_link()
• wp_set_all_user_settings()
• get_page_by_path()
• save_mod_rewrite_rules()

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an unreachable break; from the 'allblogs' -> 'delete' case in wp-admin/network/sites.php, as the line before exits entirely.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR updates to remove an unreachable return $new_template_item; statement from _get_block_template_file(), as $template_type will bail at the top of the function( ​https://github.com/justlevine/wordpress-develop/blob/ae2dd43b83c6a985ebd7ecf07ba03d3a19c53ca0/src/wp-includes/block-template-utils.php#L324-L326) if it's not a wp_template or wp_template_part.
For the same reasons, the second if() was also redundant and able to be removed.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an unnecessary empty( $connection_type ) from inside request_filesystem_credentials(), as the left-side of the same conditional ( 'ssh' !== $connection_type ) will already catch a possibly empty value.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an unnecessary is_wp_error() check from wp_authenticate_application_password(), as the \WP_Error instance is created earlier in the method, and the only mutability are whether the instance actually ->has_errors().

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

[SergeyBiryukov]

In 60299:

Users: Correct get_the_author_posts() to always return an integer.

This matches the documented @return type.

Follow-up to [5638], [13576], [36085], [60296].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60296 and r60299.

[johnbillion]

In 60300:

Media: Fix the documented return type of the wp_update_attachment_metadata() function.

Props justlevine

See #63268

https://core.trac.wordpress.org/changeset/60300

[SergeyBiryukov]

In 60303:

Docs: Add missing @deprecated tag for ::set_imagick_time_limit().

Follow-up to [56250].

Props justlevine.
See #63268.

I believe this is a stylistic choice, so every case always has a break unless it intends to fall through. Whether or not that's documented or enforced in the coding standards I am not sure.

Whether or not that's documented or enforced in the coding standards I am not sure.

It's a PHPStan level-4 violation, so while there's there's no tooling-related urge to merge this, whoever is advocating the stylistic argument should keep in mind that once we hit that level this will look like:

...
        require_once ABSPATH . 'wp-admin/admin-footer.php';
        exit;
break; // @phpstan-ignore deadCode.unreachable (Optional comment about how this was a stylistic choice )

        case 'spam':
...

PS: there are _other_ parts of the codebase where a custom $this->exits(); will need a break; to prevent WPCS from reformatting the subsequent case statements. That's not the case here and won't be needed elsewhere either once PHPStan (annotations) are merged, and we can start using the @never type to indicate an exit.

E.g.
​https://github.com/WordPress/wordpress-develop/blob/3277cd6e5dfd2eaa782e828f40c56aa056b0661a/src/wp-includes/html-api/class-wp-html-processor.php#L2107-L2115

Thanks for the PR! Merged in r60303.

[SergeyBiryukov]

In 60307:

Docs: Correct parameter type for the oembed_dataparse filter.

This commit updates the filter documentation to indicate that it can both take and return a string|false value.

Documentation for functions attached to the filter has been similarly corrected:

• WP_oEmbed::_strip_newlines()
• wp_filter_oembed_result()
• wp_filter_oembed_iframe_title_attribute()

Follow-up to [12153], [25723], [34903], [34974], [44942].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60307.

[SergeyBiryukov]

In 60309:

Coding Standards: Remove extra empty() in request_filesystem_credentials().

This commit removes an unnecessary empty( $connection_type ) check, as the left side of the same conditional already accounts for a possibly empty value.

Follow-up to [37467].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60309.

[SergeyBiryukov]

In 60310:

Coding Standards: Remove extra check in wp_authenticate_application_password().

This commit removes an unnecessary is_wp_error() check, as the WP_Error instance is created earlier in the method, and the only mutability is whether the instance actually ::has_errors().

Follow-up to [49109].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60310.

[swissspidy]

If you need more inspiration for some functions that could use some improved types: ​https://github.com/szepeviktor/phpstan-wordpress/issues/274

[SergeyBiryukov]

In 60311:

Docs: Correct property type for WP_Tax_Query::$no_results.

Follow-up to [17686].

Props justlevine, johnbillion.
See #63268.

Thanks for the PR! Merged in r60311.

[SergeyBiryukov]

In 60312:

Docs: Add missing DocBlock for the _() function in compat.php.

Follow-up to [3901], [17603], [17620].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60312.

[SergeyBiryukov]

In 60328:

Coding Standards: Remove unreachable return in _get_block_template_file().

This commit removes an unreachable return statement, as the function will bail early if $template_type is not a wp_template or wp_template_part.

For the same reason, the second if conditional was also redundant and is now removed.

Follow-up to [52062].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60328.

[SergeyBiryukov]

In 60330:

Docs: Correct parameter and return types for some comment functions.

This commit corrects some parameter type issues concerning arrays. More specifically:

• separate_comments() now correctly indicates that it returns an array of WP_Comment objects, keyed by their string types.
• _close_comments_for_old_posts() now correctly indicates that it both takes and returns an array of WP_Post objects.

Follow-up to [8892], [8897], [8949], [32587], [42876].

Props justlevine, johnbillion.
See #63268.

Thanks for the PR! Merged in r60330.

[SergeyBiryukov]

In 60331:

Coding Standards: Remove unnecessary isset() check in get_calendar().

$newrow is defined right before the for loop, and the loop only toggles its value between true and false.

Follow-up to [34463].

Props justlevine.
See #63268.

[SergeyBiryukov]

In 60332:

Coding Standards: Remove unnecessary isset() check in wp_delete_term().

The top of the foreach loop already continues early if $default is not set.

Follow-up to [5533], [10813], [50389].

Props justlevine.
See #63268.

[SergeyBiryukov]

In 60339:

Coding Standards: Remove unnecessary isset() check in _get_block_templates_files().

$area is proven set by the left side of the conditional.

Follow-up to [55687].

Props justlevine.
See #63268.

[SergeyBiryukov]

In 60344:

Coding Standards: Remove unnecessary isset() check in wp-admin/post.php.

wp_die() is called a few lines earlier if $post_type_object is falsey.

Follow-up to [12597], [17169], [24201].

Props justlevine.
See #63268.

[SergeyBiryukov]

In 60351:

Coding Standards: Remove unnecessary isset() check in Custom_Image_Header::step_2().

The only path where $oitar is not set returns in an earlier conditional.

Follow-up to [20806].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60331, r60332, r60339, r60344, and r60351.

[SergeyBiryukov]

In 60353:

Coding Standards: Explicitly return null in functions saving URL rewrite rules.

This matches the documented @return type in save_mod_rewrite_rules() and iis7_save_url_rewrite_rules().

Follow-up to [12726], [17328], [43362].

Props justlevine.
See #63268.

[SergeyBiryukov]

In 60360:

Coding Standards: Explicitly return null in get_page_by_path().

This matches the documented @return type.

Follow-up to [37479].

Props justlevine.
See #63268.

This PR fixes the DocBlock return type for _oembed_rest_pre_serve_request() to correctly indicate that it can return _any_ bool value and not just true.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an unnecessary conditional from inside do_trackbacks(), as a falsey $to_ping causes the function to return early when the variable is defined (L3053 as of writing)ץ

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

It's the entrypoint into this codeblock two lines above

This PR removes unnecessary checks for $cpage inside get_comment_link(), where its truthiness is a pre-requisite for entering the code block (L835)

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an unreachable early continue from WP_Theme_JSON::get_block_nodes(). The $include_node_paths_only` variable is used to early-continue on L2784, preventing this one from ever being reached.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an always-true conditional check from WP_Customize_Manager::handle_changeset_trash_request(). $changeset_post_id early-returns if false right before this codeblock on L3165

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an unnecessary check for ! $enable inside WP_Upgrader:maintenance_mode(). The first part of the block checks if $enable is truthy, so it is always false in the elseif()`.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes a needless if ( ! $add_new_screen ){} conditional wrapper from inside wp-admin/nav-menus.php. That variable is already checked and falsey on L1107 and is a prerequisite for reaching this code.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an unnecessary null !== $args['taxonomy] check inside get_terms(), as the same conditional already calls isset( $args['taxonomy'] ), which checks for explicit nulls.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes two unnecessary conditional checks from within wp_resolve_numeric_slug_conflicts():

• a truthy check for $compare, when a falsey $compare causes an earlier return on the preceeding codeblock (L409)
• A check to ensure $maybe_page isn't an empty string, when $maybe_page is already cast to an int (L448).

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an always-true check for elseif ( is_multisite() ...) within delete_expired_transients(), where the preceding if() is for ! is_multisite().

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

…

This PR removes an unnecessary is_object( $user ) call from inside wpmu_validate_blog_signup() (the condition before the || is that it is ! is_object() ), and instead replaces it with a property_exists( 'user_login' ) check before an attempt is made to access that property.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an always-true check for false === $metadata from inside wp_get_theme_data_template_parts(), as a non-false value causes an early-return right before. To clarify this behavior the needless priming of $metadata = false; was removed as well, as there is no scenario where it will not be immediately overwritten.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes an always-true check for $exif_description inside wp_read_image_metadata(). This logical branch is only reachable if $exif_description is truthy, above on L960.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

This PR removes a needless check for $found_marker in insert_with_markers() from an elseif() that is preceeded by an if ( ! $found_marker ) check.

While this issue was surfaced via PHPStan in ​https://github.com/WordPress/wordpress-develop/pull/7619 (trac: https://core.trac.wordpress.org/ticket/61175 ), it can be remediated independently of that ticket.

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

[SergeyBiryukov]

In 60361:

Coding Standards: Explicitly return null in wp_set_all_user_settings().

This matches the documented @return type.

Follow-up to [22256], [37263], [39932], [48320].

Props justlevine.
See #63268.

[SergeyBiryukov]

In 60362:

Coding Standards: Explicitly return null in get_page_of_comment().

This matches the documented @return type.

Follow-up to [9367].

Props justlevine.
See #63268.

[SergeyBiryukov]

In 60401:

Docs: Correct @return value for _oembed_rest_pre_serve_request().

Follow-up to [35436].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60401.

[SergeyBiryukov]

In 60406:

Coding Standards: Remove unnecessary conditional from do_trackbacks().

The check for a truthy $to_ping value is redundant, as a falsey value would cause the function to return early a few lines above.

Follow-up to [1794], [2612].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60406.

[SergeyBiryukov]

In 60408:

Coding Standards: Remove redundant $cpage checks from get_comment_link().

These checks would always be true, as $cpage is checked for a truthy value before entering the code block.

Includes removing a duplicate user_trailingslashit() call, as there is a subsequent call a few lines below.

Follow-up to [34735].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60408.

[SergeyBiryukov]

In 60409:

Coding Standards: Remove unreachable continue from WP_Theme_JSON::get_block_nodes().

The outer loop uses a similar check for $include_node_paths_only to continue early a few lines above, preventing this one from ever being reached.

Follow-up to [59213].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60409.

[SergeyBiryukov]

In 60414:

Coding Standards: Remove redundant conditional in WP_Customize_Manager.

This check would always be true, as ::handle_changeset_trash_request() returns early a few lines above if $changeset_post_id is false.

Follow-up to [41667], [48211].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60414.

[SergeyBiryukov]

In 60417:

Coding Standards: Remove redundant check in WP_Upgrader:maintenance_mode().

The first part of the conditional checks if $enable is truthy, so it is always false in the elseif.

Follow-up to [11005].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60417.

[SergeyBiryukov]

In 60440:

Coding Standards: Remove redundant check in wp-admin/nav-menus.php.

The $add_new_screen variable is already checked as falsey a few lines above, and is a prerequisite for reaching this code.

Follow-up to [51539].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60440.

[SergeyBiryukov]

In 60445:

Coding Standards: Remove redundant check in get_terms().

The same conditional already calls isset(), which explicitly checks if the value is different from null.

Follow-up to [36614], [37599].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60445.

Thanks for the PR! Merged in r60447.

[SergeyBiryukov]

In 60448:

Coding Standards: Remove redundant check in delete_expired_transients().

The check in the elseif branch would always be true, as the preceding if is for ! is_multisite().

Follow-up to [41963].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60448.

[SergeyBiryukov]

In 60450:

Coding Standards: Replace redundant check in wpmu_validate_blog_signup().

The is_object() check would always be true in the second part of the conditional. This commit adds an instanceof WP_User check instead before accessing the user_login property.

Follow-up to [41963], ​mu:550, ​mu:1958, [12603].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60450.

For reference, it appears that the user_login property does not actually exist on the WP_User object, but is retrieved dynamically via ::__get(), so the property_exists() check didn't work as expected:

There was 1 failure:

1) Tests_Multisite_wpmuValidateBlogSignup::test_validate_blogname with data set #7 ('existinguserfoo', 'Site names must not collide w...login.')
Site names must not collide with an existing user login.
Failed asserting that an array contains 'blogname'.

I've adjusted the check to $user instanceof WP_User instead.

[SergeyBiryukov]

In 60459:

Coding Standards: Remove extra check in wp_get_theme_data_template_parts().

The false === $metadata check would always be true, as a non-false value causes an early return right before. To clarify this behavior, the needless priming of $metadata = false was removed as well, as there is no scenario where it will not be immediately overwritten.

Follow-up to [56385].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60459.

[SergeyBiryukov]

In 60475:

Coding Standards: Remove redundant check in wp_read_image_metadata().

This logical branch is only reachable if $exif_description is already verified as truthy a few lines above.

Follow-up to [57267].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60475.

[SergeyBiryukov]

In 60480:

Coding Standards: Remove redundant check in insert_with_markers().

The check in the elseif branch would always be true, as the preceding if is for ! $found_marker.

Follow-up to [34740].

Props justlevine.
See #63268.

Thanks for the PR! Merged in r60480.

Tests updated so they're comparing to a numeric-string instead of an int count.

[SirLouen]

Can we edit the title of this (and future release tasks)

To: Static Analysis code quality improvements for X.Y ?
(Or whatever is more generalistic, not only improvements that come from PHPStan, but that could come from any static analysis tool, or any static analysis in general).

I know that PHPCS is also technically an Static Analysis tool, but we already have a Code Standards improvements for X.Y.

I think users will find it easier to write here than if this is exclusively dedicated to anything that comes from PHPStan specifically.

cc @SergeyBiryukov @westonruter @justlevine

[justlevine]

@SirLouen the original intention for this ticket was specifically to remediate PHPStan violations, in order to reduce the friction (via errors/baselines) needed to adopt PHPStan (https://core.trac.wordpress.org/ticket/61175), as well as track progress and the impact of using that tool to bolster the justifications in favor of adopting it.

As a contributor and not a committer, I'd love for there to *also* be "Code Quality" catch-all (versus a ticket to explicitly justify each change) that may anecdotally remove errors from the PHPStan baseline, but I'm hesitant to repurpose this one.

IMO "Code Quality" is a very subjective term if it's not tied to a specific tool. This ticket is AFAIK what's enabling @SergeyBiryukov , @johnbillion , et. al. to review and make iterative progress without needing to spend additional time/cognitive energy on triage. ("No unnecessary refactors" still applies, but since we're starting from a clear and defined list the scope of potential changes is limited. Nobody's gonna randomly go Liskoff or unnecessarily OOP a bunch of legacy code). Ultimately, I defer to both of them and whatever makes things easiest for them 🙇

I know that PHPCS is also technically an Static Analysis tool, but we already have a Code Standards improvements for X.Y.

To be unnecessarily pedantic, PHPCS is a _linting_ tool and not static analysis 😅 but still I think there too, it's the virtue of it being a tool with a specific ruleset + violation list that allows for relatively efficient triage.

• "Is PHPCS complaining?" > "Do we care that PHPCS is complaining?" > "How much do we care that PHPCS is complaining vs the effort/cost of a refactor"

is a lot easier than

• "is this subjective code pattern measurably better than that also subjective one?"

I think users will find it easier to write here than if this is exclusively dedicated to anything that comes from PHPStan specifically.

I would love to see other contributors take the ​| PHPStan baseline and start triaging/patching with me.

That baseline is the perfect list of good first issues: small, scoped, requiring minimal if any new Unit tests. Just need enough know-how to know what's "worth doing" and what's testing noise (e.g. the PHPCS equivalent of an invalid comment end char vs not sanitizing a var).

But maybe I'm being too optimistic?

[SirLouen]

Replying to justlevine:

@SirLouen the original intention for this ticket was specifically to remediate PHPStan violations, in order to reduce the friction (via errors/baselines) needed to adopt PHPStan (https://core.trac.wordpress.org/ticket/61175), as well as track progress and the impact of using that tool to bolster the justifications in favor of adopting it.

Maybe a specific task should be set for this purpose. I have not fully set up the PHPStan with the baseline you added yet for my regular triage. But as I commented the other day, I don't even know if with this level, errors that are being spotted in the wild, would have been also spotted by PHPStan.

But we cannot restrict this to PHPStan violations, because they are strictly of the same nature. I don't agree that PHPStan violations are easier to triage (in fact we can be introducing bugs if we triage them too optimistically). They have to be triaged with the same carefulness as any other. Because PHPStan can miss more than a funfair shotgun.

To be unnecessarily pedantic, PHPCS is a _linting_ tool and not static analysis 😅 but still I think there too, it's the virtue of it being a tool with a specific ruleset + violation list that allows for relatively efficient triage.

PS: It appears that they don't agree. 🙊

[justlevine]

Maybe a specific task should be set for this purpose... But we cannot restrict this to PHPStan violations,

@SirLouen not sure I follow.

This ticket is the specific task set for triaging PHPStan issues. It was birthed as a request from https://core.trac.wordpress.org/ticket/52217#comment:112 which was also specific to PHPStan. (If I understood) you're suggesting that we rename and repurpose this ticket instead of opening up a new one to address broader "Code Quality" concerns. I'm suggesting a separate ticket can be created alongside this one (or after closing this one if that's what Sergey prefers, but until the PHPStan proposal is officially accepted there's value in keeping the history on this ticket specific and relevant).

don't agree that PHPStan violations are easier to triage

Easier to triage than what?

My argument wasn't specific to PHPStan, it's "having a scoped list of errors that need evaluation". We already agree that 1) PHPStan is a helpful tool and 2) just because PHPStan flags an error doesn't mean we should listen to it. Having that scope filters out any other (possibly justifiable, but still irrelevant here) topics so it's focused on answering "is this PHPStan error worth remediating or just noise", then as with any core workflow there's obviously still independent context and code review or the proposed remediation.

don't even know if with this level, errors that are being spotted in the wild

IIRC both on this ticket and on #52217 we caught some 6.8 regressions. If 6.9 is similar we'll see a good number from GB and new features once we hit beta1.

There's also the bugs that are hidden by less important PHPStan errors. For example, it can't alert you that about str_starts_with( $not_a_string, $also_not_a_string ) throwing a fatal error, if it's warning you that "The PHPDoc for $not_a_string says string|null, but it's actually a WP_Post|WP_Error.

If there's something I can document better or otherwise do to assist here, please let me know. The errors in the baseline are pretty self-documenting, and I tried to be as detailed about usage in the [PR description](​https://github.com/WordPress/wordpress-develop/pull/7619). All you should really need to do is

1. Checkout the PR and composer install to get the deps.
2. Comment out one of the errors from one of the baselines (or an entire level from the root phpstan.neon.dist if you're feeling frisky)
3. Run composer run analyse -- -vv (the first time will take a bit, but future runs are cached)
4. Hop over to the location of the error shown and decide if/how to remediate it.

You can also take a look at my remediation branch on ​https://github.com/WordPress/wordpress-develop/compare/trunk...justlevine:wordpress-develop:chore/phpstan/level-0 . I've been working there holistically (to help make sure that the remediations make sense in a larger context) and then cherry-pick out to a clean PR against truck the ones that are least likely to disrupt work-in-progress but are still independently justifiable changes.

PS: It appears that they don't agree. 🙊

Heh, I was unnecessarily pedantic and wrong 💀 thanks for the catch!
I'm rereading and have no idea why I said that, I think I was trying to highlight the difference between linting by line and linting by following the executing flow but thats very clearly not what I wrote and I'm also not sure why I thought that was tangentially relevant so 🥴 ?