Skip to content

PHP 8.1 deprecation warnings #141

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

Closed
acelaya opened this issue Dec 9, 2021 · 7 comments
Closed

PHP 8.1 deprecation warnings #141

acelaya opened this issue Dec 9, 2021 · 7 comments

Comments

@acelaya
Copy link

acelaya commented Dec 9, 2021

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
Copy link
Author

acelaya commented Dec 10, 2021

I have created #143, which should fix this issue.

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

@acelaya
Copy link
Author

acelaya commented Dec 21, 2021

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

@cebe cebe added this to the 1.7.0 milestone Feb 9, 2022
@Eugentis
Copy link

Eugentis commented Mar 9, 2022

@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
Copy link
Owner

cebe commented Mar 11, 2022

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

@garethellis36
Copy link

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:

  • Make this change in a new major version which bumps the minimum PHP version to 7.4
  • OR use the attribute and keep the minimum PHP version at 7.1

@garethellis36
Copy link

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
Copy link
Owner

cebe commented Apr 20, 2022

This is fixed by #162

@cebe cebe closed this as completed Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants