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.

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

PHP 8 fix for Picsum imageUrl method signature due to change to parent method in Faker #13

Merged
merged 6 commits into from
Oct 12, 2022
Merged

Conversation

dansmart-box
Copy link
Contributor

@dansmart-box dansmart-box commented Oct 11, 2022

There is an issue we’re facing in upgrading to PHP 8.1 with WP CLI Fixtures.

This occurs on PHP 8.1 (and now confirmed on PHP 8.0) when running wp fixtures load. The error we’re getting is:

Fatal error: Declaration of Hellonico\Fixtures\Provider\Picsum::imageUrl($width = 640, $height = 480, $filters = [],
 $format = 'jpg', $unused = false, $unused_ = false) must be compatible with Faker\Provider\Image::imageUrl($width = 640, 
$height = 480, $category = null, $randomize = true, $word = null, $gray = false, $format = 'png') 
in /var/www/.wp-cli/packages/vendor/hellonico/wp-cli-fixtures/src/Provider/Picsum.php on line 19

Taking a look at Picsum.php, it has a different number of parameters to the parent method in Image.php

Changing Picsum.php to add the additional parameter gets it working again.

It seems the interface of the Faker\Provider\Image::imageUrl() function was changed in version 1.20.0 (https://github.com/FakerPHP/Faker/blob/main/src/Faker/Provider/Image.php) - and at the very bottom of the Faker PR someone comments that it's broken Backwards Compatiblity: (FakerPHP/Faker#473).
We're finding that it's only PHP 8.x that is reporting this compatibility error (due to increased PHP 8 type checking - see the section on Contravariance in (https://php.watch/versions/8.0/lsp-errors)).

The Faker reference is in wp-cli-fixtures' composer.json: https://github.com/nlemoine/wp-cli-fixtures/blob/main/composer.json:

"require": {
  "php": "^7.3 || ^8.0",
  "fakerphp/faker": "^1.13",
  "nelmio/alice": "^3.8",
  "wp-cli/wp-cli": "^2.4"
 },

This PR updates the Picsum::imageUrl() method signature to match the parent from Faker, and also updates the dependency to the latest version 1.20.0.

Tested on:

  • PHP 7.4
  • PHP 8.0
  • PHP 8.1

@jenkoian
Copy link
Contributor

I guess they're accepting PRs now - in which case maybe worth trying to get Faker to consider FakerPHP/Faker#323 or similar. IIRC placeholder.com is awfully slow.

@dansmart-box
Copy link
Contributor Author

Note: I've had to modify the picsum() function also which loads from Picsum, as it now returns a 403 error unless you add a dummy user agent (see smknstd/fakerphp-picsum-images@e438bbf for another example). This should allow it to pass the tests.

@nlemoine
Copy link
Owner

Thanks @dansmart-box for updating this! Everything looks ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants