Hi Nicolas and Claude,
First of all, thanks for your comments! Let me answer them one by one:
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.
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. :)
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. 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
- 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.
- 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.
4.) Claude, I think you make a good point! So non-private, untyped
properties could be converted to typed properties a little bit more
gradually
if we made this "tentative" type mechanism available for properties as
well. Coincidentally, I recently came up with the idea that I'd like to fix
type
issues with internal class properties (e.g. lots of them don't respect the
strict_types directive), and I also thought about migrating them to typed
properties for PHP 9.0. All in all, I don't want to include properties in
the current proposal, but I'd probably work on it next time.
Regards:
Máté
Claude Pache <claude.pa...@gmail.com> ezt írta (időpont: 2021. márc. 8., H,
16:19):
>
> > Le 6 mars 2021 à 23:56, Máté Kocsis <kocsismat...@gmail.com> a écrit :
> >
> > Hi Internals,
> >
> > I've just finished the first iteration of the RFC (
> > https://wiki.php.net/rfc/internal_method_return_types) as well as the
> > implementation based on the feedback Nikita and Nicolas provided. Thus,
> I'd
> > like to start the discussion phase.
> >
> > Regards:
> > Máté
>
> Hi,
>
> Two remarks:
>
> (1) The RFC is about return type of non-final methods. The same issue
> might arise for typed properties, whose type are supposed to be invariant
> under inheritance; although it is admittedly rarely useful to redeclare
> them in the subclass. Should typed properties be handled in the same way,
> or should we simply recommend to not redeclare properties?
>
> (2) Nicolas Grekas has beaten about E_STRICT (E_DEPRECATED is fine in this
> case). I’m just adding this: By reintroducing E_STRICT you are effectively
> reverting the [Reclassify E_STRICT notices] RFC, whose motivation was to
> “simplify our error model and resolve the currently unclear role of strict
> standards notices”. Thus, you should propose a clear role of those
> resurrected E_STRICT notices, that justifies the complication of the error
> model.
>
> [Reclassify E_STRICT notices]:
> https://wiki.php.net/rfc/reclassify_e_strict
>
> —Claude
