Apply Rector suggestions for PHP 8.1

[asumaran]

Submission Review Guidelines:

• I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have reviewed my code for security best practices.
• Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

Subset of WPCOM changes for PHP 8.1 compatibility based on the D129164-code diff.

Ref: p7H4VZ-4x1-p2

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.

In any case, I'm including testing instructions for these changes.

plugins/woocommerce/includes/wc-notice-functions.php
plugins/woocommerce/includes/wc-cart-functions.php

• Verify notice is still shown after adding a product to the cart and the option “Redirect to the cart page after successful addition” is checked.

plugins/woocommerce/includes/wc-cart-functions.php

• Verify adding product to the cart while rates and shipping are enabled work as expected.

plugins/woocommerce/includes/wc-order-functions.php

• Verify refunds works as usual

plugins/woocommerce/includes/wc-product-functions.php

• Verify product prices are correct including tax
• Verify product prices are correct excluding tax

plugins/woocommerce/includes/wc-template-functions.php

• unfortunately we can't test this one as the wc_display_item_downloads isn’t used anywhere in WooCommerce.
• However, I made sure wc_display_item_downloads is working as expected by calling the method manually. It displays the download links as expected

  

Changelog entry

• Automatically create a changelog entry from the details below.

Significance

• Patch
• Minor
• Major

Type

• Fix - Fixes an existing bug
• Add - Adds functionality
• Update - Update existing functionality
• Dev - Development related task
• Tweak - A minor adjustment to the codebase
• Performance - Address performance issues
• Enhancement - Improvement to existing functionality

Message

Comment

[github-actions]

Test Results Summary

Commit SHA: ec6ba75

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 38s
E2E Tests	287	0	0	6	0	293	6m 58s

---

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.

[prettyboymp]

Changes look good.

[github-actions]

Hi @prettyboymp, @Konamiman,

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:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[Konamiman]

Hi, changes look good on my side too but would it be possible to have testing instructions please? Or do these changes como from a pull request (or similar) with its own testing instructions?

[asumaran]

@Konamiman, I've updated the testing instructions section in the PR description. I don't think adding testing instructions is necessary as the PR does not introduce or modify existing functionality but refactors for PHP 8.1. Unless @prettyboymp has something to add?

[Konamiman]

I don't think adding testing instructions is necessary as the PR does not introduce or modify existing functionality but refactors for PHP 8.1.

Just a general note: testing instructions aren't only for new or modified functionality, they are also valuable to ensure that nothing breaks after a refactor.

The changes look good to me so I'll approve the PR, but since I haven't really tested anything beyond the visual inspection I'll add the "needs analysis" label.

[alopezari]

Thanks for the heads up @Konamiman!

@asumaran even if there aren't any logical changes, regression issues might raise. Considering WooCommerce is a complex application with tons of flows, would you suggest any specific existing flows we should test more carefully to cover potential side effects caused by these changes?

[asumaran]

@alopezari, this PR is part of a series of PRs I've created to push upstream the changes already made in WPCOM. As these shouldn't change the code logic, I didn't consider adding testing steps.

Given the changes cover practically the entire WooCommerce extension, adding testing steps for each PR will take a while since I need to get familiar with the context of each change beyond the refactor.

In the meantime, I'll put the open PRs on hold until testing steps are added and focus on adding test instructions to the merged PRs.

cc @prettyboymp In case wants something to add.

[prettyboymp]

Hi @alopezari, as @asumaran mentioned, these changes are purely to deal with errors that are now escalated to a new level in PHP8. This includes logic paths that potentially use undefined variables, or attempt to iterate non-iterable or non-countable values.
These are changes that we're having to re-apply each time we update WooCommerce in production to be sure the the transition to PHP8 on our servers won't suddenly cause fatal errors. Each in itself is relatively simple data check.

[alopezari]

Thanks @asumaran @prettyboymp!

We plan to cover these PRs as part of the critical flows testing (WooCommerce & WooCommerce Blocks.

I understand these are minor changes that don't alter WooCommerce's functionality, I was just a bit worried about the lack of testing instructions since every single line of code could break the entire app. We test our critical flows for every release, so theoretically that would cover these changes. However, we would appreciate if you could share some flows affected by these changes so that we can pay special attention to them (also known as targeted or selective regression testing). There is no need to define an exhaustive test plan or process, just some simple indications would be great. Otherwise we will just test our critical flows as usual.

Thanks!

[asumaran]

@alopezari I've added testing instructions to the PR's description. @Konamiman Please feel free to merge.

[alopezari]

Thank you very much @asumaran! Much appreciated 🙌

[asumaran]

@Konamiman Plese merge the PR. I've added testing instructions.