Add return type to satisfy interface implementation and fix deprecation warning #20628
Conversation
| @@ -101,7 +101,7 @@ public function __toString() | |||
| return $this->setting . ' ' . implode(' OR ', $checks); | |||
| } | |||
|
|
|||
| public function jsonSerialize() | |||
| public function jsonSerialize(): mixed | |||
There was a problem hiding this comment.
@michalkleiner The typehint mixed was introduced with PHP 8.0. See https://www.php.net/manual/en/language.types.declarations.php
So we can't use it as our code needs to be compatible with PHP 7.2+
There was a problem hiding this comment.
In that case we can't be implementing \JsonSerializable since that's the cause of the warning.
There was a problem hiding this comment.
It can be suppressed using the #[\ReturnTypeWillChange] comment
There was a problem hiding this comment.
Yep, that was the alternative solution. Will update the PR to add that instead of the return type.
There was a problem hiding this comment.
@michalkleiner I just had another thought on that. Actually we could also simply set string as return type hint.
As this is more specific as mixed, it should be compatible with that definition, but should be a lot better than the ReturnTypeWillChange workaround.
There was a problem hiding this comment.
Yep, that works for me. Updated the PR to that effect. Do we want to add it to the __toString method as well since that's the direct source of the output for the method in question?
There was a problem hiding this comment.
Yes. why not.
Imho we should try to add return types where ever it is safely possible.
Description:
Add return type
mixedto satisfy the interface the class is implementing.fixes #20627
Review