Apply Rector suggestions for PHP 8.1 #43233

asumaran · 2024-01-03T01:45:22Z

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 D128575-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 wanted to provide some basic testing steps in case we want to test some file changes:

plugins/woocommerce/src/Admin/API/OnboardingTasks.php
- Go to wp-admin/admin.php?page=wc-admin&task=products
- Click Grouped product.
- You should be able to see guided tour.
plugins/woocommerce/src/Admin/API/Plugins.php
- No need to test this one. It just initialized a variable with a default value.
- Also, I haven’t found a way to test this one. Seems related to Square.
plugins/woocommerce/src/Admin/API/Reports/Categories/DataStore.php
- This one doesn't need testing. The return type just got updated in the docblock. There's no code change.
plugins/woocommerce/src/Admin/API/Reports/Coupons/Stats/Segmenter.php
- Didn’t find a way to test this but testing Coupon stats should be enough.
- Go to Analytics > Coupons. Charts should be visible.
plugins/woocommerce/src/Admin/API/Reports/Orders/Stats/Segmenter.php
- Didn’t find a way to test this but testing Orders stats should be enough.
- Go to Analytics > Orders. Charts should be visible.
plugins/woocommerce/src/Admin/API/Reports/Products/Stats/Segmenter.php
- Didn’t find a way to test this but testing Products stats should be enough.
- Go to Analytics > Products. Charts should be visible.
plugins/woocommerce/src/Admin/API/Reports/Segmenter.php
- Didn’t find a way to test this but checking any Analytics page should be enough. the code doesn’t do anything harmful. just adds a type check before count
plugins/woocommerce/src/Admin/API/Reports/Taxes/DataStore.php
plugins/woocommerce/src/Admin/API/Reports/Taxes/Stats/DataStore.php
- Create a couple tax rates, apply a different country code for each one.
- Create two orders one for each tax rate.
- Go to Analytics > Taxes. It should show orders correctly.
plugins/woocommerce/src/Admin/API/Reports/TimeInterval.php
- Didn’t find a way to test this. Code only adds a type check. In any case unit tests should cover it.
plugins/woocommerce/src/Admin/API/Reports/Variations/Stats/Segmenter.php
- Doesn't need testing. The return type just got updated in the docblock and just initializes a variable with a default value.
plugins/woocommerce/src/Admin/API/Themes.php
- No need to test this one as it just fixes how the variable is initialized.
- In any case visiting any WooCommerce page should confirm it works since the code is run on every page to register the wc-admin/themes REST route.
plugins/woocommerce/src/Admin/PageController.php
- Visit any WooCommerce page. The page title should be correct.
plugins/woocommerce/src/Admin/RemoteInboxNotifications/PluginsActivatedRuleProcessor.php
- No need to test this as it only added a type check before count
plugins/woocommerce/src/Admin/RemoteInboxNotifications/RemoteInboxNotificationsEngine.php
- No need to test this as it only added a type check before count
plugins/woocommerce/src/Admin/RemoteInboxNotifications/SpecRunner.php
- No need to test this as it only added a type check before count

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 · 2024-01-03T02:08:55Z

Test Results Summary

Commit SHA: 15641d1

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 37s
E2E Tests	316	0	0	28	0	344	7m 26s

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 · 2024-01-17T21:47:09Z

Hi @coreymckrill,

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

coreymckrill · 2024-01-19T00:15:40Z

plugins/woocommerce/src/Admin/API/Themes.php

