Update PHPStan to 1.11.5 #1318

westonruter · 2024-06-24T21:50:44Z

The following PHPStan failures are occurring in PHPStan 1.11.5:

 ------ --------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   plugins/dominant-color-images/class-dominant-color-image-editor-gd.php                                                                             
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------- 
  37     Strict comparison using === between false and int<1, max> will always evaluate to false.                                                           
         🪪  identical.alwaysFalse                                                                                                                          
  37     Strict comparison using === between false and int<1, max> will always evaluate to false.                                                           
         🪪  identical.alwaysFalse                                                                                                                          
  81     Call to function is_array() with array{red: int<0, 255>, green: int<0, 255>, blue: int<0, 255>, alpha: int<0, 127>} will always evaluate to true.  
         🪪  function.alreadyNarrowedType                                                                                                                   
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------------- 
  Line   plugins/webp-uploads/tests/test-image-edit.php                                              
 ------ -------------------------------------------------------------------------------------------- 
  53     Strict comparison using === between 'full-orig' and *NEVER* will always evaluate to false.  
         🪪  identical.alwaysFalse                                                                   
 ------ --------------------------------------------------------------------------------------------

westonruter · 2024-06-24T22:09:41Z

plugins/dominant-color-images/class-dominant-color-image-editor-gd.php

@@ -32,11 +32,11 @@ public function get_dominant_color() {
 		}
 		// The logic here is resize the image to 1x1 pixel, then get the color of that pixel.
 		$shorted_image = imagecreatetruecolor( 1, 1 );
