[wp-trac] [WordPress Trac] #50833: [PHP 8] GD resources are GdImage object instances

[WordPress Trac]

#50833: [PHP 8] GD resources are GdImage object instances
------------------------------+-----------------------------
 Reporter:  ayeshrajans       |       Owner:  SergeyBiryukov
     Type:  defect (bug)      |      Status:  reviewing
 Priority:  normal            |   Milestone:  5.6
Component:  Media             |     Version:
 Severity:  normal            |  Resolution:
 Keywords:  needs-patch php8  |     Focuses:
------------------------------+-----------------------------

Comment (by jrf):

 @ayeshrajans Thanks for initiating this.

 I've reviewed the attached patch and it mostly looks good & ready to
 commit.

 Five remarks, though each very small:

 1. The new `is_gd_image()` function in `src/wp-admin/includes/image.php`
 checks against `is_resource()`, which is the same as was previously done
 inline, but I wonder if this check should be enhanced with an additional
 check against [https://www.php.net/manual/en/function.get-resource-
 type.php get_resource_type()`].
 [https://www.php.net/manual/en/resource.php Based on the PHP docs], the
 applicable resource type should be `gd`.

 2. Along the same lines as 1, IMO a test should be added passing a non-
 GdImage resource to the `is_gd_image()` function to document the behaviour
 of the function in that situation.

 3. The function description for the new `is_gd_image()` function needs
 work as it can be misinterpreted as is. Alternatively, a good description
 for the `@return` tag could take away the confusion. It currently can be
 interpreted as if `true` would, for instance, mean it is a resource and
 `false` a GDImage object.
 > Returns whether the provided argument is of a resource, or a PHP 8
 GdImage object.
 > @return bool

 4. Nitpick: the parameter description alignment in the docblock of the
 `save()` function in `src/wp-includes/class-wp-image-editor-gd.php` is
 off. (4 spaces after longest param, should be 1 space). Also: the
 parameter description for the `$image` parameter of the `is_gd_image()`
 function is missing and AFAIK `@ticket` tags should only be used in the
 tests, not in the docblocks in `src`. Oh and the `@since` tag should have
 a complete version number, i.e. `5.6.0`.

 5. Only two of the four testcases need for the GD extension to be loaded,
 but the whole test is now being skipped if GD is not available. Consider
 using a data provider for the test cases, where one of the passed
 parameters could be whether GD is needed or not. If left as is, consider
 using a `@requires` annotation instead of the condition in the test
 itself.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/50833#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform