Add PHPStan strict rules (except for empty.notAllowed) by westonruter · Pull Request #1241 · WordPress/performance · GitHub

Note: This does not fix the empty.notAllowed issues since they are more complicated and deserve a separate PR.

Just adding phpstan-strict-rules results in the following 863 errors in the codebase:

    712 staticMethod.dynamicCall
     60 empty.notAllowed
     36 booleanNot.exprNotBoolean
     21 if.condNotBoolean
     11 booleanAnd.leftNotBoolean
      5 cast.useless
      4 booleanOr.rightNotBoolean
      4 arrayFilter.strict
      3 method.childReturnType
      2 ternary.condNotBoolean
      2 booleanAnd.rightNotBoolean
      1 variable.implicitArray
      1 plus.leftNonNumeric
      1 elseif.condNotBoolean

This report was generated via:

npm run phpstan -- -- -v | grep 🪪 | cut -c16- | sed 's/ *$//' | sort | uniq -c | sort -nr

When ignoring staticMethod.dynamicCall, booleanNot.exprNotBoolean, and if.condNotBoolean in tests, this goes down to just 144 errors:

     60 empty.notAllowed
     34 booleanNot.exprNotBoolean
     16 if.condNotBoolean
     11 booleanAnd.leftNotBoolean
      5 cast.useless
      4 booleanOr.rightNotBoolean
      4 arrayFilter.strict
      3 method.childReturnType
      2 ternary.condNotBoolean
      2 booleanAnd.rightNotBoolean
      1 variable.implicitArray
      1 plus.leftNonNumeric
      1 elseif.condNotBoolean

The following remain to be addressed:

empty.notAllowed (60)
booleanNot.exprNotBoolean (34)
if.condNotBoolean (16)
booleanAnd.leftNotBoolean (11)
cast.useless (5)
booleanOr.rightNotBoolean (4)
arrayFilter.strict (4) (rule disabled)
method.childReturnType (3)
ternary.condNotBoolean (2)
booleanAnd.rightNotBoolean (2)
variable.implicitArray (1)
plus.leftNonNumeric (1)
elseif.condNotBoolean (1)

…ndNotBoolean for tests

The one last PHPStan error remaining is empty.notAllowed. There are 60 instances of this.

I'm not sure why this issue is being reported for assertion method calls in 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 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: swissspidy <swissspidy@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.

This looks good to me. I've added some suggestions, but none blocking so I'm pre-approving. Feel free to apply or ignore.

Same as above.

