search

OllieJones / fast-woo-order-lookup

WooCommerce / WordPress plugin to speed up searches for orders
GNU General Public License v2.0
13 stars 1 forks source link

Integration of Composer and PHP CodeSniffer for Improved Standards Compliance #24

Open MaximeMichaud opened 2 months ago

MaximeMichaud commented 2 months ago

Hello,

This pull request represents a more substantial update compared to my last submission. I have integrated Composer, which I had only used sparingly before, but I have learned a lot about it in the past few hours. After reviewing several popular plugins, I decided to implement Composer for this plugin as well.

In this pull request, not everything is perfect, and there may be adjustments needed in the long term, but for now, it seems adequate.

Key Changes:

  1. PHP CodeSniffer Integration:

    • I have included a custom phpcs.xml file. This configuration is not overly strict and is aligned with WordPress coding standards. Please review it to determine if you'd like to adjust the strictness.

  2. Composer Configuration:

    • The composer.json file has been expanded to include more than what is currently necessary, foreseeing future needs. This should not cause any issues at present.
    • I have updated the .gitignore to properly exclude the vendor directory and included the composer.lock file to ensure consistent dependency versions across all setups.

  3. Dependency Management:

    • Added a dependabot.yml configuration for Composer to regularly check and update dependencies, ensuring optimal and secure operation.

  4. Linting Workflow:

    • Introduced lint_phpcs.yml, utilizing a matrix strategy to check PHP versions from 7.3 to 8.3. It's worth noting that versions below PHP 7.3 do not support the dependencies, and PHP 7.3 has been nearing its end-of-life for three years now (PHP End of Life Dates). It may be beneficial to increase the minimum PHP version supported by the plugin, but it currently functions well across all specified versions.
    • A rule is in place to check compatibility: <rule ref="PHPCompatibility"/><config name="testVersion" value="5.6-"/>

This setup has taken several hours to implement and refine. Feel free to make any adjustments to the composer.json if you have specific preferences or requirements.

Should you accept this pull request, further code modifications may be necessary, or you might opt to tweak the phpcs.xml, though that could be considered a workaround depending on the context :)

I look forward to your feedback and am happy to make any further adjustments as needed.

MaximeMichaud commented 2 months ago

https://github.com/OllieJones/fast-woo-order-lookup/pull/24/commits/7bbace3a5b869af3057bb1fc9167d4e6be965427

The recent commit was the result of using phpcbf to automatically fix coding standards issues. Here’s a summary of the corrections:

vendor/bin/phpcbf

--------------------------------------------------------------------------------
FILE                                                            FIXED  REMAINING
--------------------------------------------------------------------------------
...aud-fork\fast-woo-order-lookup\code\class-custom-fields.php  9      1
...meMichaud-fork\fast-woo-order-lookup\code\class-textdex.php  52     7
...ichaud-fork\fast-woo-order-lookup\fast-woo-order-lookup.php  77     23
--------------------------------------------------------------------------------
A TOTAL OF 138 ERRORS WERE FIXED IN 3 FILES
--------------------------------------------------------------------------------

Time: 5.66 secs; Memory: 24MB

I don’t believe this should break anything, but just in case, could you please take a look? It fixed a number of issues, but the remaining ones will need to be addressed manually.

MaximeMichaud commented 2 months ago

For what remains to be resolved (This comes from the CI of my repo, but the results would be the same when this pull request is accepted).

