Fix passing error messages to wp_die() when plugin activation fails

[westonruter]

When working on preparing for the 3.0.0 release in #1125 I ran npm run since -- -r 3.0.0 which touched includes/admin/load.php. When I went to commit, lint-staged failed the pre-commit check with the following error:

Script build-cs/vendor/bin/phpstan analyse --memory-limit=2048M handling the phpstan event returned with error code 1
 ------ ----------------------------------------------------------------------- 
  Line   includes/admin/load.php                                                
 ------ ----------------------------------------------------------------------- 
  295    Parameter #1 $text of function esc_html expects string, array<string>  
         given.                                                                 
 ------ ----------------------------------------------------------------------- 

In looking at the code in question, I see there was indeed an error as $result->get_error_messages() returns an array of strings, and this can't be passed to esc_html() without first imploding to a string. It's confusing from the codebase because further up there is this:

performance/includes/admin/load.php

Lines 269 to 275 in 2b7b878

	if ( is_wp_error( $result ) ) {
	wp_die( esc_html( $result->get_error_message() ) );
	} elseif ( is_wp_error( $skin->result ) ) {
	wp_die( esc_html( $skin->result->get_error_message() ) );
	} elseif ( $skin->get_errors()->has_errors() ) {
	wp_die( esc_html( $skin->get_error_messages() ) );
	}

Specifically note the last line wp_die( esc_html( $skin->get_error_messages() ) ). This is actually valid because WP_Ajax_Upgrader_Skin::get_error_messages() returns a string while the method of the same name on WP_Error returns string[].

I discovered this issue because I have a local phpstan.neon which has a stricter level (at level 6) than phpstan.neon.dist (at level 0):

includes:
	- phar://phpstan.phar/conf/bleedingEdge.neon
	#- phpstan-baseline.neon
parameters:
	level: 6
	paths:
		- load.php
		- plugins/
		- includes/
		- tests/
		- uninstall.php
	scanDirectories:
		- vendor/wp-phpunit/wp-phpunit/
	ignoreErrors:
		- '#^(Function|Method) .+? return type has no value type specified in iterable type array#'
		- '#^(Function|Method) .+? has parameter .+? with no value type specified in iterable type array#'
		- '#^(Function|Method) .+? has no return type specified#'
		- '#^Function od_handle_rest_request\(\) has parameter \$request with generic class WP_REST_Request but does not specify its types: T#'
	dynamicConstantNames:
		- PERFLAB_OBJECT_CACHE_DROPIN_VERSION

The above issue was identified with level 5:

checking types of arguments passed to methods and functions

Nevertheless, PhpStorm's static analysis also flagged this as an error:

Compare with PHPStan's error being surfaced in PhpStorm (which was returned above via lint-staged):

Sure enough, when I simulate a failure in activate_plugin(), I did get a PHP warning:

We really should prioritize increasing the PHPStan level (#775) so we can catch these issues earlier than a couple days before the release.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>
Co-authored-by: mukeshpanchal27 <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[felixarntz]

@westonruter LGTM, though I'd prefer your alternative suggestion.

[mukeshpanchal27]

Nice catch, @westonruter. We should prioritize #775 as soon as possible.