-		$image_width   = imagesx( $this->image );
-		$image_height  = imagesy( $this->image );
-		if ( false === $shorted_image || false === $image_width || false === $image_height ) {


In PhpStorm's stubs, it defines:

/** * Create a new true color image * @link https://php.net/manual/en/function.imagecreatetruecolor.php * @param int $width * Image width. * * @param int $height * Image height. * * @return resource|GdImage|false an image resource identifier on success, false on errors. */ #[Pure] function imagecreatetruecolor(int $width, int $height): GdImage|false {} /** * Get image width * @link https://php.net/manual/en/function.imagesx.php * @param resource|GdImage $image * @return int|false Return the width of the image or false on * errors. */ #[Pure] function imagesx(GdImage $image): int {} /** * Get image height * @link https://php.net/manual/en/function.imagesy.php * @param resource|GdImage $image * @return int|false Return the height of the image or false on * errors. */ #[Pure] function imagesy(GdImage $image): int {}

All three have phpdoc that say they can return false, but only imagecreatetruecolor() has the PHP type hint which says it can return false, and this is confirmed in the PHP docs: https://www.php.net/manual/en/function.imagecreatetruecolor.php

In contrast, the PHP docs for the other functions say nothing about them returning false:

https://www.php.net/manual/en/function.imagesx.php
https://www.php.net/manual/en/function.imagesy.php

So it seems PhpStorm's docs are incorrect.

westonruter · 2024-06-24T22:10:46Z

plugins/dominant-color-images/class-dominant-color-image-editor-gd.php

-				if ( ! is_array( $rgba ) ) {
-					return new WP_Error( 'unable_to_obtain_rgba_via_imagecolorsforindex' );
-				}


PHP docs say that this function only returns an array: https://www.php.net/manual/en/function.imagecolorsforindex.php

But PhpStorm's stubs say it can return false:

/** * Get the colors for an index * @link https://php.net/manual/en/function.imagecolorsforindex.php * @param resource|GdImage $image * @param int $color * The color index. * * @return array|false an associative array with red, green, blue and alpha keys that * contain the appropriate values for the specified color index or FALSE on failure */ #[Pure] #[LanguageLevelTypeAware(['8.0' => 'array'], default: 'array|false')] #[ArrayShape(["red" => "int", "green" => "int", "blue" => "int", "alpha" => "int"])] function imagecolorsforindex(GdImage $image, int $color) {}

Again, it seems PhpStorm is wrong.

According to the changelog:

imagecolorsforindex() now throws a ValueError exception if color is out of range; previously, false was returned instead.

I wonder what versions of GD we need to support?

Ah, then we should probably wrap this in a try/catch block to account for this.

westonruter · 2024-06-24T22:11:41Z

plugins/webp-uploads/tests/test-image-edit.php

 		foreach ( $backup_sizes as $size => $properties ) {
 			$size_name = str_replace( '-orig', '', $size );
-
-			if ( 'full-orig' === $size ) {


I have no idea why PHPStan is saying that this is an error:

Strict comparison using === between 'full-orig' and NEVER will always evaluate to false.

In any case, it seems better to just unset the array key rather than to check if with each iteration.

westonruter · 2024-06-24T22:12:08Z

plugins/webp-uploads/tests/test-image-edit.php

-
-		$metadata = wp_get_attachment_metadata( $attachment_id );


This appears to be dead code, perhaps code which was going to have assertions added to it, but they never were written.

westonruter · 2024-06-24T22:12:28Z

plugins/webp-uploads/tests/test-image-edit.php

@@ -136,7 +130,7 @@ public function test_it_should_prevent_to_back_up_the_sources_when_the_sources_a
 		$backup_sizes = get_post_meta( $attachment_id, '_wp_attachment_backup_sizes', true );
 		$this->assertIsArray( $backup_sizes );

-		foreach ( $backup_sizes as $size_name => $properties ) {


Removing unused variable.

westonruter · 2024-06-24T22:12:32Z

plugins/webp-uploads/tests/test-image-edit.php

@@ -281,7 +275,7 @@ static function () {
 		foreach ( $metadata['sizes'] as $size_name => $properties ) {
 			$this->assertImageNotHasSizeSource( $attachment_id, $size_name, 'image/jpeg' );
 			$this->assertImageHasSizeSource( $attachment_id, $size_name, 'image/webp' );
-			foreach ( $properties['sources'] as $mime_type => $values ) {


Removing unused variable.

github-actions · 2024-06-24T22:13:39Z

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 <westonruter@git.wordpress.org>
Co-authored-by: joemcgill <joemcgill@git.wordpress.org>
Co-authored-by: mukeshpanchal27 <mukesh27@git.wordpress.org>

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

joemcgill

Looks good to me.

mukeshpanchal27

The changes look good to me, and I pre-approve the PR. There is one piece of feedback that needs to be addressed: #1318 (comment)

mukeshpanchal27 · 2024-06-25T06:41:35Z

@westonruter @joemcgill I recommend that we stick to a specific PHPStan version for now, as it blocks the progress of other PRs. When I worked on #1252, I encountered similar errors, and it didn't allow me to push changes without resolving these PHPStan errors.

Sticking to the current version will prevent blocking other PRs and we can plan to update eventually when a new version comes up, ensuring a smooth workflow.

westonruter · 2024-06-25T16:45:22Z

@mukeshpanchal27 I don't know if I agree. When would we upgrade then? Resolving PHPStan errors should in general just make the code better. In your PR #1252 you may have gotten PHPStan errors if you switched to this branch, ran composer install, and then switched back to your other branch without doing composer install again. However, GitHub Actions shouldn't have been reporting any errors against trunk since it is still pinned on the older version of PHPStan.

westonruter added 3 commits June 24, 2024 14:50

Update PHPStan to 1.11.5

0b8031a

Eliminate checks for seemingly impossible false return values

fd35a2b

Fix static analysis issues in Test_WebP_Uploads_Image_Edit

60dc6f4

westonruter force-pushed the update/phpstan-1.11.5 branch from fea003c to 60dc6f4 Compare June 24, 2024 22:07

westonruter commented Jun 24, 2024

View reviewed changes

westonruter added the [Type] Bug An existing feature is broken label Jun 24, 2024

westonruter added this to the dominant-color-images n.e.x.t milestone Jun 24, 2024

westonruter marked this pull request as ready for review June 24, 2024 22:13

westonruter requested review from adamsilverstein, felixarntz, pbearne and spacedmonkey as code owners June 24, 2024 22:13

westonruter added the [Plugin] Image Placeholders Issues for the Image Placeholders plugin (formerly Dominant Color Images) label Jun 24, 2024

Fix grammar typos

0ccc991

westonruter requested a review from swissspidy June 24, 2024 22:36

joemcgill approved these changes Jun 25, 2024

View reviewed changes

mukeshpanchal27 approved these changes Jun 25, 2024

View reviewed changes

Add try/catch block around imagecolorsforindex()

f6b49ae

westonruter added 2 commits June 25, 2024 09:51

Elaborate on comment explaining why type casting is added

f0ab803

Further refine command explaining type casting

d86eda0

westonruter merged commit 49beab8 into trunk Jun 25, 2024
13 checks passed

westonruter deleted the update/phpstan-1.11.5 branch June 25, 2024 18:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update PHPStan to 1.11.5 #1318

Update PHPStan to 1.11.5 #1318

westonruter commented Jun 24, 2024 •

edited

Loading

westonruter Jun 24, 2024

westonruter Jun 24, 2024

joemcgill Jun 25, 2024 •

edited

Loading

westonruter Jun 25, 2024

westonruter Jun 24, 2024

westonruter Jun 24, 2024

westonruter Jun 24, 2024

westonruter Jun 24, 2024

github-actions bot commented Jun 24, 2024 •

edited

Loading

joemcgill left a comment

mukeshpanchal27 left a comment

mukeshpanchal27 commented Jun 25, 2024

westonruter commented Jun 25, 2024

Update PHPStan to 1.11.5 #1318

Update PHPStan to 1.11.5 #1318

Conversation

westonruter commented Jun 24, 2024 • edited Loading

westonruter Jun 24, 2024

Choose a reason for hiding this comment

westonruter Jun 24, 2024

Choose a reason for hiding this comment

joemcgill Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

westonruter Jun 25, 2024

Choose a reason for hiding this comment

westonruter Jun 24, 2024

Choose a reason for hiding this comment

westonruter Jun 24, 2024

Choose a reason for hiding this comment

westonruter Jun 24, 2024

Choose a reason for hiding this comment

westonruter Jun 24, 2024

Choose a reason for hiding this comment

github-actions bot commented Jun 24, 2024 • edited Loading

joemcgill left a comment

Choose a reason for hiding this comment

mukeshpanchal27 left a comment

Choose a reason for hiding this comment

mukeshpanchal27 commented Jun 25, 2024

westonruter commented Jun 25, 2024

westonruter commented Jun 24, 2024 •

edited

Loading

joemcgill Jun 25, 2024 •

edited

Loading

github-actions bot commented Jun 24, 2024 •

edited

Loading