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 <le...@php.net> wrote:
>
>
> On Sun, Aug 21, 2016 at 9:07 AM, Levi Morrison <le...@php.net> wrote:
>
>>
>>
>> On Sun, Aug 21, 2016 at 8:37 AM, Aaron Piotrowski <aa...@trowski.com>
>> wrote:
>>
>>> Hi Marco,
>>>
>>> > On Aug 19, 2016, at 1:31 PM, Marco Pivetta <ocram...@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.
>
