Deprecate resource mentions in GDToolkit method names

Thanks for working on this! I guess the patch is the latest/canonical version here and not the merge request, so I am going to close the MR for clarity.

I made some small improvements to the change record.

1. +++ b/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php
   @@ -28,11 +28,9 @@
   -  protected $resource = NULL;
   +  protected ?\GdImage $image = NULL;
   

   This is, technically, an internal BC break to make the typehint stricter. I think it is probably low-risk, but should be restricted to a minor only and maybe be mentioned in the CR.

2. +++ b/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php
   @@ -112,6 +110,47 @@ public static function create(ContainerInterface $container, array $configuratio
   +    if ($name === 'resource') {
   ...
   +    if ($name == 'resource') {
   ...
   +    if ($name == 'resource') {
   ...
   +    if ($name == 'resource') {
   

   Why are some of these == but the others ===?

3. +++ b/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php
   @@ -112,6 +110,47 @@ public static function create(ContainerInterface $container, array $configuratio
   +      @trigger_error('Accessing the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.', E_USER_DEPRECATED);
   ...
   +      @trigger_error('Setting the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.', E_USER_DEPRECATED);
   ...
   +      @trigger_error('Checking the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.', E_USER_DEPRECATED);
   ...
   +      @trigger_error('Unsetting the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.', E_USER_DEPRECATED);
   

   Method names in all the deprecation messages should have parens. OTOH the correct way to document PHP member properties is \Drupal\whatever\Class::$property. Both ::resource and ::image need to be fixed one way or the other.

   Also, these seem to refer to an image method or property that does not exist?

4. +++ b/core/tests/Drupal/Tests/Component/Utility/RectangleTest.php
   @@ -77,13 +77,10 @@ public function testRotateDimensions($width, $height, $angle, $exp_width, $exp_h
   -   *     if (is_resource($old_res)) {
   -   *       imagedestroy($old_res);
   -   *     }
   

   I wondered why this is being removed from the example code. Answer above is:

   imagedestroy is a noop since PHP 8.0, we can remove it and adjust the inline comment.

   So that's fine.

NW mainly for #2, although the CR could use some expansion about deprecated properties/stricter property types and etc. as well. Thanks!

Saving credits for reviewers.

+++ b/core/tests/Drupal/KernelTests/Core/Image/ToolkitGdTest.php
@@ -460,9 +460,9 @@ public function testGifTransparentImages(): void {
+    $gd_mage = $image->getToolkit()->getImage();
+    $color_index = imagecolorat($gd_mage, $image->getWidth() - 1, 0);
+    $color = array_values(imagecolorsforindex($gd_mage, $color_index));

@@ -473,17 +473,17 @@ public function testGifTransparentImages(): void {
+    $gd_mage = $image->getToolkit()->getImage();
+    $color_index = imagecolorat($gd_mage, $image->getWidth() - 1, 0);
+    $color = array_values(imagecolorsforindex($gd_mage, $color_index));

One more thing -- the local variable here is mis-named. While it is kind of magic, I think this should be "image" and not "mage". 🧙‍♂️

Addressed 43.2 and 45.
Please review.

re-queued and updated CR with s/resource/image internal BC break

Re-roll after 🐛 GD toolkit & operations should catch \Throwable to fail gracefully in case of errors Fixed

The changes in patch #48 look good.

Wonder if we can update the IS and CR with the additional things being deprecated outside the 2 mentioned.

Added to summary rename of s/resource/image - there's no other changes

the name of the methods on the CR is outdated

ah, yes, s/GdImage/Image - updated IS and CR

+++ b/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php
@@ -28,11 +28,9 @@
-  protected $resource = NULL;
+  protected ?\GdImage $image = NULL;

@@ -135,12 +178,43 @@ public function setResource($resource) {
+  public function setImage(?\GdImage $image): static {
...
+  public function getImage(): ?\GdImage {

The last submitted patch, 48: 3265953-48.patch, failed testing. View results →

Seems random failure.

The last submitted patch, 48: 3265953-48.patch, failed testing. View results →

I have updated the CR for readability and converted to a table.

+++ b/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php
@@ -112,6 +110,47 @@ public static function create(ContainerInterface $container, array $configuratio
+      @trigger_error('Accessing the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.', E_USER_DEPRECATED);
...
+      @trigger_error('Setting the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.', E_USER_DEPRECATED);
...
+      @trigger_error('Checking the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.', E_USER_DEPRECATED);
...
+      @trigger_error('Unsetting the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.', E_USER_DEPRECATED);

@@ -120,14 +159,18 @@ public static function create(ContainerInterface $container, array $configuratio
+   * @deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use
...
+    @trigger_error(__METHOD__ . '() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::setImage() instead. See https://www.drupal.org/node/3265963', E_USER_DEPRECATED);

@@ -135,12 +178,43 @@ public function setResource($resource) {
+   * @deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use
...
+    @trigger_error(__METHOD__ . '() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::getImage() instead. See https://www.drupal.org/node/3265963', E_USER_DEPRECATED);

+++ b/core/tests/Drupal/KernelTests/Core/Image/ToolkitGdTest.php
@@ -532,4 +532,29 @@ public function testGetRequirements(): void {
+    $this->expectDeprecation('Drupal\system\Plugin\ImageToolkit\GDToolkit::setResource() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::setImage() instead. See https://www.drupal.org/node/3265963');
...
+    $this->expectDeprecation('Checking the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.');
...
+    $this->expectDeprecation('Drupal\system\Plugin\ImageToolkit\GDToolkit::getResource() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::getImage() instead. See https://www.drupal.org/node/3265963');
...
+    $this->expectDeprecation('Accessing the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.');
...
+    $this->expectDeprecation('Setting the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.');
...
+    $this->expectDeprecation('Unsetting the \Drupal\system\Plugin\ImageToolkit\GDToolkit::resource property is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. Use \Drupal\system\Plugin\ImageToolkit\GDToolkit::image instead.');

needs update to 10.2.0

Updated

Change looks good. Hopefully can get in early for 10.2

Committed and pushed 3b44eb23976 to 11.x. Thanks!

andypost → closed merge request !1869

Published the change record.

Automatically closed - issue fixed for 2 weeks with no activity.

Updated the CR to say 10.2 since it doesn't (appear) to have landed in 10.1.