Hello Maté,
https://wiki.php.net/rfc/internal_method_return_types) as well as the
> implementation based on the feedback Nikita and Nicolas provided.
Thanks for moving this topic forward, it's very much needed.
I like the word "tentative" you're using to describe the missing return
types.
I would be good with the RFC but I want to give a few different ideas to
challenge it a bit.
Here they are:
- About using "E_STRICT", I'm not really sold. I think these notices
should follow the same path as deprecations as far as reporting is
concerned. By using E_STRICT, many error handlers will throw exceptions,
while the code is fine really. Deprecations don't lead to exceptions
usually (at least they never do in any Symfony stack). Semantically, a
deprecation fits well to me also.
- Suppressing the notice is very much needed, but I think we can merge
the corresponding attribute with the TentativeReturnType that you mention
in "future scope".
- As a corollary, we might not need the new methods on ReflectionMethod:
instead, we might make getAttributes() return a TentativeReturnType
attribute for internal methods too.
To summarize my proposal, I think we need only one TentativeReturnType
attribute, that we would put on internal methods, and that would also be
allowed on userland methods. ReflectionMethod::gettAttributes() would
report these attributes for both internal and userland methods.
The attribute would need to convey the allowed types. This could be done
with a list of strings. Taking your example from the RFC, this would look
like this:
class MyDateTime extends DateTime{
/**
* @return DateTime|false
*/
#[TentativeReturnType(DateTime::class, 'false')] public
function modify(string $modifier) { return false; }}
In this example, this would duplicate the docblock, but many docblocks
contain extended return-type info that the engine doesn't understand
anyway (list of types, etc.)
When engine sees these attributes, it would behave as you describe in
the RFC, with one possible extension: it could also validate the
supplied list of types and check LSP on it (when there is a parent
type).
Does this make sense to you?
Nicolas
