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

Clash with PHP 8.1 JsonSerializable Interface #144

Closed
MarkBaker opened this issue Jun 25, 2021 · 2 comments
Closed

Clash with PHP 8.1 JsonSerializable Interface #144

MarkBaker opened this issue Jun 25, 2021 · 2 comments

Comments

@MarkBaker
Copy link
Contributor

MarkBaker commented Jun 25, 2021

Running tests against the latest PHP 8.1 nightly causes fatal errors with a clash between MyClabs Enum and the new return typehints implementation in the interface for jsonSerializable in PHP 8.1

PHPUnit\Framework\Exception: PHP Fatal error:  During inheritance of JsonSerializable: Uncaught Declaration of MyCLabs\Enum\Enum::jsonSerialize() should be compatible with JsonSerializable::jsonSerialize(): mixed

and the stack trace (for reference)

/home/runner/work/PhpSpreadsheet/PhpSpreadsheet/vendor/myclabs/php-enum/src/Enum.php:301
/home/runner/work/PhpSpreadsheet/PhpSpreadsheet/vendor/myclabs/php-enum/src/Enum.php:22

As PHP 8.1 is now in alpha release stage, I thought I'd better raise an issue to ensure that you are aware of the problem.

This is against version 1.8.0 of the Enum library

@MarkBaker
Copy link
Contributor Author

MarkBaker commented Jun 28, 2021

The issue actually relates to this issue raised in PHP by jrfnl with the interface definition for jsonSerializable having a mixed return type and is highlighted here.

This can easily be resolved by specifying the return type in the implementation. However, the mixed pseudo-datatype was only introduced in PHP 8.0, and simply matching the signature would preclude usage of the library with versions of PHP < 8.0.

There are three possible solutions that I am aware of for this that would still allow usage with PHP < 8.0

  • Create your own interface definition for jsonSerializable Example. which doesn't require a mixed return datatype. While simpler to implement, this does feel a very dirty approach, and could create problems for anybody checking that the Enum class implements the native jsonSerializable, and would need to implement the json encoding as well; but this may be the only solution to the mixed datatypes that can be returned.
  • Use a Covariant return type, such as array or string - an example here - as suggested by Nikita Popov in the PHP issue linked above; although as the return value could be any datatype, this creates other problems.
  • Also suggested by Nikita Popov in the PHP issue linked above, use the #[ReturnTypeWillChange] annotation to suppress the Deprecation notice. Example here. For versions of PHP prior to 8.0 when annotations were introduced, this will simply be treated as a comment inside the docblock, so will not trigger any additional notices

A combination of the first and third solutions might be a good approach, eliminating the deprecation notice, with the annotation highlighting that there is a difference between the implementation signature and the interface signature.

@MarkBaker MarkBaker changed the title Clash with PHP 8.1 Enum Implementation Clash with PHP 8.1 JsonSerializable Interface Jun 28, 2021
@mnapoli
Copy link
Member

mnapoli commented Jun 29, 2021

Thank you @MarkBaker, this should be fixed in the latest release.

@mnapoli mnapoli closed this as completed Jun 29, 2021
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

No branches or pull requests

2 participants