Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:94391
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.161.178 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAFMT4NrEPA31tL-vuyOHSO6=w+HjzGD6e-WUwyeROP2PKZ82Qw@mail.gmail.com>
References: <CAFMT4Noe07E9--sasW-Fiw5B1hFFZbXDG2_wumkfZCbqM9wh7A@mail.gmail.com>
 <CAF+90c9RCfLdFGgLVXtc3iPy-LcPK3cfKqHNz-7Mm+QygER1Tg@mail.gmail.com>
 <1f39f97a-6aaf-8550-9f82-a7e80465f903@telia.com> <CAFMT4NrEPA31tL-vuyOHSO6=w+HjzGD6e-WUwyeROP2PKZ82Qw@mail.gmail.com>
Date: Tue, 5 Jul 2016 17:38:16 +0200
Message-ID: <CAF+90c-bFOovuJKDYpHJjBuBBT7cGFTPzvGwmCJijLSbGDyGdA@mail.gmail.com>
To: Levi Morrison <levim@php.net>
Cc: =?UTF-8?Q?Bj=C3=B6rn_Larsson?= <bjorn.x.larsson@telia.com>, 
	PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=94eb2c147ba2db2bcb0536e53d22
Subject: Re: [PHP-DEV] [RFC][Vote] ReflectionType Improvements
From: nikita.ppv@gmail.com (Nikita Popov)

--94eb2c147ba2db2bcb0536e53d22
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Tue, Jul 5, 2016 at 3:21 PM, Levi Morrison <levim@php.net> wrote:

> On Mon, Jul 4, 2016 at 10:51 AM, Bj=C3=B6rn Larsson
> <bjorn.x.larsson@telia.com> wrote:
> > Den 2016-06-30 kl. 23:57, skrev Nikita Popov:
> >>
> >> On Thu, Jun 30, 2016 at 6:06 PM, Levi Morrison <levim@php.net> wrote:
> >>
> >>> The RFC for improving ReflectionType[1] is now in voting phase. The
> >>> voting
> >>> window is June 30th through July 8th. I have not finished the patch b=
ut
> >>> I'll have it done before the end of voting.
> >>>
> >>>    [1]: https://wiki.php.net/rfc/ReflectionTypeImprovements
> >>
> >>
> >> Replying on-list as multiple people asked:
> >>
> >> I'm voting against this RFC, because it introduced not only the
> >> ReflectionNamedType class (which is reasonable), but also the
> >> ReflectionClassType class (which is not).
> >>
> >> My main objection to ReflectionClassType is that it is autoloading
> >> dependent (*). Something like getReturnType() on a class type hint wil=
l
> >> either return a ReflectionClassType if the class can be loaded, or a
> >> ReflectionNamedType if it can't. I think this is confusing and I'm sur=
e
> >> that this will lead to broken code: For example, people will try to us=
e
> >> "$type instanceof ReflectionClassType" to check whether something is a
> >> class type hint, while the currently correct way, which works
> >> independently
> >> of class loading, is to check isBuiltin() instead.
> >>
> >> I don't think that most consumers of ReflectionType are interested in
> >> obtaining a ReflectionClass for the type hint anyway (which is the onl=
y
> >> functionality that ReflectionClassType provides), and if necessary thi=
s
> >> can
> >> be easily done in userland. There is no need to over-complicate the
> >> ReflectionType functionality in this manner, especially with the
> proposed
> >> semantics.
> >>
> >> Nikita
> >>
> > Maybe one should split the vote into separate for each function.
> > I mean pity if vote fails because one function is not attractive while
> > the other one is...
> >
> > What do you think?
> >
> > Regards //Bj=C3=B6rn Larsson
>
> I think that it's disappointing that I received no feedback on this
> RFC at all and yet have this ratio of votes in the negative.
>
> I'll take this opportunity to share a portion of a conversation I had
> with Nikita that demonstrates why ReflectionClassType is actually
> useful:
>
> > Nikita: But does it really help them? I don't see a lot of differences
> between these two:
>     if ($type instanceof ReflectionClassType) {
>         $r =3D $type->getClass();
>     }
>     // and
>     if (class_exists($type->getName())) {
>         $r =3D new ReflectionClass($type->getName());
>     }
> > Levi: ...[I]t will fail for interfaces.] ReflectionClass can be built
> for those but class_exists() will be false.
> > Nikita: True :)
>
> The (currently) correct code would be:
>
>     if (class_exists($type->getName()) ||
> interface_exists($type->getName())) {
>         $r =3D new ReflectionClass($type->getName());
>     }
>
> But if we add some other type such as enums we may have to add more
> conditions. As long as ReflectionClass can be made from the type then
> doing it in the engine is fully forward compatible.
>
> I hope people who voted no will share why they have done so, but I
> also hope they'll switch to yes if they voted no because they don't
> see it as being useful.
>

To add to this, another issue we discussed OTR but didn't really come to a
conclusion on, is the handling of self and parent type hints by
ReflectionClassType.

On the one hand, ReflectionClassType can be advantageous here, because it
can resolve self/parent itself and return the appropriate ReflectionClass.
Otherwise the user would have to do this themselves (if getting a
ReflectionClass is a goal).

On the other hand, self/parent are not always statically resolvable. For
methods in traits self does not refer to the trait, but rather to the class
the trait will be used in. This means that when reflecting on a self type
on a trait you wouldn't get a ReflectionClassType (for something that is
clearly a class type hint). A similar issue also exists for self/parent on
closures.

Regards,
Nikita

--94eb2c147ba2db2bcb0536e53d22--