Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95345
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.42 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAFMT4NpJuXXtvbCFvxYfxq=fJncirO4b9_oBT46vTvrSLiJ-uA@mail.gmail.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>
 <EB599E31-B569-40DB-9E1C-1F9921AF687E@trowski.com> <CAFMT4NoACO4c_62nV8cc2gXfHdRH8JbmJm=XcANAV3CGhwij2w@mail.gmail.com>
 <CAFMT4NpJuXXtvbCFvxYfxq=fJncirO4b9_oBT46vTvrSLiJ-uA@mail.gmail.com>
Date: Sun, 21 Aug 2016 17:42:30 +0200
Message-ID: <CADyq6sJYJy_STaD30P0OJX_Fpa4abN1b8Qk4a_qvKfEq1C0tsQ@mail.gmail.com>
To: Levi Morrison <levim@php.net>
Cc: Aaron Piotrowski <aaron@trowski.com>, internals <internals@lists.php.net>, 
	Stanislav Malyshev <smalyshev@gmail.com>
Content-Type: multipart/alternative; boundary=001a1144fb74c3c62a053a96c837
Subject: Re: [PHP-DEV] ReflectionType::__toString() prepending \ to class names
From: ocramius@gmail.com (Marco Pivetta)

--001a1144fb74c3c62a053a96c837
Content-Type: text/plain; charset=UTF-8

Including the question mark still breaks code-gen based on simple nullable
types. PHP 7.0.x-compliant code will do following:

 - reflect function foo(Bar $bar = null) {}
 - cast ReflectionType to string
 - crash (unrecognized `?` symbol)

That's part of what I tested on Friday on just one of the many libraries
involved.

The new API already includes `ReflectionType#allowsNull()`, so that's
sufficient to build forward-compliant codegen. Breaking BC is a no-go
though.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On Sun, Aug 21, 2016 at 5:39 PM, Levi Morrison <levim@php.net> wrote:

>
>
> On Sun, Aug 21, 2016 at 9:07 AM, Levi Morrison <levim@php.net> wrote:
>
>>
>>
>> On Sun, Aug 21, 2016 at 8:37 AM, Aaron Piotrowski <aaron@trowski.com>
>> wrote:
>>
>>> Hi Marco,
>>>
>>> > On Aug 19, 2016, at 1:31 PM, Marco Pivetta <ocramius@gmail.com> wrote:
>>> >
>>> > Hi Aaron et all,
>>> >
>>> > I tried to implement support for 7.1 in zend-code as a start:
>>> >
>>> > https://github.com/zendframework/zend-code/pull/87
>>> >
>>> > A few issues arise:
>>> >
>>> > * `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)
>>> >
>>>
>>> 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:
>>> >
>>> > ./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
>>>
>>>
>>> --
>>> PHP Internals - PHP Runtime Development Mailing List
>>> To unsubscribe, visit: http://www.php.net/unsub.php
>>>
>>
>> This is too big of a revert. You must attempt to generate exactly what
>> was written in user code which requires the prepended question mark.
>>
>
> To clarify we do resolve certain things such as names which are aliased by
> use and the like. But we can't resolve all names such as self and parent
> because of traits. The use of toString is to dump out the type information
> in a string that would be suitable for regenerating that exact type
> declaration only. It is not meant to be parsed or analyzed. It was designed
> this way from the beginning, and any deviation of this is a misuse. By not
> prepending the ? we will break other libs, so it's a BC break either way.
> Please put the question mark back as discussed in the beginning. Do not
> include the leading slash.
>

--001a1144fb74c3c62a053a96c837--