search

cebe / php-openapi

Read and write OpenAPI yaml/json files and make the content accessible in PHP objects.
MIT License
466 stars 87 forks source link

PHP 8.1 deprecation warnings #141

Closed acelaya closed 2 years ago

acelaya commented 2 years ago

Hey!

I just came across this lib, and it's very nice. However, it throws a couple of deprecation warnings when using it with PHP 8.1

Deprecated: Return type of cebe\openapi\spec\Paths::offsetExists($offset) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Paths.php on line 186
Deprecated: Return type of cebe\openapi\spec\Paths::offsetGet($offset) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Paths.php on line 197
Deprecated: Return type of cebe\openapi\spec\Paths::offsetSet($offset, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Paths.php on line 208
Deprecated: Return type of cebe\openapi\spec\Paths::offsetUnset($offset) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Paths.php on line 218
Deprecated: Return type of cebe\openapi\spec\Paths::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Paths.php on line 229
Deprecated: Return type of cebe\openapi\spec\Paths::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Paths.php on line 239
Deprecated: Return type of cebe\openapi\json\JsonReference::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/json/JsonReference.php on line 129
Deprecated: 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 in /app/vendor/cebe/php-openapi/src/spec/Responses.php on line 176
Deprecated: Return type of cebe\openapi\spec\Responses::offsetGet($offset) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Responses.php on line 187
Deprecated: Return type of cebe\openapi\spec\Responses::offsetSet($offset, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Responses.php on line 198
Deprecated: Return type of cebe\openapi\spec\Responses::offsetUnset($offset) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Responses.php on line 208
Deprecated: Return type of cebe\openapi\spec\Responses::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Responses.php on line 219
Deprecated: Return type of cebe\openapi\spec\Responses::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/cebe/php-openapi/src/spec/Responses.php on line 229

If you are interested in supporting PHP 8.1, I can contribute a fix for these.

acelaya commented 2 years ago

I have created https://github.com/cebe/php-openapi/pull/143, which should fix this issue.

Once you have approved the workflow, I will address anything else which does not work.

acelaya commented 2 years ago

I have pushed the fixes for phpstan. Could you kindly approve the builds again?

Eugentis commented 2 years ago

@cebe do you have any terms about release v.1.7.0? Could you aggregate it in some branch for some dev preparations with PHP 8.1?

cebe commented 2 years ago

I have no timeline yet, but I'll merge PHP 8.1 fixes into master soon.

garethellis36 commented 2 years ago

I had a look at doing a pull request for this. The problem is, I understood from reading previous PRs that you did not want to accept the use of the #[ReturnTypeWillChange] attribute, but preferred to see the correct return types added.

However, the return type of cebe\openapi\spec\Responses::offsetGet() needs to be mixed; that type was only added in PHP 7.4, and this library supports PHP 7.1. That is the only method that is an issue - all others deprecations in PHP 8.1 can have the correct return types added without problems.

Therefore I think the only choices are:

garethellis36 commented 2 years ago

Can you clarify something please? The doc block for Responses::offsetGet() says "can return all value types", hence my assertion above that it needs mixed.

    /**
     * Offset to retrieve
     * @link http://php.net/manual/en/arrayaccess.offsetget.php
     * @param mixed $offset The offset to retrieve.
     * @return mixed Can return all value types.
     */
    public function offsetGet($offset)
    {
        return $this->getResponse($offset);
    }

However, the docblock for getResponse() suggests that it will actually return one of two classes, or null:

    /**
     * @param string $statusCode HTTP status code
     * @return Response|Reference|null
     */
    public function getResponse($statusCode)
    {
        return $this->_responses[$statusCode] ?? null;
    }

If I am reading the code correctly, Response and Reference both implement two common interfaces: SpecObjectInterface, and DocumentContextInterface. In which case, could this issue be resolved by adding a shared interface? Excuse the naming, I'm not familiar enough with the concepts to propose something better:

interface SharedInterface extends SpecObjectInterface, DocumentContextInterface {}

offsetGet() can then have a PHP 7.1-compatible return type:

    /**
     * Offset to retrieve
     * @link http://php.net/manual/en/arrayaccess.offsetget.php
     * @param mixed $offset The offset to retrieve.
     * @return mixed Can return all value types.
     */
    public function offsetGet($offset): ?SharedObjectInterface
    {
        return $this->getResponse($offset);
    }
cebe commented 2 years ago

This is fixed by #162