Make implementations of built-in interfaces PHP 8.1 friendly

[om4csaba]

Problem

Unfortunately, PHP 8.1 throws Fatal Error if the built-in interface implementation is incompatible with the new, type enhanced interfaces.

Example error:

Fatal error: During inheritance of ArrayAccess: Uncaught Return type of cebe\openapi\spec\Responses::offsetExists($offset) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

Example differences:

-public function offsetExists($offset);
+public function offsetExists(mixed $offset):bool;

Solution

As the above error messages also refer to it, the solution is relatively straightforward. The implementation needs to be updated with type information, or the #[ReturnTypeWillChange] attribute must be added to the affected methods.

This pull implements a the later, to maintain the current PHP compatibility: PHP >= 7.1.0.

Fortunately, attributes syntax just comments on PHP 7.x, the added lines may look weird but ignored by PHP.

Note: Attributes are "references" to classes, so namespace rules apply. That's why the attributes incorporate the backslash as well, i.e.: #[\ReturnTypeWillChange].

Test

To successfully run PHPUnit tests under PHP 8.1, I used the following commands:

composer config minimum-stability dev
composer require phpunit/phpunit:dev-master --dev
composer update --prefer-stable
vendor/bin/phpunit

I tested these changes on 7.1.33, 7.2.34, 7.3.30, 7.4.23, 8.0.10 and 8.1.0beta3

References

Affected built-in interfaces:

• https://www.php.net/manual/en/class.jsonserializable.php
• https://www.php.net/manual/en/class.arrayaccess.php
• https://www.php.net/manual/en/class.countable

ReturnTypeWillChange attribute introduced in this rfc: https://wiki.php.net/rfc/internal_method_return_types

Attributes in general: https://www.php.net/manual/en/language.attributes.php

[marcelthole]

i really like this pr to add support for PHP 8.1 although it's still a RC. Maybe some additional notes:

• add explicit support for each version in the composer.json like ^7.1.0 || ~8.0.0 || ~8.1.0
• add php 8.1 in the github test matrix

[om4csaba]

Hi @marcelthole,

I intentionally left out the composer.json changes because of the implied minimum stability. The PHPUnit 10.x is not out yet, and the 9.5.x is failing under PHP 8.1. So, to make the Composer install or update work in PHP 8.1, apart from the changes you are mentioned, the config also needs to include something like this:

{
"minimum-stability": "dev",
"prefer-stable": true
}

In my (not exhausting) test, running the Composer with the above config installs the dev version of Symfony packages. So that's why I instead just documented how to install it on 8.1.

If this constrain is OK with you, I am happy to push a commit for that.

[Neirda24]

Since the support of PHP is at least 7.1. It means return types already exists. why not directly add those instead of the attribute ?

[marcelthole]

@Neirda24 for the Paths object your solution would work. The offsetGet could return ?PathItem and the problem is fixed. For the Responses class this is not possible because the offsetGet Method of the Responses could return Response|Reference|null and union types are not supported below PHP 8.0.

So yeah, some points here could use the correct return type but not all of them.

[Neirda24]

Indeed. @marcelthole . But that's already a good leap forward. Keep in ,ind that those warnings only come from native PHP objects (like ArrayAccess). I think those are the priority to fix. Then indeed use types where we can and phpdoc types where Union should be used.

[om4csaba]

As now PHP 8.1 released, I removed all unnecessary dependency manipulations.

The PHPUnit 10.x is not out yet, and the 9.5.x is failing under PHP 8.1.

It turned out PHPUnit 9.5.x is compatible with PHP 8.1. No need for change.

Since the support of PHP is at least 7.1. It means return types already exists. why not directly add those instead of the attribute ?

On older PHP, the built-in classes do not define return types. So if a user-land code defines it, it will fail on 7.x - 8.0. If not, on 8.1+.

The #[ReturnTypeWillChange] attribute is the only method to support ArrayAccess, JsonSerializable and Countable on PHP 7.x to 8.1.

Edited to add: Because Liskov, we can define return types, the failure comes from parameter types.

[om4csaba]

Hi @cebe, I ran GH Actions against this pull in our fork. It looks like newer PHP versions are failing with older Symfony: https://github.com/OM4/php-openapi/runs/4672017289?check_suite_focus=true

I address those errors in #134

[garethellis36]

@cebe Hi - please can you let me know if there is anything I can do to help get the required changes for PHP 8.1 compatibility merged? This is the remaining blocker for my application's upgrade. I'm happy to pitch in if necessary :)

[gniewomir-mohawk]

@cebe Hi - please can you let me know if there is anything I can do to help get the required changes for PHP 8.1 compatibility merged? This is the remaining blocker for my application's upgrade. I'm happy to pitch in if necessary :)

Hi, another volunteer here :) If i can help, I will.

[shadowhand]

@cebe I sent you an email with a sponsorship offer to get this moving.

[shadowhand]

I've opened a PR against this PR that will fix the build failure: https://github.com/OM4/php-openapi/pull/3

[cebe]

Thanks @shadowhand , I merged that fix in #162. Thank you @om4csaba ! I merged this with some adjustments, see #162.