Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PHP 8.4 compatibility #2647

Open
mikkamp opened this issue Oct 18, 2024 · 4 comments
Open

PHP 8.4 compatibility #2647

mikkamp opened this issue Oct 18, 2024 · 4 comments
Labels
type: technical debt This issue/PR represents/solves the technical debt of the project.

Comments

@mikkamp
Copy link
Contributor

mikkamp commented Oct 18, 2024 •
edited
Loading

Describe the issue:

With the upcoming release of PHP 8.4 there is another set of changes that are marked as deprecated. In order to continue preventing deprecation notices we will need to resolve these for any sites that switch to PHP 8.4

The following deprecations in PHP 8.4 are the ones we should address:

Implicitly nullable parameter declarations deprecated

There are several occurrences of this in the code where a non nullable type is declared and the default is set to null. There might be a better regex, but I found several occurrences by searching for function.*= null.

Some examples include:

Deprecate proprietary CSV escaping mechanism

We use this in the following code: https://github.com/woocommerce/google-listings-and-ads/blob/2.8.6/src/DB/Table/BudgetRecommendationTable.php#L105

We should map to a function where we can pass the parameters to str_getcsv:

			$csv = array_map(
				function ( $row ) {
					str_getcsv( $row, ',', '"', '\\' );
				},
				file( $path )
			);

Expected behavior:

No deprecation messages when running the extension with PHP 8.4. Also to run the unit tests with PHP 8.4
@mikkamp mikkamp added the type: technical debt This issue/PR represents/solves the technical debt of the project. label Oct 18, 2024
@mikkamp
Copy link
Contributor Author

mikkamp commented Nov 14, 2024

Additional problems to resolve in included packages:

symfony/validator 5.4.30 warning:

Deprecated: Symfony\Component\Validator\Context\ExecutionContextFactory::__construct(): Implicitly marking parameter $translationDomain as nullable is deprecated, the explicit nullable type must be used instead in google-listings-and-ads/vendor/symfony/validator/Context/ExecutionContextFactory.php on line 29

This is resolved in 5.4.35 although we can consider updating to a newer version 5.4.47 or if there are no additional blockers one of the 6.4.x or 7.1.x versions.

$ composer update symfony/validator --with-dependencies

  - Upgrading symfony/polyfill-ctype (v1.29.0 => v1.31.0)
  - Upgrading symfony/polyfill-php73 (v1.28.0 => v1.31.0)
  - Upgrading symfony/polyfill-php80 (v1.29.0 => v1.31.0)
  - Upgrading symfony/polyfill-php81 (v1.29.0 => v1.31.0)
  - Upgrading symfony/translation-contracts (v2.5.2 => v2.5.3)
  - Upgrading symfony/validator (v5.4.30 => v5.4.47)

When updating the symfony/validator package the test_invalid_image_names no longer fails because additional unicode characters are now allowed in the UrlValidator. We should revisit this test and confirm if the unicode characters are still a problem for WP/some hosts. If they are then we might need to customize the ImageUrlValidator to once again restrict the characters.

google/apiclient 2.16.0 warnings

Deprecated: Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Google\Client::authorize(): Implicitly marking parameter $http as nullable is deprecated, the explicit nullable type must be used instead in google-listings-and-ads/vendor/google/apiclient/src/Client.php on line 451

Deprecated: Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Google\Client::fetchAccessTokenWithAssertion(): Implicitly marking parameter $authHttp as nullable is deprecated, the explicit nullable type must be used instead in google-listings-and-ads/vendor/google/apiclient/src/Client.php on line 318

Implicitly marking parameter $previous as nullable is deprecated, the explicit nullable type must be used instead
google-listings-and-ads/vendor/google/apiclient/src/Service/Exception.php:39

Not fixed yet, reported issue googleapis/google-api-php-client#2636

guzzle/guzzlehttp 7.8.1 warnings

PHP Deprecated:  Automattic\WooCommerce\GoogleListingsAndAds\Vendor\GuzzleHttp\ClientInterface::getConfig(): Implicitly marking parameter $option as nullable is deprecated, the explicit nullable type must be used instead in google-listings-and-ads/vendor/guzzlehttp/guzzle/src/ClientInterface.php on line 83

PHP 8.4 support was added in 7.8.2

$ composer update guzzlehttp/guzzle --with-dependencies
   
  - Upgrading guzzlehttp/guzzle (7.8.1 => 7.9.2)
  - Upgrading guzzlehttp/promises (2.0.2 => 2.0.4)
  - Upgrading guzzlehttp/psr7 (2.6.2 => 2.7.0)
  - Upgrading psr/http-factory (1.0.2 => 1.1.0)

google/auth 1.37.1 warnings