FILE: code/class-custom-fields.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 59 | WARNING | Silencing errors is strongly discouraged. Use proper error
    |         | checking instead. Found: @version_compare(
    |         | WOOCOMMERCE_VERSION,...
    |         | (WordPress.PHP.NoSilencedErrors.Discouraged)
--------------------------------------------------------------------------------

FILE: fast-woo-order-lookup.php
--------------------------------------------------------------------------------
FOUND [14](https://github.com/MaximeMichaud/fast-woo-order-lookup/actions/runs/11079069061/job/30787467616#step:5:15) ERRORS AND 9 WARNINGS AFFECTING 19 LINES
--------------------------------------------------------------------------------
  62 | WARNING | [ ] The method parameter $order is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
  72 | WARNING | [ ] The method parameter $props is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
  78 | WARNING | [ ] The method parameter $_meta_value is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
 109 | ERROR   | [ ] Method name "getInstance" in class FastWooOrderLookup is
     |         |     not in snake case format, try "get_instance"
     |         |     (WordPress.NamingConventions.ValidFunctionName.MethodNameInvalid)
 110 | WARNING | [x] Loose comparisons are not allowed. Expected: "==="; Found:
     |         |     "==" (Universal.Operators.StrictComparisons.LooseEqual)
 213 | WARNING | [ ] The method parameter $term is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
 213 | WARNING | [ ] The method parameter $search_fields is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
 247 | ERROR   | [ ] Variable "$newQuery" is not in valid snake_case format,
     |         |     try "$new_query"
     |         |     (WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase)
 248 | ERROR   | [ ] Variable "$newQuery" is not in valid snake_case format,
     |         |     try "$new_query"
     |         |     (WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase)
 249 | ERROR   | [ ] Variable "$newQuery" is not in valid snake_case format,
     |         |     try "$new_query"
     |         |     (WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase)
 249 | ERROR   | [ ] Variable "$newQuery" is not in valid snake_case format,
     |         |     try "$new_query"
     |         |     (WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase)
 250 | ERROR   | [ ] Empty ELSEIF statement detected
     |         |     (Generic.CodeAnalysis.EmptyStatement.DetectedElseif)
 250 | ERROR   | [ ] Variable "$newQuery" is not in valid snake_case format,
     |         |     try "$new_query"
     |         |     (WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase)
 253 | ERROR   | [ ] Variable "$newQuery" is not in valid snake_case format,
     |         |     try "$new_query"
     |         |     (WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase)
 253 | ERROR   | [ ] Variable "$newQuery" is not in valid snake_case format,
     |         |     try "$new_query"
     |         |     (WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase)
 256 | ERROR   | [ ] Variable "$newQuery" is not in valid snake_case format,
     |         |     try "$new_query"
     |         |     (WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase)
 264 | ERROR   | [ ] Missing parameter type
     |         |     (Squiz.Commenting.FunctionComment.MissingParamType)
 288 | ERROR   | [ ] Missing parameter type
     |         |     (Squiz.Commenting.FunctionComment.MissingParamType)
 289 | ERROR   | [ ] Missing parameter type
     |         |     (Squiz.Commenting.FunctionComment.MissingParamType)
 293 | WARNING | [ ] The method parameter $args is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
 348 | WARNING | [ ] Silencing errors is strongly discouraged. Use proper error
     |         |     checking instead. Found: @is_a( $order,...
     |         |     (WordPress.PHP.NoSilencedErrors.Discouraged)
 370 | WARNING | [ ] Not using strict comparison for in_array; supply true for
     |         |     $strict argument.
     |         |     (WordPress.PHP.StrictInArray.MissingTrueStrict)
 465 | ERROR   | [ ] A file should either contain function declarations or OO
     |         |     structure declarations, but not both. Found 3 function
     |         |     declaration(s) and 1 OO structure declaration(s). The
     |         |     first function declaration was found on line 465; the
     |         |     first OO declaration was found on line 45
     |         |     (Universal.Files.SeparateFunctionsFromOO.Mixed)
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: code/class-textdex.php
--------------------------------------------------------------------------------
FOUND 7 ERRORS AFFECTING 7 LINES
--------------------------------------------------------------------------------
 299 | ERROR | Empty line required before block comment
     |       | (Squiz.Commenting.BlockComment.NoEmptyLineBefore)
 3[17](https://github.com/MaximeMichaud/fast-woo-order-lookup/actions/runs/11079069061/job/30787467616#step:5:18) | ERROR | The use of count() inside a loop condition is not allowed;
     |       | assign the return value to a variable and use the variable in
     |       | the loop condition instead
     |       | (Squiz.PHP.DisallowSizeFunctionsInLoops.Found)
 375 | ERROR | Comment closer must be on a new line
     |       | (Squiz.Commenting.BlockComment.CloserSameLine)
 380 | ERROR | Comment closer must be on a new line
     |       | (Squiz.Commenting.BlockComment.CloserSameLine)
 467 | ERROR | Use placeholders and $wpdb->prepare(); found $query
     |       | (WordPress.DB.PreparedSQL.NotPrepared)
 501 | ERROR | Missing parameter type
     |       | (Squiz.Commenting.FunctionComment.MissingParamType)
 539 | ERROR | Missing parameter type
     |       | (Squiz.Commenting.FunctionComment.MissingParamType)
--------------------------------------------------------------------------------

Time: [28](https://github.com/MaximeMichaud/fast-woo-order-lookup/actions/runs/11079069061/job/30787467616#step:5:29)5ms; Memory: 14MB
MaximeMichaud commented 2 months ago

I have other changes, but I will wait before committing, and I will test the modified version of the plugin in production. So, this would remain:

$ vendor/bin/phpcs
WEE 3 / 3 (100%)

FILE: ...\Git\MaximeMichaud-fork\fast-woo-order-lookup\code\class-custom-fields.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 59 | WARNING | Silencing errors is strongly discouraged. Use proper error
    |         | checking instead. Found: @version_compare(
    |         | WOOCOMMERCE_VERSION,...
    |         | (WordPress.PHP.NoSilencedErrors.Discouraged)
--------------------------------------------------------------------------------

FILE: ...uments\Git\MaximeMichaud-fork\fast-woo-order-lookup\code\class-textdex.php
--------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------------
 469 | ERROR | Use placeholders and $wpdb->prepare(); found $query
     |       | (WordPress.DB.PreparedSQL.NotPrepared)
 503 | ERROR | Missing parameter type
     |       | (Squiz.Commenting.FunctionComment.MissingParamType)
 541 | ERROR | Missing parameter type
     |       | (Squiz.Commenting.FunctionComment.MissingParamType)
--------------------------------------------------------------------------------

FILE: ...nts\Git\MaximeMichaud-fork\fast-woo-order-lookup\fast-woo-order-lookup.php
--------------------------------------------------------------------------------
FOUND 5 ERRORS AND 9 WARNINGS AFFECTING 13 LINES
--------------------------------------------------------------------------------
  62 | WARNING | [ ] The method parameter $order is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
  72 | WARNING | [ ] The method parameter $props is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
  78 | WARNING | [ ] The method parameter $_meta_value is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
 110 | WARNING | [x] Loose comparisons are not allowed. Expected: "==="; Found:
     |         |     "==" (Universal.Operators.StrictComparisons.LooseEqual)
 213 | WARNING | [ ] The method parameter $term is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
 213 | WARNING | [ ] The method parameter $search_fields is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
 250 | ERROR   | [ ] Empty ELSEIF statement detected
     |         |     (Generic.CodeAnalysis.EmptyStatement.DetectedElseif)
 264 | ERROR   | [ ] Missing parameter type
     |         |     (Squiz.Commenting.FunctionComment.MissingParamType)
 288 | ERROR   | [ ] Missing parameter type
     |         |     (Squiz.Commenting.FunctionComment.MissingParamType)
 289 | ERROR   | [ ] Missing parameter type
     |         |     (Squiz.Commenting.FunctionComment.MissingParamType)
 293 | WARNING | [ ] The method parameter $args is never used
     |         |     (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
 348 | WARNING | [ ] Silencing errors is strongly discouraged. Use proper error
     |         |     checking instead. Found: @is_a( $order,...
     |         |     (WordPress.PHP.NoSilencedErrors.Discouraged)
 370 | WARNING | [ ] Not using strict comparison for in_array; supply true for
     |         |     $strict argument.
     |         |     (WordPress.PHP.StrictInArray.MissingTrueStrict)
 465 | ERROR   | [ ] A file should either contain function declarations or OO
     |         |     structure declarations, but not both. Found 3 function
     |         |     declaration(s) and 1 OO structure declaration(s). The
     |         |     first function declaration was found on line 465; the
     |         |     first OO declaration was found on line 45
     |         |     (Universal.Files.SeparateFunctionsFromOO.Mixed)
--------------------------------------------------------------------------------
MaximeMichaud commented 1 month ago

All the code that was modified previously has been in production since yesterday, and I checked my debug.log file, which has been active since the beginning of September.

It seems that there are no issues at all. However, I noticed a log entry from a few weeks ago. I haven't encountered it again since, but I'm mentioning it here just so you're aware of it:

[13-Sep-2024 23:05:54 UTC] PHP Fatal error:  Uncaught Error: Call to a member function get_id() on bool in /var/www/dev-prod/wp-content/plugins/woocommerce/sr>Stack trace:
#0 /var/www/dev-prod/wp-content/plugins/fast-woo-order-lookup/fast-woo-order-lookup.php(355): Automattic\WooCommerce\Internal\DataStores\CustomMetaDataStore->>#1 /var/www/dev-prod/wp-content/plugins/fast-woo-order-lookup/fast-woo-order-lookup.php(165): Fast_Woo_Order_Lookup\FastWooOrderLookup->update_meta_keys()
#2 /var/www/dev-prod/wp-includes/class-wp-hook.php(322): Fast_Woo_Order_Lookup\FastWooOrderLookup->update_textdex()
#3 /var/www/dev-prod/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters()
#4 /var/www/dev-prod/wp-includes/plugin.php(517): WP_Hook->do_action()
#5 /var/www/dev-prod/wp-includes/load.php(1280): do_action()
#6 [internal function]: shutdown_action_hook()
#7 {main}
  thrown in /var/www/dev-prod/wp-content/plugins/woocommerce/src/Internal/DataStores/CustomMetaDataStore.php on line 68

This error is not related to this pull request, but I'm mentioning it just in case :)