Update PHPStan level to 6 #1203
Conversation
…neric class WP_REST_Request but does not specify its types: T
fc342b2 to
2d56764
Compare
| @@ -118,7 +118,7 @@ public static function get_post( string $slug ): ?WP_Post { | |||
| */ | |||
| public static function get_url_metrics_from_post( WP_Post $post ): array { | |||
| $this_function = __FUNCTION__; | |||
| $trigger_warning = static function ( $message ) use ( $this_function ) { | |||
| $trigger_warning = static function ( $message ) use ( $this_function ): void { | |||
There was a problem hiding this comment.
Just realized that a "trigger warning" is a noun too 😊
plugins/webp-uploads/helper.php
Outdated
| @@ -239,12 +239,13 @@ function webp_uploads_generate_image_size( $attachment_id, $size, $mime ) { | |||
| * Returns the attachment sources array ordered by filesize. | |||
| * | |||
| * @since 1.0.0 | |||
| * @todo This function is not used anywhere. | |||
There was a problem hiding this comment.
We should check for when this function was first introduced and when it became no longer used.
There was a problem hiding this comment.
I think it's better to maintain a separate file for deprecated functions?
| 'file' => 'leaves.webp', | ||
| 'filesize' => 1234, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Explicit typing uncovered the wrong data was being tested.
… update/phpstan-level-6 * 'trunk' of https://github.com/WordPress/performance: Add comments explaining why PHPStan is ignored Fix function stub docs
| "format:all": [ | ||
| "@format", | ||
| "@format:auto-sizes", | ||
| "@format:dominant-color-images", | ||
| "@format:embed-optimizer", | ||
| "@format:optimization-detective", | ||
| "@format:speculation-rules", | ||
| "@format:webp-uploads" | ||
| ], | ||
| "format:auto-sizes": "@format -- ./plugins/auto-sizes --standard=./plugins/auto-sizes/phpcs.xml.dist", | ||
| "format:dominant-color-images": "@format -- ./plugins/dominant-color-images --standard=./plugins/dominant-color-images/phpcs.xml.dist", | ||
| "format:embed-optimizer": "@format -- ./plugins/embed-optimizer --standard=./plugins/embed-optimizer/phpcs.xml.dist", | ||
| "format:optimization-detective": "@format -- ./plugins/optimization-detective --standard=./plugins/optimization-detective/phpcs.xml.dist", | ||
| "format:speculation-rules": "@format -- ./plugins/speculation-rules --standard=./plugins/speculation-rules/phpcs.xml.dist", | ||
| "format:webp-uploads": "@format -- ./plugins/webp-uploads --standard=./plugins/webp-uploads/phpcs.xml.dist", |
There was a problem hiding this comment.
@thelovekesh AFAIK this will be obsolete with what you've got pending in another PR.
There was a problem hiding this comment.
Yes @westonruter but that's still under consideration.
… update/phpstan-level-6 * 'trunk' of https://github.com/WordPress/performance: Add test case for when RGB values are too high Ensure RGB variable is int fix: avoid needless array allocation in rgb to hex conversion
| @@ -42,11 +42,14 @@ public function toString(): string { | |||
| /** | |||
| * Evaluates the constraint for the provided attachment ID. | |||
| * | |||
| * @param int $attachment_id Attachment ID. | |||
| * @param mixed $other Attachment ID. | |||
There was a problem hiding this comment.
not sure I understand why this is mixed
There was a problem hiding this comment.
Ah yes, it's in order to be compatible with the class that this class extends (\PHPUnit\Framework\Constraint\Constraint) which is:
/**
* Evaluates the constraint for parameter $other. Returns true if the
* constraint is met, false otherwise.
*
* This method can be overridden to implement the evaluation algorithm.
*
* @param mixed $other value or object to evaluate
*
* @codeCoverageIgnore
*/
protected function matches($other): bool
{
return false;
}
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
| @@ -15,10 +15,14 @@ | |||
| * | |||
| * @since 1.0.0 | |||
| * | |||
| * @param array $attr Attributes for the image markup. | |||
| * @return array The filtered attributes for the image markup. | |||
| * @param array<string, string>|mixed $attr Attributes for the image markup. | |||
There was a problem hiding this comment.
This is always expected to be an array. Would it be better to add strong typing to the param, or are you trying to avoid doing so because it is filtered content? I have similar questions about several other places where similar changes are being applied.
There was a problem hiding this comment.
Yes, because it is filtered another plugin would put anything here. So it seems safer to use mixed and then do runtime type checking to avoid a fatal error.
There was a problem hiding this comment.
Would you want a _doing_it_wrong() to be added in cases like this so that the error isn't silently suppressed?
There was a problem hiding this comment.
If the strategy is to treat all input from filters as $mixed then I think this is fine for consistency. Making sure that we're doing early type validation—as you're doing—seems like enough to me.
| public function it_should_return_an_error_when_creating_an_additional_image_source_with_invalid_parameters( $attachment_id, $size_data, $mime, $destination_file = null ) { | ||
| $this->assertInstanceOf( WP_Error::class, webp_uploads_generate_additional_image_source( $attachment_id, $size_data, $mime, $destination_file ) ); | ||
| public function it_should_return_an_error_when_creating_an_additional_image_source_with_invalid_parameters( int $attachment_id, string $image_size, array $size_data, string $mime ): void { | ||
| $this->assertInstanceOf( WP_Error::class, webp_uploads_generate_additional_image_source( $attachment_id, $image_size, $size_data, $mime ) ); |
There was a problem hiding this comment.
The arguments being passed were wrong here.
34bd0c7 to
6768c98
Compare
See #775.
This fixes the following PHPStan issues:
[ERROR] Found 381 errors
Please review individual commits for changes.