PHPStan code quality improvements for 7.0

[SergeyBiryukov]

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

Implementing PHPStan is tracked separately in #61175.

Previously:

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

Trac ticket:
https://core.trac.wordpress.org/ticket/64238
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.

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/64238
https://core.trac.wordpress.org/ticket/63268

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/64238
https://core.trac.wordpress.org/ticket/63268

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/64238
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/64238
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/64238
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/64238
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/64238
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/64238
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/64238
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/64238
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/64238
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/64238
https://core.trac.wordpress.org/ticket/63268

[SergeyBiryukov]

In 61243:

Coding Standards: Remove redundant check in wpdb::set_sql_mode().

This commit removes an unnecessary empty( $modes_str ) check, as the previous code block already checks the same value and returns early.

Follow-up to [30587], [56475].

Props justlevine.
See #64238.

Thanks for the PR! Merged in r61243.

[SergeyBiryukov]

In 61247:

Coding Standards: Cast base_convert() to an integer before arithmetic operations.

Follow-up to [35477], [53104], [56101].

Props justlevine.
See #64238.

Thanks for the PR! Merged in r61247.

[westonruter]

In 61280:

Docs: Fix phpdoc types for WP_Speculation_Rules members.

What was previously mixed is actually a speculation rule which can be more accurately be typed as array<string, mixed>. Additionally, the return value of jsonSerialize is fixed to indicate it returns an array of lists as opposed an array of associative arrays.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/8946

Follow-up to [59837].

Props justlevine, johnbillion, westonruter.
See #64238, #63268, #62503.

Committed in r61280 (9e2d045)

[westonruter]

In 61281:

Docs: Explicitly return null when documented instead of void in link-template.php functions.

The phpdoc @return tags for these functions indicate they may return null. This also fixes "Missing return argument" warnings which an IDE may also complain about.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/8955

Props justlevine.
See #64238.

Committed in r61281 (c9c6518)

[westonruter]

In 61282:

Docs: Cast header values to strings in WP_REST_Comments_Controller::get_items().

Follow-up to [38832].

Props justlevine, johnbillion.
See #64238.

