-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade PHPStan to 1.11.6 #1325
Conversation
…processor and add tests
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. |
// Find the ID of each image by the class. | ||
// TODO: It would be preferable to use the $processor->class_list() method but there seems to be some typing issues with PHPStan. | ||
$class_name = $processor->get_attribute( 'class' ); | ||
if ( ! is_string( $class_name ) || ! preg_match( '/(?:^|\s)wp-image-([1-9]\d*)(?:\s|$)/i', $class_name, $matches ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more robust than before since it constraints looking for the class name inside the class
attribute. Previously the string could be anywhere in the img
tag.
continue; | ||
} | ||
|
||
// Make sure we use the last item on the list of matches. | ||
$attachment_id = (int) $class_name[1]; | ||
|
||
if ( ! $attachment_id ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will never be zero because the regex is now [1-9]\d*
.
@@ -533,29 +533,28 @@ function webp_uploads_update_image_references( $content ): string { | |||
} | |||
|
|||
// This content does not have any tag on it, move forward. | |||
// TODO: Eventually this should use the HTML API to parse out the image tags and then update them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or, switch to using the wp_content_img_tag
filter? #1259
Looks good! some test failing? |
@adamsilverstein I re-ran the failed jobs. They're passing now. Seems like a wp-env hiccup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spiffy!
@@ -39,9 +39,9 @@ public function __invoke( OD_HTML_Tag_Walker $walker ): bool { | |||
if ( | |||
is_string( $style ) | |||
&& | |||
0 < (int) preg_match( '/background(-image)?\s*:[^;]*?url\(\s*[\'"]?\s*(?<background_image>.+?)\s*[\'"]?\s*\)/', $style, $matches ) | |||
1 === preg_match( '/background(?:-image)?\s*:[^;]*?url\(\s*[\'"]?\s*(?<background_image>.+?)\s*[\'"]?\s*\)/', $style, $matches ) // Not compatible with strict rules. See <https://github.com/phpstan/phpstan/issues/11262>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bah. I should have removed the comment as it is now irrelevant after 75cd653 per phpstan/phpstan#11262 (comment). I'll do so as part of another PR.
This upgrades PHPStan to 1.11.6 which introduced two new issues.
This is a case where PHPStan is not aware that at this point, the array keys are actually not nullable at this point. This is a bug in PHPStan which I've filed as phpstan/phpstan#11262. The issue seems to be that if the return value of
preg_match()
is casted to abool
orint
then the array shape doesn't come through. So it's now checking to see if the return value is exactly1
instead. Additionally, another check for'' !== $matches['background_image']
will be able to be removed once phpstan/phpstan#11222 is implemented, since we'll know that the array shape here has anon-empty-string
.There was addressing the second
if
statement in the following code:performance/plugins/webp-uploads/hooks.php
Lines 542 to 549 in b71d0a0
It is a useless condition because since
preg_match()
is known to have succeeded, the$class_name
will never be empty.This fixes the issue by removing that condition, but also makes the code more robust by leveraging the HTML Tag Processor. It also adds missing test assertions for
webp_uploads_update_image_references()
.