Apply Rector suggestions for PHP 8.1 #41482
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asumaran
added
the
plugin: woocommerce
|
Hi @jorgeatorres, Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
Test Results SummaryCommit SHA: ad4e25c
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
asumaran
force-pushed
the
as-update-to-php-81-2
branch
from
e1ef18e
to
89e5760
Compare
|
@prettyboymp I've removed a few false positives. Those are explained on every commit just in case you want to double check. |
prettyboymp
requested changes
Jan 12, 2024
plugins/woocommerce/includes/emails/class-wc-email-new-order.php
Outdated
Show resolved
Hide resolved
|
Changes look good. May need to change the changelog entry though. |
asumaran
force-pushed
the
as-update-to-php-81-2
branch
2 times, most recently
from
73ef850
to
64ed233
Compare
asumaran
requested review from
a team and
jorgeatorres
and removed request for
a team
jorgeatorres
approved these changes
Jan 23, 2024
There was a problem hiding this comment.
@asumaran: This looks good to me. Left some feedback about using static vs self but I'm pre-approving for now.
asumaran
force-pushed
the
as-update-to-php-81-2
branch
from
93bd4a0
to
3419c5b
Compare
|
@jorgeatorres can we merge this one? |
$missing_tables will always be an array
Rector issue: AddDefaultValueForUndefinedVariableRector. This time the early return is valid so there’s no need to move the $process_limit assignment to the top.
$orders is used as an array lines below. Add `is_array` just in case the “paginate” param is used which would change $orders to an object.
Simplifies variable check given that they are used as an array lines below.
In the previous loop the $data variable is being built as an array of arrays. Then $value in this loop will allways be array.
get_children’s method. of a WC_Product instance will always return an array.
Ask if varialbe is array since it only can be array or false.
WC_Product->get_visible_children() will always return an array.
WC_Order->get_items() will always return an array.
The get_matching_rates private method will always return an array. Updated the @return declaration to be an array instead of boolean.
$this->get_children() will always return an array.
asumaran
force-pushed
the
as-update-to-php-81-2
branch
from
3419c5b
to
ad4e25c
Compare
It looks good to me, but I see that @prettyboymp hasn't approved yet. |
prettyboymp
approved these changes
Jan 31, 2024
|
@jorgeatorres it's approved now and all tests passed. should be good to merge 👍 |
github-actions
bot
added
the
needs: analysis
nigeljamesstevenson
added
needs: external testing
thealexandrelara
pushed a commit
that referenced
this pull request
Feb 1, 2024
* Apply Rector suggestions * Add changefile(s) from automation for the following project(s): woocommerce * Restore false positive $missing_tables will always be an array * Restore false positive. Rector issue: AddDefaultValueForUndefinedVariableRector. This time the early return is valid so there’s no need to move the $process_limit assignment to the top. * Ask if $orders is an array $orders is used as an array lines below. Add `is_array` just in case the “paginate” param is used which would change $orders to an object. * Check if variable is array Simplifies variable check given that they are used as an array lines below. * Restore code as taxes is always an array * Restore false positive In the previous loop the $data variable is being built as an array of arrays. Then $value in this loop will allways be array. * Restore false positive get_children’s method. of a WC_Product instance will always return an array. * Simplify variable check Ask if varialbe is array since it only can be array or false. * Restore false positive WC_Blocks_Utils::get_blocks_from_page() will always return an array. * Restore false positive There’s a check asking if $existing_meta_data[ $meta_data->key ] is array. * Restore false positive $child_ids will always be an array. * Restore false positives WC_Product->get_visible_children() will always return an array. * Restore false positives WC_Product->get_children() will always return an array. * Restore false positive WC_Product->get_visible_children() will always return an array. * Restore false positive WC_Order->get_items() will always return an array. * Restore false positive The get_matching_rates private method will always return an array. Updated the @return declaration to be an array instead of boolean. * Remove unnecessary array type casting. * Restore false positive $this->get_children() will always return an array. * Fix lint issues * Remove unnecessary default assignment * Remove unnecessary is_array check * Remove unnecessary is_array check * Remove unnecessary is_array check * Change default value from null to false * Remove unnecessary is_array check * Update changelog entry * Update changelog entry * Update changelog entry * Add changefile(s) from automation for the following project(s): woocommerce * Update changelog entry * Use self for consistency --------- Co-authored-by: github-actions <github-actions@github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this Pull Request:
Applied suggestions based on Rector's static analysis for PHP 8.1.
This is a continuation of #41253 and #40801
Ref: D128845-code
How to test the changes in this Pull Request:
The changes on this PR are based on changes currently made in WPCOM, As there are no logic changes, only compatibility with PHP 8.1, it was deemed unnecessary to include testing instructions. To put it simply, the current tests should be able to complete without any problems, and that should be sufficient.
Anyway, here's some testing instructions:
plugins/woocommerce/includes/class-wc-product-factory.php
plugins/woocommerce/includes/class-wc-query.php
http://site.tld/shop/products should display correct prices without issues.plugins/woocommerce/includes/class-wc-tracker.php
pnpm test:php:env --filter=WC_Tracker_Test) takes care of it.plugins/woocommerce/includes/emails/class-wc-email-new-order.php
plugins/woocommerce/includes/gateways/cod/class-wc-gateway-cod.php
plugins/woocommerce/includes/gateways/paypal/class-wc-gateway-paypal.php
Changelog entry
Significance
Type
Message
Apply Rector suggestions for PHP 8.1
Comment