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

Suppress deprecated notice on PHP8.1 #236

Merged
merged 3 commits into from
Jan 31, 2022
Merged

Conversation

pyrou
Copy link
Contributor

@pyrou pyrou commented Nov 21, 2021

Related Issue/Intent

Resolves #235

Changes

Fix following notice

Return type of BenSampo\Enum\Enum::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

Breaking changes

Code is backward compatible for all currently supported php versions (^7.3|^8.0)

Fix following notice : Return type of BenSampo\Enum\Enum::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
@atymic
Copy link
Contributor

atymic commented Nov 21, 2021

This method never returns a string so it's going to cause type errors.

@pyrou
Copy link
Contributor Author

pyrou commented Nov 21, 2021

This method was documented as returning a string, it is calling ˋtoArray()ˋ that in its turn is documented to return a string.

In the end, it returns $value documented to hold a mixed value. So you are right, this can lead to a type error.

I'll change return type to mixed (including all related PHPdoc)

EDIT: mixed not compatible PHP ^7.3. As suggested by @spawnia, annotation ReturnTypeWillChange is used instead.

@pyrou
Copy link
Contributor Author

pyrou commented Dec 8, 2021

Ping @BenSampo

Co-authored-by: Benedikt Franke <[email protected]>
@eduardoturconi
Copy link

ping @BenSampo

1 similar comment
@nitrogenium
Copy link

ping @BenSampo

@BenSampo BenSampo merged commit 8a02a0d into BenSampo:master Jan 31, 2022
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.

Incompatible with PHP8.1
6 participants