Deprecated: Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Google\Auth\FetchAuthTokenInterface::fetchAuthToken(): Implicitly marking parameter $httpHandler as nullable is deprecated, the explicit nullable type must be used instead in google-listings-and-ads/vendor/google/auth/src/FetchAuthTokenInterface.php on line 31

Deprecated: Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Google\Auth\Credentials\InsecureCredentials::fetchAuthToken(): Implicitly marking parameter $httpHandler as nullable is deprecated, the explicit nullable type must be used instead in google-listings-and-ads/vendor/google/auth/src/Credentials/InsecureCredentials.php on line 42

Fixed in 1.43.0
Should be updated with the googleads/google-ads-php package. See pcTzPl-2zd-p2

league/container 3.4.1 deprecations

Deprecated: Automattic\WooCommerce\GoogleListingsAndAds\Vendor\League\Container\Inflector\Inflector::__construct(): Implicitly marking parameter $callback as nullable is deprecated, the explicit nullable type must be used instead in google-listings-and-ads/vendor/league/container/src/Inflector/Inflector.php on line 40

PHP 8.4 compatibility is fixed in 4.2.3
However this means we have to update to psr/container 2.0 which brings some breaking changes. We'll need to make sure we prefix that composer package as it's also used by WooCommerce so we can't update.

Note that WC 9.5 is moving to an independent container, but the composer dependency hasn't been removed yet.

automattic/jetpack-connection

Deprecated: Automattic\Jetpack\Connection\Manager::setup_xmlrpc_handlers(): Implicitly marking parameter $xmlrpc_server as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/vendor/automattic/jetpack-connection/src/class-manager.php on line 171

Resolved here: Automattic/jetpack#40147
But not released as a new package yet.

WooCommerce 9.4.1 deprecations is being covered here: woocommerce/woocommerce#52081

deprecation messages 
Deprecated: Automattic\WooCommerce\Internal\Utilities\URL::get_path(): Implicitly marking parameter $path_override as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/Internal/Utilities/URL.php on line 359

Deprecated: Automattic\WooCommerce\Admin\PluginsHelper::activate_plugins(): Implicitly marking parameter $logger as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/Admin/PluginsHelper.php on line 423

Deprecated: Automattic\WooCommerce\Admin\PluginsHelper::install_plugins(): Implicitly marking parameter $logger as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/Admin/PluginsHelper.php on line 220

Deprecated: Automattic\WooCommerce\Internal\Features\FeaturesController::verify_did_woocommerce_init(): Implicitly marking parameter $function_name as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/Internal/Features/FeaturesController.php on line 549

Deprecated: Automattic\WooCommerce\Internal\BatchProcessing\BatchProcessingController::update_processor_state(): Implicitly marking parameter $last_error as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/Internal/BatchProcessing/BatchProcessingController.php on line 260

Deprecated: Automattic\WooCommerce\Caching\ObjectCache::get(): Implicitly marking parameter $get_from_datastore_callback as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/Caching/ObjectCache.php on line 229

Deprecated: Automattic\WooCommerce\Database\Migrations\TableMigrator::db_get_results(): Implicitly marking parameter $query as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/Database/Migrations/TableMigrator.php on line 83

Deprecated: Automattic\WooCommerce\Internal\DependencyManagement\ServiceProviders\AbstractInterfaceServiceProvider::add_with_implements_tags(): Implicitly marking parameter $shared as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/Internal/DependencyManagement/ServiceProviders/AbstractInterfaceServiceProvider.php on line 59

Deprecated: Automattic\WooCommerce\Internal\DependencyManagement\AbstractServiceProvider::add(): Implicitly marking parameter $shared as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/Internal/DependencyManagement/AbstractServiceProvider.php on line 153

Deprecated: Automattic\WooCommerce\Internal\DependencyManagement\ExtendedContainer::add(): Implicitly marking parameter $shared as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/Internal/DependencyManagement/ExtendedContainer.php on line 68

Deprecated: Automattic\WooCommerce\StoreApi\Schemas\V1\CheckoutSchema::get_checkout_response(): Implicitly marking parameter $payment_result as nullable is deprecated, the explicit nullable type must be used instead in woocommerce/src/StoreApi/Schemas/V1/CheckoutSchema.php on line 214

Deprecated: Automattic\WooCommerce\Admin\Features\PaymentGatewaySuggestions\Init::get_suggestions(): Implicitly marking parameter $specs as nullable is deprecated, the explicit nullable type must be used instead in /tmp/woocommerce-9.4.1/plugins/woocommerce/src/Admin/Features/PaymentGatewaySuggestions/Init.php on line 36

Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\MultichannelMarketing\GLAChannelTest::test_get_campaigns_returns_marketing_campaigns
Automattic\WooCommerce\Admin\Marketing\MarketingCampaign::__construct(): Implicitly marking parameter $cost as nullable is deprecated, the explicit nullable type must be used instead

