Conversation
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| if ( ! is_array( $metadata ) ) { | ||
| $metadata = array(); | ||
| } |
There was a problem hiding this comment.
Checks like this and the use of mixed as a possible param type are needed because plugins can add filters that stick any type in for $metadata.
8aaed27 to
b9356ad
Compare
| * @param string $output_format The image editor default output format mapping. | ||
| * @param string $filename Path to the image. | ||
| * @param string $mime_type The source image mime type. |
There was a problem hiding this comment.
Note that webp_uploads_filter_image_editor_output_format() is added as a filter for image_editor_output_format. This filter is defined as taking these params:
* @param string[] $output_format {
* An array of mime type mappings. Maps a source mime type to a new
* destination mime type. Default empty array.
*
* @type string ...$0 The new mime type.
* }
* @param string $filename Path to the image.
* @param string $mime_type The source image mime type.
So the use of string here for $output_format is incorrect. It should have been array<string, string> in the plugin, and in core string[] is not as specific as it could be (since it implies numeric keys).
Additionally, I tried using just string PHP types for $filename and $mimetype, but when running unit tests I got a failure. This is because the function that applies the filter is \WP_Image_Editor::get_output_format() and this function takes null as default values for $filename and $mime_type, which is what is passed when using a factory to create an image during testing. So core should actually be typing the filter:
* @param string|null $filename Path to the image.
* @param string|null $mime_type The source image mime type.
These are accounted for with the new function signature for webp_uploads_filter_image_editor_output_format().
| $request = new WP_REST_Request(); | ||
| $request['id'] = $attachment_id; |
There was a problem hiding this comment.
PHPStan was complaining here:
37 Cannot assign offset 'id' to WP_REST_Request<array>.
37 WP_REST_Request<array> does not accept int|WP_Error.
Apparently WP_REST_Request is not typed sufficiently in core for PHPStan to accept setting an array offset. So opting for the set_param() method is an easy fix for this.
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @param int $attachment_id Attachment post ID. Defaults to global $post. |
There was a problem hiding this comment.
| * @param int $attachment_id Attachment post ID. Defaults to global $post. | |
| * @param int $attachment_id Attachment post ID. Defaults 0. |
There was a problem hiding this comment.
True, but wp_get_attachment_metadata defaults to $post which is what this function will do when zero is passed. I copied this phpdoc from core.
There was a problem hiding this comment.
Actually, instead of adding a helper wrapper function with the type definition, maybe it would be better to add a stub function definition for PHPStan to read?
There was a problem hiding this comment.
Yes, a stub is better for consistent PHPDoc comments.
There was a problem hiding this comment.
I've implemented the stub in 3e8fed7 which improved a lot.
|
|
||
| /** | ||
| * Metadata potentially amended by webp_uploads_create_sources_property(). | ||
| * | ||
| * Note the sources key is not normally present in the response for wp_get_attachment_metadata(). The sources | ||
| * key here, however, is being injected via the 'wp_generate_attachment_metadata' filter via the | ||
| * webp_uploads_create_sources_property() function. | ||
| * | ||
| * @see webp_uploads_create_sources_property() | ||
| * | ||
| * @var array{ | ||
| * width: int, | ||
| * height: int, | ||
| * file: non-falsy-string, | ||
| * sizes: array, | ||
| * image_meta: array, | ||
| * filesize: int, | ||
| * sources?: array<string, array{ file: string, filesize: int }> | ||
| * } $metadata | ||
| */ |
There was a problem hiding this comment.
This is all eliminated because of the introduction of the function stub which includes the additional typing for the array keys added by filters in the webp-uploads and dominant-color-images plugins.
See #775.
This fixes the following PHPStan issues:
[ERROR] Found 42 errors
Please review individual commits for changes.