@@ -192,7 +192,7 @@ public function get_item_schema() {
 	 * @return array
 	 */
 	public function get_collection_params() {
-		$params['context']   = $this->get_context_param( array( 'default' => 'view' ) );
+		$params = [ 'context' => $this->get_context_param( array( 'default' => 'view' ) ) ];


Suggested change

$params = [ 'context' => $this->get_context_param( array( 'default' => 'view' ) ) ];

$params = array( 'context' => $this->get_context_param( array( 'default' => 'view' ) ) );

In WP code style rules, the longform array syntax is preferred to the shorter []. In this case, I think it also makes it easier to understand.

@coreymckrill I've fixed all PHP Lint issues 2355e20

coreymckrill

👍 Looks good! Thanks @asumaran

* PHP8 fixes for src/Admin * Add changelog entry * Fix PHP Lint issues

louwie17 · 2024-03-26T11:44:14Z

plugins/woocommerce/src/Admin/API/Reports/Products/Stats/Segmenter.php

@@ -165,7 +165,7 @@ protected function get_segments( $type, $query_params, $table_name ) {

 			$segments = $this->get_product_related_segments( $type, $segmenting_selections, $segmenting_from, $segmenting_where, $segmenting_groupby, $segmenting_dimension_name, $table_name, $query_params, $unique_orders_table );
 		} elseif ( 'variation' === $this->query_args['segmentby'] ) {
-			if ( ! isset( $this->query_args['product_includes'] ) || count( $this->query_args['product_includes'] ) !== 1 ) {
+			if ( ! isset( $this->query_args['product_includes'] ) || ! is_array( count( $this->query_args['product_includes'] ) ) || count( $this->query_args['product_includes'] ) !== 1 ) {


@asumaran it appears this change actually broke our Analytics view when comparing variation products within Analytics > Products and then selecting a variable product.
I assume this wasn't suppose to be wrapped in a count method?

I created a PR with a fix: #45939

@louwie17 I'll give a look at the PR.

asumaran self-assigned this Jan 3, 2024

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Jan 3, 2024

asumaran force-pushed the as-diff-D128575 branch from d11830b to aa75d99 Compare January 3, 2024 19:15

prettyboymp reviewed Jan 12, 2024

View reviewed changes

asumaran marked this pull request as ready for review January 12, 2024 22:54

asumaran force-pushed the as-diff-D128575 branch from aa75d99 to 060d364 Compare January 17, 2024 21:45

asumaran requested review from a team and coreymckrill and removed request for a team January 17, 2024 21:46

coreymckrill reviewed Jan 19, 2024

View reviewed changes

asumaran force-pushed the as-diff-D128575 branch from 060d364 to 2355e20 Compare January 19, 2024 00:27

asumaran marked this pull request as draft January 23, 2024 18:26

asumaran added 3 commits February 21, 2024 17:22

PHP8 fixes for src/Admin

2e3d808

Add changelog entry

a6b3d38

Fix PHP Lint issues

15641d1

asumaran force-pushed the as-diff-D128575 branch from 2355e20 to 15641d1 Compare February 21, 2024 22:23

asumaran marked this pull request as ready for review February 21, 2024 22:28

asumaran requested a review from coreymckrill February 21, 2024 22:28

coreymckrill approved these changes Feb 22, 2024

View reviewed changes

coreymckrill merged commit f4bccaa into trunk Feb 22, 2024
38 checks passed

coreymckrill deleted the as-diff-D128575 branch February 22, 2024 23:37

github-actions bot added this to the 8.7.0 milestone Feb 22, 2024

github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Feb 22, 2024

nigeljamesstevenson added needs: internal testing Indicates if the PR requires further testing conducted by Solaris status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Feb 23, 2024

Konamiman pushed a commit that referenced this pull request Mar 13, 2024

Apply Rector suggestions for PHP 8.1 (#43233)

b5a80a3

* PHP8 fixes for src/Admin * Add changelog entry * Fix PHP Lint issues

louwie17 reviewed Mar 26, 2024

View reviewed changes

louwie17 mentioned this pull request Mar 26, 2024

Remove accidental count call within an is_array check #45939

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply Rector suggestions for PHP 8.1 #43233

Apply Rector suggestions for PHP 8.1 #43233

asumaran commented Jan 3, 2024 •

edited

github-actions bot commented Jan 3, 2024 •

edited

prettyboymp left a comment

github-actions bot commented Jan 17, 2024

coreymckrill Jan 19, 2024

asumaran Jan 19, 2024

coreymckrill left a comment

louwie17 Mar 26, 2024

louwie17 Mar 26, 2024

asumaran Mar 26, 2024

	$params = [ 'context' => $this->get_context_param( array( 'default' => 'view' ) ) ];
	$params = array( 'context' => $this->get_context_param( array( 'default' => 'view' ) ) );

Apply Rector suggestions for PHP 8.1 #43233

Apply Rector suggestions for PHP 8.1 #43233

Conversation

asumaran commented Jan 3, 2024 • edited

Submission Review Guidelines:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Changelog entry

Significance

Type

Message

Comment

github-actions bot commented Jan 3, 2024 • edited

Test Results Summary

prettyboymp left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 17, 2024

coreymckrill Jan 19, 2024

Choose a reason for hiding this comment

asumaran Jan 19, 2024

Choose a reason for hiding this comment

coreymckrill left a comment

Choose a reason for hiding this comment

louwie17 Mar 26, 2024

Choose a reason for hiding this comment

louwie17 Mar 26, 2024

Choose a reason for hiding this comment

asumaran Mar 26, 2024

Choose a reason for hiding this comment

asumaran commented Jan 3, 2024 •

edited

github-actions bot commented Jan 3, 2024 •

edited