Hi Máté,
1.) A few other people off-list also shared their negative feelings about
> the E_STRICT. So I'm ok to use E_DEPRECATED instead.
> Now that I implemented the suppressing mechanism, I think using a separate
> error type is not needed anymore that much.
>
Wonderful!
> 2.) Unifying the TentativeReturnType and SuppressReturnTypeNotice
> attributes is an interesting idea, and I would love to get
> rid of the latter one. Although, my concern is that doing so would
> eliminate the possibility for an overriding method to declare
> its own native return types (it's possible that they've already defined
> return types): so any child class could literally define
> return types at its own will, while it was not possible to do before. I'm
> eager to listen to any solution that would address my concerns. :)
>
I'm not sure I get your concern: how would #[TentativeReturnType] conflict
with native return types?
I didn't think about it, but #[TentativeReturnType] could be useful to
raise a deprecation when a supertype wants to tell child classes that's is
going to tighten its return type, and that they should do so if theirs is
too broad. This is basically a generalization of what we're talking about,
isn't it?
> 3.) I'm not sure you noticed in the PR, but in fact, I also implemented
> the TentativeReturnType attribute. Currently, it can be used
> as below when DateTime::modify() is overridden to return the ?DateTime
> type:
>
> class MyDateTime extends DateTime
> {
> #[TentativeReturnType]
> public function modify(string $modifier): ?DateTime { return null; }
> }
>
> How does this syntax look like for you?
>
> Compared to your proposal, the main difference is that the attribute
> doesn't itself store any type information, but it's done among the
> function information as normally.
Oh, I didn't get this at first: you're telling that this native return type
would not be enforced when the attribute is declared? This should answer my
previous comment. I'm not sure yet this is a good idea, because it would
prevent userland libs from using this attribute in an effective way until
they bump to 8.1...
> In my opinion, the current implementation
> has multiple advantages over the one you proposed:
> - As far as I can tell, it would be *very* cumbersome to parse and validate
> type information from strings, rather than reusing the
> capabilities we already have. But I'd be happy to be corrected if I'm
> wrong.
>
- Apart from the compiler itself, I think parsing type info is more
> straightforward as currently implemented for users as well as any tooling
>
Maybe Nikita can shed some light on this (the 1st item mainly)?
> - We should also think about people who already declared return types for
> overriding methods. I don't see any good reason why they
> should repeat this information in the attribute.
>
In my proposal, the path for userland would be:
- in their next minor (still supporting PHP < 8.1): add the attribute;
- in their following major: user a native return type and remove the
attribute.
> - I'm also not sure how the attribute could be used to retrieve the
> tentative type? Should it return a ReflectionType? Returning
> just the string representation seems like subideal for me.
>
It would contain a list of strings in my proposal, to keep things boring
and expected yes.
Nicolas
