Sorry to disagree, but I don’t see a problem in an implementation/subclass that has more specific types (and with PHP 7.4 this is even allowed if both specify type hints).
I don’t think that it makes a big difference either because I would definitely not want to make the jsonSerializable() part of the public PHP API. But in my code I try to have no method without return type annotation, so personally I prefer to be stricter than the interface in this case.
Technically, it’s okay to allow contravariance in the return types, i.e. be more strict in overriding/implementing methods, BUT in this very specific case, I see the :string return type as a false friend, because it (and the really bad method name) implies that the returned value is already the serialized value. Every coder knows, serialization effectively means [reversibly] convert to string. But as stated above, the PHP implementation will run this through json_encode, so what happens if you return a stringified json object from this method (because you need to follow the string return type) in a future change? It results in something broken
In this case you changed the behavior whether you document it in the type hints or not. It’s totally OK to do so if the method is not part of an API (and the object in question can’t be extended). And in this case it’s totally fine to fix the return type as well
It’s totally OK to do so if the method is not part of an API (and the object in question can’t be extended)
Everything that is not final or private is potentially used like an API (i.e. extended/overridden) and then you get a breaking behaviour. But yes, normally I also prefer the strict typing benefits over this potential issue, just in this case, there’s no real benefit (::jsonSerialize() isn’t called anywhere explicitly where the typehint could help prevent an non-obvious issue)
And in this case it’s totally fine to fix the return type as well
Sure, if you recognize in the first place, that you need to do this, but that’s my whole point really. It’s not obvious to everyone, that they should change the return type of the method and not just serialize their new object structure to a string and return that.
it makes clear what is actually returned and thus can help avoid bugs
Just want to restate that I think that does not really hold in this case, because jsonSerialize() is normally never called from user-land. So the strongest argument “pro” strict return type is “it’s technically ok”, which is pretty weak after all.
Point in case stays, if you have a class that declares jsonSerialize(): string and someone extends that class, he is no longer able to make jsonSerialize() return anything else than string.
Make the method (or class) final and it’s okay again (which it is in the case of NodeName), but without that a strict typehint on this interface method IMO adds more friction than it provides benefit.
Can we just agree not to introduce a CGL that would forbid users from specifying the return type annotation?
That’s all that concerns me.
Folks can still create non-final classes implementing the JsonSerializable interface if they really want (hint: they don’t ) - non of my business
) - non of my business
