Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95342
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain trowski.com designates 199.38.81.6 as permitted sender)
Content-Type: text/plain; charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
In-Reply-To: <CADyq6sJve5XyRmubtyY6MeSsJYLFn2Y=W5a1kYy_7rAhH-Kn7g@mail.gmail.com>
Date: Sun, 21 Aug 2016 09:37:56 -0500
Cc: internals <internals@lists.php.net>,
 Stanislav Malyshev <smalyshev@gmail.com>
Content-Transfer-Encoding: quoted-printable
Message-ID: <EB599E31-B569-40DB-9E1C-1F9921AF687E@trowski.com>
References: <0668D584-EE51-4932-85D7-01D8BF70E409@trowski.com> <CADyq6s+qQEwXSqZUB6cs=zabfp28is57ZGK4nKb4TxivvhiZ8Q@mail.gmail.com> <15E198DD-FF66-475D-ABBC-54ECD2B6BF62@trowski.com> <CADyq6s+PEq9dtbKvv=7dWH6maePTwKTj9ki9W_25w4waC+orMw@mail.gmail.com> <D3DD20C1-9EF4-44EB-83E7-2DCB115C8057@trowski.com> <CADyq6s+sCY3UGACKXr-bJi8z_YNOOZwdMJuzDgA5M3VvJkyqHg@mail.gmail.com> <E4EE6269-8D50-4529-9A21-10B40D4D3B4A@trowski.com> <CADyq6sK22PQaji6pa727k6VS-pK6HzQhM5ccBORhC303_O7JgA@mail.gmail.com> <CADyq6s+_fS--RaO9zm4rxnLJt1EsCAY4evhn3SjAnyC0cT=-UQ@mail.gmail.com> <CADyq6sJve5XyRmubtyY6MeSsJYLFn2Y=W5a1kYy_7rAhH-Kn7g@mail.gmail.com>
To: Marco Pivetta <ocramius@gmail.com>
Subject: Re: [PHP-DEV] ReflectionType::__toString() prepending \ to class names
From: aaron@trowski.com (Aaron Piotrowski)

Hi Marco,

> On Aug 19, 2016, at 1:31 PM, Marco Pivetta <ocramius@gmail.com> wrote:
>=20
> Hi Aaron et all,
>=20
> I tried to implement support for 7.1 in zend-code as a start:
>=20
> https://github.com/zendframework/zend-code/pull/87
>=20
> A few issues arise:
>=20
> * `ReflectionType#__toString()` is too volatile, especially if we want =
to
> support multiple versions of PHP, therefore it's a good idea to not =
think
> too much about it, and instead deprecate it. Most issues I had while
> working with the feature were related with string formatting, and =
that's
> simply gotta die: just using a more specific API should cut it =
(getName,
> getClass, isNullable, etc. As few strings as possible, please!).
> * A page where we can see the current state of the `ReflectionType` =
API
> (and its subtypes) would be golden.
> * `ReflectionType#__toString()` seems to crash in very interesting =
ways
> when `?string` is reflected (see issue above - couldn't isolate =
precisely)
>=20

I've reverted the changes so that `ReflectionType::__toString()` is now =
identical to 7.0, including *not* prepending a ? for nullable types. The =
method is now just an alias of `ReflectionNamedType::getName()`.

`ReflectionType::__toString()` should be discouraged for code generation =
going forward, as it seems there's just not a way to add type features =
in a BC way. My attempt to incorporate nullable types in a way that =
would allow for even more complex types such as `callable(?\Type\Name, =
?bool)` just caused too many problems.

> I am currently going through the changes, and just figured that 7.1 =
implements https://wiki.php.net/rfc/reflectiontypeimprovements, even =
though the RFC was declined:
>=20
> ./sapi/cli/php -r 'class Foo { public function bar() : ?Foo {} } =
var_dump((new ReflectionMethod("Foo", "bar"))->getReturnType());'
> object(ReflectionNamedType)#2 (0) {
> }

Only `ReflectionNamedType` was added so the object returned from =
parameter and return types could have a `getName()` method. The rest of =
the RFC was not implemented. This should be completely BC while allowing =
future types like unions or callables. See some discussion here: =
https://github.com/php/php-src/pull/2068

Aaron Piotrowski