Suggested change

      
            			if ( is_string( $html_processor->get_attribute( 'type' ) ) ) {
          
            			if ( null !== $html_processor->get_attribute( 'type' ) ) ) {

In this case, I think the code should stay as-is. Since get_attribute can return true in the case of a boolean attribute, we wouldn't want to try passing a true to set_attrbute(). I believe PHPStan would complain about that anyway.

I could go either way here, but for consistency...

Suggested change

      
            	if ( is_string( $href ) && '' !== $href ) {
          
            	if ( null !== $href && '' !== $href ) {

This should stay as is because if <a href> is encountered, then $href here would be true which would be a type error since the function returns a string or null.

This is such a common pattern in WP that I wouldn't even think of needing to specify, but makes sense.

Could just return the value early in this case.

Suggested change

      
            	if ( '' === $resource_url ) {
          
            		return '';
          
            	if ( '' === $resource_url ) {
          
            		return $resource_url;

Co-authored-by: Joe McGill <801097+joemcgill@users.noreply.github.com>

LGTM, pending the merge conflict resolution

… add/phpstan-strict-rules * 'trunk' of https://github.com/WordPress/performance: (206 commits) fix(webp-uploads): prevent picture element when JPEG output disabled Prepare 0.4.1 release of Optimization Detective Update attribute order in Image Prioritizer test after eliminating excessive seek() calls Account for visitors calling next_token() not just seek() Update attribute order in embed-optimizer test due to not seeking Add test to ensure tag visitation does not exceed seek limit Fix attribute order in tests now that seeking is not happening Add missing bookmark name to warning message Only seek back to the current tag if a tag visitor seeked Migrate commander for v12 Upgrade commander to 12.1.0 Bump yoast/phpunit-polyfills from 1.1.1 to 2.0.1 Bump @wordpress/scripts from 26.19.0 to 28.3.0 Bump @wordpress/env from 9.6.0 to 10.3.0 Migrate husky Update Octokit to use dynamic import Bump husky from 8.0.3 to 9.1.0 Add labels for Dependabot PRs Bump web-vitals from 3.5.0 to 4.2.1 Bump phpstan/extension-installer from 1.3.1 to 1.4.1 ...

… add/phpstan-strict-rules * 'trunk' of https://github.com/WordPress/performance: Fix OD test case for WP 6.7-alpha

Suggested change

      
            			1 !== preg_match( '/(?:^|\s)wp-image-([1-9]\d*)(?:\s|$)/i', $class_name, $matches )
          
            			1 !== (int) preg_match( '/(?:^|\s)wp-image-([1-9]\d*)(?:\s|$)/i', $class_name, $matches )

Do we needs to cast int here?

No, because it can either be an int or false, and we're checking to see if it is not identical to the value.

The use of (int) isn't needed here either...

Well, actually it is. Because it's preg_match_all() so we don't know the number of image tags being matched. Otherwise, it could be 1 === .... So by casting the return value to (int), it will convert both 0 and false to 0. Alternatively, this could be:

Suggested change

      
            	if ( 0 === (int) preg_match_all( '/<(img)\s[^>]+>/', $content, $img_tags, PREG_SET_ORDER ) ) {
          
            	if ( false === (bool) preg_match_all( '/<(img)\s[^>]+>/', $content, $img_tags, PREG_SET_ORDER ) ) {

But false indicates the error state. The integer return value is the normal case, the number of matches.

Thanks @westonruter, LGTM!

@westonruter Could you please merge as it not allow me. Shows that Merging can be performed automatically with 1 approving review.

westonruter added this to the performance-lab 3.2.0 milestone May 23, 2024

westonruter force-pushed the add/phpstan-strict-rules branch from 031d197 to 220bdfa Compare May 23, 2024 22:45

westonruter added the [Type] Enhancement A suggestion for improvement of an existing feature label May 23, 2024

westonruter force-pushed the add/phpstan-strict-rules branch from 1f74e35 to 0dc2c63 Compare May 23, 2024 23:11

westonruter mentioned this pull request May 24, 2024

Add picture element support #73

Merged

westonruter modified the milestones: performance-lab 3.2.0, performance-lab n.e.x.t Jun 4, 2024

westonruter added 9 commits June 13, 2024 10:30

Add phpstan-strict-rules

94f4f22

Ignore staticMethod.dynamicCall, booleanNot.exprNotBoolean, and if.co…

211d192

…ndNotBoolean for tests

Fix cast.useless

f2c5120

Fix method.childReturnType

0c5c42b

Fix plus.leftNonNumeric

66dd4d6

Disable strictArrayFilter rule

db11733

Fix variable.implicitArray

165b15b

Fix instance of booleanAnd.leftNotBoolean

d3e0f22

Update phpstan to 1.11.4

db82f1e

westonruter force-pushed the add/phpstan-strict-rules branch from 00f04f7 to db82f1e Compare June 13, 2024 08:35

westonruter added 6 commits June 13, 2024 10:57

Fix interface.nameCase

7de2d96

Fix booleanNot.exprNotBoolean

87ecaad

Fix ternary.condNotBoolean

44d8375

Fix booleanOr.rightNotBoolean

94884c5

Fix booleanAnd.leftNotBoolean

5b5cd68

Fix if.condNotBoolean

dad5312

westonruter commented Jun 13, 2024

View reviewed changes

Ignore empty.notAllowed temporarily

228035f

westonruter changed the title Add PHPStan strict rules Add PHPStan strict rules (except for empty.notAllowed) Jun 13, 2024

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

westonruter requested review from adamsilverstein and felixarntz as code owners June 13, 2024 10:13

westonruter requested review from pbearne and spacedmonkey as code owners June 13, 2024 10:13

westonruter requested a review from mukeshpanchal27 June 13, 2024 10:13

westonruter requested a review from swissspidy June 13, 2024 10:14

westonruter mentioned this pull request Jul 1, 2024

Upgrade PHPStan to 1.11.6 #1325

Merged

westonruter modified the milestones: performance-lab 3.3.0, performance-lab n.e.x.t Jul 9, 2024

joemcgill approved these changes Jul 11, 2024

View reviewed changes

Treat a boolean src attribute as if it is an inline script

831105c

Co-authored-by: Joe McGill <801097+joemcgill@users.noreply.github.com>

westonruter mentioned this pull request Jul 17, 2024

Fix setting sizes attribute on PICTURE > SOURCE elements #1354

Merged

swissspidy approved these changes Jul 22, 2024

View reviewed changes

westonruter added 4 commits July 22, 2024 13:43

Fix remaining strict rule violations

0413c60

Merge branch 'trunk' of https://github.com/WordPress/performance into…

350ddbf

… add/phpstan-strict-rules * 'trunk' of https://github.com/WordPress/performance: Fix OD test case for WP 6.7-alpha

Fix and improve conditions in auto_sizes_filter_image_tag and add typing

d4af3a6

mukeshpanchal27 reviewed Jul 23, 2024

View reviewed changes

westonruter commented Jul 23, 2024

View reviewed changes

Comment thread plugins/webp-uploads/hooks.php Outdated

Remove unnecessary casting to int

8e90f62

westonruter commented Jul 23, 2024

View reviewed changes

Comment thread tests/bootstrap.php Outdated

westonruter added 2 commits July 22, 2024 21:36

Remove unnecessary casting

4f4f6bd

Fix logic inversion

eaf1f65

mukeshpanchal27 approved these changes Jul 23, 2024

View reviewed changes

westonruter merged commit de93b65 into trunk Jul 23, 2024

westonruter deleted the add/phpstan-strict-rules branch July 23, 2024 05:10

This was referenced Jul 23, 2024

Eliminate use of empty() in Embed Optimizer and Optimization Detective #1391

Merged

Add strict PHPStan rules for Speculative Loading #1392

Merged

westonruter mentioned this pull request Jul 11, 2025

Increase PHPStan to rule level to 1 (and beyond) #775

Closed

13 tasks

               		add_action( 'wp_footer', 'embed_optimizer_lazy_load_scripts' );
               		if ( $html_processor->seek( 'script' ) ) {
-              			if ( $html_processor->get_attribute( 'type' ) ) {
+              			if ( is_string( $html_processor->get_attribute( 'type' ) ) ) {

               	// 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.
-              	if ( ! preg_match_all( '/<(img)\s[^>]+>/', $content, $img_tags, PREG_SET_ORDER ) ) {
+              	if ( 0 === (int) preg_match_all( '/<(img)\s[^>]+>/', $content, $img_tags, PREG_SET_ORDER ) ) {

               			identifier: cast.string
               			path: */tests/*
+              		-
+              			identifier: staticMethod.dynamicCall

	if ( is_string( $html_processor->get_attribute( 'type' ) ) ) {
	if ( null !== $html_processor->get_attribute( 'type' ) ) ) {

               	}
               	$href = $p->get_attribute( 'href' );
-              	if ( $href && is_string( $href ) ) {
+              	if ( is_string( $href ) && '' !== $href ) {

	if ( is_string( $href ) && '' !== $href ) {
	if ( null !== $href && '' !== $href ) {

               		}
-              		if ( did_action( 'perflab_server_timing_send_header' ) && ! doing_action( 'perflab_server_timing_send_header' ) ) {
+              		if ( 0 !== did_action( 'perflab_server_timing_send_header' ) && ! doing_action( 'perflab_server_timing_send_header' ) ) {

	1 !== preg_match( '/(?:^\|\s)wp-image-([1-9]\d*)(?:\s\|$)/i', $class_name, $matches )
	1 !== (int) preg_match( '/(?:^\|\s)wp-image-([1-9]\d*)(?:\s\|$)/i', $class_name, $matches )

	if ( 0 === (int) preg_match_all( '/<(img)\s[^>]+>/', $content, $img_tags, PREG_SET_ORDER ) ) {
	if ( false === (bool) preg_match_all( '/<(img)\s[^>]+>/', $content, $img_tags, PREG_SET_ORDER ) ) {

Add PHPStan strict rules (except for empty.notAllowed) by westonruter · Pull Request #1241 · WordPress/performance · GitHub

Uh oh!

Add PHPStan strict rules (except for empty.notAllowed) - #1241

Add PHPStan strict rules (except for empty.notAllowed)#1241westonruter merged 24 commits intotrunkWordPress/performance:trunkfrom add/phpstan-strict-rulesWordPress/performance:add/phpstan-strict-rulesCopy head branch name to clipboard

Conversation

westonruter commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

westonruter commented Jun 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joemcgill left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swissspidy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mukeshpanchal27 left a comment

Choose a reason for hiding this comment

Uh oh!

mukeshpanchal27 commented Jul 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add PHPStan strict rules (except for empty.notAllowed)#1241
westonruter merged 24 commits into
trunkfrom
add/phpstan-strict-rules

westonruter commented May 23, 2024 •
edited
Loading

github-actions Bot commented Jun 13, 2024 •
edited
Loading