Committed in r61282 (​https://github.com/WordPress/wordpress-develop/commit/43f956385976bf08f3a12749c4471bfffb7c09a9)

[westonruter]

In 61283:

Docs: Fix return type for WP_HTML_Decoder::read_character_reference().

Developed in ​https://github.com/WordPress/wordpress-develop/pull/8945

Follow-up to [58281].

Props justlevine.
See #64238, #61072.

Committed in r61283 (e870c4a)

@justlevine I found that wp_create_category() was incorrectly saying that it could return WP_Error, when in fact it is supposed to always return an integer. In the case of failure, it returns zero, since wp_insert_category() isn't called with the $wp_error arg.

In trunk when I run:

phpstan analyze --memory-limit=2G --level=3 src/wp-admin/includes/class-wp-screen.php

I get:

Note: Using configuration file /Users/westonruter/repos/wordpress-develop/phpstan.neon.
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ----------------------------------------------------------------------------------- 
  Line   class-wp-screen.php                                                                
 ------ ----------------------------------------------------------------------------------- 
  306    Variable $post_id might not be defined.                                            
         🪪  variable.undefined                                                             
         at src/wp-admin/includes/class-wp-screen.php:306                                   
  552    Method WP_Screen::get_option() should return string but returns null.              
         🪪  return.type                                                                    
         at src/wp-admin/includes/class-wp-screen.php:552                                   
  558    Method WP_Screen::get_option() should return string but returns null.              
         🪪  return.type                                                                    
         at src/wp-admin/includes/class-wp-screen.php:558                                   
  605    Method WP_Screen::get_help_tab() should return array but returns null.             
         🪪  return.type                                                                    
         at src/wp-admin/includes/class-wp-screen.php:605                                   
  740    Method WP_Screen::get_screen_reader_text() should return string but returns null.  
         🪪  return.type                                                                    
         at src/wp-admin/includes/class-wp-screen.php:740                                   
  953    Property WP_Screen::$columns (int) does not accept string.                         
         🪪  assign.propertyType                                                            
         at src/wp-admin/includes/class-wp-screen.php:953      

In this branch, the last five are eliminated.

I guess the bug here has never been noticed before because no where in core is the REST API used to set the featured image for an attachment?

We should add a unit test for this.

@justlevine ok, I found the issue, and I've fixed it in 720512a. The issue is that with fixing the $schema here, it's allowing handle_featured_media() to be called by \WP_REST_Attachments_Controller::update_item() when the this method is also calling parent::update_item() in which case it calls \WP_REST_Posts_Controller::update_item() which _also_ calls the handle_featured_media() method. The handle_featured_media() method wasn't accounting for the case where an attempt is made to update a featured media to be the same as the featured media currently set.

This appears to have been introduced in r57603 (54d72c5deee2c5cfa2dadef4409b7beb1cefea38) to fix Core-41692 (as noted above).

The idempotency issue is only noticed for updating attachments because it short-circuits with an error when handle_featured_media() returns false:

​https://github.com/WordPress/wordpress-develop/blob/ba4d80d346eb2f71e1ccc122af466451759d84ba/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php#L224-L230

This doesn't happen when updating posts:

​https://github.com/WordPress/wordpress-develop/blob/ba4d80d346eb2f71e1ccc122af466451759d84ba/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L817-L819

So while the tests have been fixed, it's clear that test coverage is not complete, because this code is never run:

​https://github.com/WordPress/wordpress-develop/blob/ba4d80d346eb2f71e1ccc122af466451759d84ba/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php#L227-L229

So we need to make sure that the error scenario is tested.

So we need to make sure that the error scenario is tested.

OK, added in e6c6c68

Summarized in comment on original ticket: https://core.trac.wordpress.org/ticket/41692#comment:44

[westonruter]

In 61298:

Docs: Update typing for wp_create_category().

• Ensure that wp_create_category() returns int as opposed to numeric-string.
• Rename $cat_name to $category_name to avoid abbreviating variables.
• Update docblock to remove erroneous int for $category_name param when only string is intended.
• Add missing tests for wp_create_category().

Developed in ​https://github.com/WordPress/wordpress-develop/pull/8861

Props justlevine, westonruter.
See #64238, #64226.

Committed in r61298 (2cf0f3d)

[westonruter]

In 61299:

Docs: Update various class @var tags to indicate nullability.

This updates various @var tags on class properties to correctly indicate that the props may be null or unset:

• WP_Dependencies::$all_queued_deps is nullable by both ::enqueue() and ::dequeue. Also, the types of the keys and values are specified.
• 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.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/8953

Props justlevine, westonruter.
See #64238, #64224.

Committed in r61299 (0d33b5c)

[westonruter]

In 61300:

Docs: Improve docblocks and types for WP_Screen properties.

• Correctly hint that WP_Screen::$_show_screen_options is null before being 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.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/8860

Props justlevine, peterwilsoncc, westonruter.
See #64238, #64224.

Committed in r61300 (2898ab5).

[westonruter]

In 61303:

Coding Standards: Remove unreachable break statement after exit in switch.

This resolves static analysis warnings about an unreachable statement.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/8956

Follow-up to [41131].

Props justlevine, johnbillion.
See #64238.

Committed in r61303 (6bea530)

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

Running this PHPStan command:

phpstan analyze --memory-limit=4G --level=8 src/wp-includes/class-wp-scripts.php src/wp-includes/class-wp-styles.php src/wp-includes/class-wp-dependencies.php src/wp-includes/class-wp-dependency.php

Results in the following report for trunk:

 ------ ------------------------------------------------------------------------------------------------------------ 
  Line   class-wp-dependencies.php                                                                                   
 ------ ------------------------------------------------------------------------------------------------------------ 
  65     Property WP_Dependencies::$args type has no value type specified in iterable type array.                    
         🪪  missingType.iterableValue                                                                               
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type                  
         at src/wp-includes/class-wp-dependencies.php:65                                                             
  105    Property WP_Dependencies::$queued_before_register type has no value type specified in iterable type array.  
         🪪  missingType.iterableValue                                                                               
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type                  
         at src/wp-includes/class-wp-dependencies.php:105                                                            
 ------ ------------------------------------------------------------------------------------------------------------ 

 ------ -------------------------------------------------------------------------------------------- 
  Line   class-wp-dependency.php                                                                     
 ------ -------------------------------------------------------------------------------------------- 
  71     Property _WP_Dependency::$extra type has no value type specified in iterable type array.    
         🪪  missingType.iterableValue                                                               
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type  
         at src/wp-includes/class-wp-dependency.php:71                                               
 ------ -------------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------------------------- 
  Line   class-wp-scripts.php                                                                                    
 ------ -------------------------------------------------------------------------------------------------------- 
  51     Property WP_Scripts::$in_footer type has no value type specified in iterable type array.                
         🪪  missingType.iterableValue                                                                           
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type              
         at src/wp-includes/class-wp-scripts.php:51                                                              
  123    Property WP_Scripts::$default_dirs type has no value type specified in iterable type array.             
         🪪  missingType.iterableValue                                                                           
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type              
         at src/wp-includes/class-wp-scripts.php:123                                                             
  597    Method WP_Scripts::localize() has parameter $l10n with no value type specified in iterable type array.  
         🪪  missingType.iterableValue                                                                           
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type              
         at src/wp-includes/class-wp-scripts.php:597                                                             
  724    Property _WP_Dependency::$translations_path (string) in isset() is not nullable.                        
         🪪  isset.property                                                                                      
         at src/wp-includes/class-wp-scripts.php:724                                                             
 ------ -------------------------------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------------------- 
  Line   class-wp-styles.php                                                                          
 ------ --------------------------------------------------------------------------------------------- 
  101    Property WP_Styles::$default_dirs type has no value type specified in iterable type array.   
         🪪  missingType.iterableValue                                                                
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type   
         at src/wp-includes/class-wp-styles.php:101                                                   
  186    Parameter #1 $src of method WP_Styles::in_default_dir() expects string, string|false given.  
         🪪  argument.type                                                                            
         at src/wp-includes/class-wp-styles.php:186                                                   
 ------ --------------------------------------------------------------------------------------------- 

 [ERROR] Found 9 errors                                                                                                 

With this PR, the issues are fixed.

[westonruter]

In 61358:

Docs: Improve accuracy for types in phpdoc for WP_Dependencies, _WP_Dependency, WP_Scripts, and WP_Styles.

This increases these classes to PHPStan level 8.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10607

See #64238.

Committed in r61358.

This addresses the following PHPStan level 6 issues:

 ------ --------------------------------------------------------------------------------------------------------------- 
  Line   class-wp-script-modules.php                                                                                    
 ------ --------------------------------------------------------------------------------------------------------------- 
  122    Method WP_Script_Modules::register() has parameter $args with no value type specified in iterable type array.       
         🪪  missingType.iterableValue                                                                                       
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type                          
         at src/wp-includes/class-wp-script-modules.php:122                                                                  
  294    Method WP_Script_Modules::enqueue() has parameter $args with no value type specified in iterable type array.        
         🪪  missingType.iterableValue                                                                                       
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type                          
         at src/wp-includes/class-wp-script-modules.php:294                                                                  
  540    Method WP_Script_Modules::get_import_map() return type has no value type specified in iterable type array.          
         🪪  missingType.iterableValue                                                                                       
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type                          
         at src/wp-includes/class-wp-script-modules.php:540                                                                  
  561    Method WP_Script_Modules::get_marked_for_enqueue() return type has no value type specified in iterable type array.  
         🪪  missingType.iterableValue                                                                                       
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type                          
         at src/wp-includes/class-wp-script-modules.php:561                                                                  
  582    Method WP_Script_Modules::get_dependencies() return type has no value type specified in iterable type array.        
         🪪  missingType.iterableValue                                                                                       
         💡  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type                          
         at src/wp-includes/class-wp-script-modules.php:582   

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

[westonruter]

In 61362:

Docs: Improve specificity of types in WP_Script_Modules and script-modules.php functions.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10614

Follow-up to [61358].

See #64238.

Committed in r61362.