woocommerce/src/Admin/Marketing/MarketingCampaign.php:68

Action scheduler deprecations within WooCommerce have been resolved here: woocommerce/action-scheduler#1205
It will be some time before these fixes make their way into WooCommerce core.

phpunit/phpunit 9.6.13 deprecations

Deprecated: PHPUnit\Framework\TestSuite::run(): Implicitly marking parameter $result as nullable is deprecated, the explicit nullable type must be used instead in google-listings-and-ads/vendor/phpunit/phpunit/src/Framework/TestSuite.php on line 602

Fixed in 9.6.18

$ composer update phpunit/phpunit --with-dependencies
   
  - Upgrading myclabs/deep-copy (1.11.1 => 1.12.1)
  - Upgrading nikic/php-parser (v4.17.1 => v5.3.1)
  - Upgrading phar-io/manifest (2.0.3 => 2.0.4)
  - Upgrading phpunit/php-code-coverage (9.2.29 => 9.2.32)
  - Upgrading phpunit/phpunit (9.6.13 => 9.6.21)
  - Upgrading sebastian/cli-parser (1.0.1 => 1.0.2)
  - Upgrading sebastian/complexity (2.0.2 => 2.0.3)
  - Upgrading sebastian/diff (4.0.5 => 4.0.6)
  - Upgrading sebastian/exporter (4.0.5 => 4.0.6)
  - Upgrading sebastian/global-state (5.0.6 => 5.0.7)
  - Upgrading sebastian/lines-of-code (1.0.3 => 1.0.4)
  - Upgrading sebastian/resource-operations (3.0.3 => 3.0.4)
  - Upgrading theseer/tokenizer (1.2.1 => 1.2.3)

@mikkamp
Copy link
Contributor Author

mikkamp commented Nov 14, 2024

After updating Guzzle we also need to resolve the following error:

For "Class "GuzzleHttp\Psr7\Utils" not found" at
google-listings-and-ads/vendor/guzzlehttp/guzzle/src/Exception/RequestException.php:92

The following string is not prefixed:

        $uri = \GuzzleHttp\Psr7\Utils::redactUserInfo($request->getUri());

We'll need to add it as a direct replacement here: https://github.com/woocommerce/google-listings-and-ads/blob/2.8.7/bin/prefix-vendor-namespace.php#L119-L132

@mikkamp
Copy link
Contributor Author

mikkamp commented Nov 14, 2024 •
edited
Loading

For the package google/google-ads we use a legacy package which supports PHP 7.4, latest can be found here: https://packagist.org/packages/googleads/google-ads-php#dev-legacy-v25.0.0

This works fine for the package, however it's dependent on several other packages which means we are stuck on older versions, because they updated minimum PHP versions:

The google/auth package fixes PHP 8.4 compatibility but only in version 1.43 which we can't update to. google/gax does not have fixes for PHP 8.4 yet, but when they do get fixed we also can't update to that version as it has a minimum requirement of PHP 8.0

List of issues with `google/gax` that have not been fixed 
Implicitly marking parameter is deprecated, the explicit nullable type must be used instead

google-listings-and-ads/vendor/google/gax/src/GapicClientTrait.php:711
google-listings-and-ads/vendor/google/gax/src/GapicClientTrait.php:767
google-listings-and-ads/vendor/google/gax/src/GapicClientTrait.php:792
google-listings-and-ads/vendor/google/gax/src/ResourceHelperTrait.php:87
google-listings-and-ads/vendor/google/gax/src/ApiException.php:62
google-listings-and-ads/vendor/google/gax/src/ApiException.php:163
google-listings-and-ads/vendor/google/gax/src/Call.php:62
google-listings-and-ads/vendor/google/gax/src/PathTemplate.php:57
google-listings-and-ads/vendor/google/gax/src/ResourceTemplate/RelativeResourceTemplate.php:290
google-listings-and-ads/vendor/google/gax/src/ResourceTemplate/Parser.php:51
google-listings-and-ads/vendor/google/gax/src/ResourceTemplate/Segment.php:66

@mikkamp
Copy link
Contributor Author

mikkamp commented Nov 25, 2024 •
edited
Loading

For the package google/apiclient the PHP 8.4 compatibility was included in version 2.18.1

Because PHP 7.4 support was dropped we are stuck on version 2.16.0 which doesn't include PHP 8.4 compatibility. Request to backport.

@eason9487 eason9487 mentioned this issue Nov 25, 2024
Closed
@mikkamp mikkamp mentioned this issue Dec 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: technical debt This issue/PR represents/solves the technical debt of the project.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@mikkamp