close
Skip to content

Fix PHPStan warnings and deprecated notices#1733

Merged
westonruter merged 5 commits intoWordPress:trunkfrom
ShyamGadde:fix/phpstan-warnings
Dec 12, 2024
Merged

Fix PHPStan warnings and deprecated notices#1733
westonruter merged 5 commits intoWordPress:trunkfrom
ShyamGadde:fix/phpstan-warnings

Conversation

@ShyamGadde
Copy link
Copy Markdown
Contributor

Summary

Fixes #1728

Signed-off-by: Shyamsundar Gadde <shyamsundar.gadde@rtcamp.com>
Signed-off-by: Shyamsundar Gadde <shyamsundar.gadde@rtcamp.com>
@ShyamGadde
Copy link
Copy Markdown
Contributor Author

ShyamGadde commented Dec 11, 2024

Leaving this as a draft since I don't have a complete explanation for why these changes fix the issues. However, as shown in the actions log, the warnings and deprecated notices no longer appear after these changes.

- plugins/performance-lab/load.php
bootstrapFiles:
- tools/phpstan/constants.php
- plugins/performance-lab/load.php
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s unclear to me why this file was included in the bootstrapFiles, considering PERFLAB_PLUGIN_DIR_PATH is already defined in tools/phpstan/constants.php:

define( 'PERFLAB_PLUGIN_DIR_PATH', __DIR__ . '/../../plugins/performance-lab/' );

After removing the file, the warnings about the duplicate constant definition no longer appear.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

@ShyamGadde ShyamGadde Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constant PERFLAB_PLUGIN_DIR_PATH was likely added to tools/phpstan/constants.php to avoid this error:

 ------ ----------------------------------------------------------------------------- 
  Line   plugins/performance-lab/includes/admin/load.php                              
 ------ ----------------------------------------------------------------------------- 
  232    Parameter #1 $value of function trailingslashit expects string, null given.  
 ------ ----------------------------------------------------------------------------- 


 [ERROR] Found 1 error  

Currently, both constants.php and plugins/performance-lab/load.php define PERFLAB_PLUGIN_DIR_PATH. However, the one in plugins/performance-lab/load.php evaluates to null:

define( 'PERFLAB_MAIN_FILE', __FILE__ );
define( 'PERFLAB_PLUGIN_DIR_PATH', plugin_dir_path( PERFLAB_MAIN_FILE ) );

This issue also becomes apparent if the order of files in bootstrapFiles is changed as follows:

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
index c66cc423..9c617d12 100644
--- a/phpstan.neon.dist
+++ b/phpstan.neon.dist
@@ -9,9 +9,9 @@ parameters:
 		- performance.php
 		- plugins/performance-lab/load.php
 	bootstrapFiles:
-		- tools/phpstan/constants.php
 		- plugins/performance-lab/load.php
+		- tools/phpstan/constants.php
 		- plugins/webp-uploads/load.php
 	scanDirectories:
 		- vendor/wp-phpunit/wp-phpunit/
 	scanFiles:

With this change, both the warning and the error occur:

PHP Warning:  Constant PERFLAB_PLUGIN_DIR_PATH already defined in /home/shyam/Workspace/rtCamp/wp-performance/tools/phpstan/constants.php on line 21
Warning: Constant PERFLAB_PLUGIN_DIR_PATH already defined in /home/shyam/Workspace/rtCamp/wp-performance/tools/phpstan/constants.php on line 21
 206/206 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ----------------------------------------------------------------------------- 
  Line   plugins/performance-lab/includes/admin/load.php                              
 ------ ----------------------------------------------------------------------------- 
  232    Parameter #1 $value of function trailingslashit expects string, null given.  
 ------ ----------------------------------------------------------------------------- 

 [ERROR] Found 1 error                                                                                                  

Since none of the other constants defined in plugins/performance-lab/load.php appear to be required for static analysis, removing this file from bootstrapFiles seems appropriate.

@mukeshpanchal27 mukeshpanchal27 added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release labels Dec 11, 2024
@mukeshpanchal27
Copy link
Copy Markdown
Member

The shivammathur/setup-php@v2 use latest PHP version and it was PHP 8.4.1 and that produces deprecated notices.

WordPress is not fully support in that version do we needs to change that version to 8.0 or 8.2?

@ShyamGadde ShyamGadde marked this pull request as ready for review December 12, 2024 13:06
@github-actions
Copy link
Copy Markdown

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: ShyamGadde <shyamgadde@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.

@westonruter westonruter merged commit 6fbbaae into WordPress:trunk Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release [Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Running PHPStan produces warnings and deprecated notices

4 participants