Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96062
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.213.172 as permitted sender)
MIME-Version: 1.0
Sender: morrison.levi@gmail.com
In-Reply-To: <CADyq6sJtAwX67paCTOH-gGc2ag8Yu9egV6TYcDyjo2P+oCbecg@mail.gmail.com>
References: <CAOWwgpn1_ZHxQq9FpiwCYoqT3kVHH0k3LgO7E+eX5N2ybjDtrg@mail.gmail.com>
 <CAF+90c9XW7G-7CtKCWt+JOm5vE1XTnC92AkHvmBe9fohTOWeKA@mail.gmail.com>
 <CANUQDCh5tCcvicRaDwr-q8gGTME0=vYs0Go0GQtdPuKcnSRN-Q@mail.gmail.com> <CADyq6sJtAwX67paCTOH-gGc2ag8Yu9egV6TYcDyjo2P+oCbecg@mail.gmail.com>
Date: Wed, 21 Sep 2016 08:39:58 -0600
Message-ID: <CAFMT4NogVn5uw+s1SO+Echdyi7O51pXXwcxsLN=Heoxd13pShw@mail.gmail.com>
To: Marco Pivetta <ocramius@gmail.com>
Cc: Niklas Keller <me@kelunik.com>, Nicolas Grekas <nicolas.grekas@gmail.com>, 
	Nikita Popov <nikita.ppv@gmail.com>, "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Fix ReflectionType::__toString() BC break
From: levim@php.net (Levi Morrison)

How big of a deal will this be come 7.2? or 8.0?

A few projects will break on 7.1 regardless of whether the question
mark is prepended or not, as has already been discussed in previous
threads. The key is as Nikita has pointed out that if we don't prepend
it this method is basically useless and must be deprecated. Whereas if
we do prepend it then it works as it supposed to and is distinct from
getName().

On Wed, Sep 21, 2016 at 7:31 AM, Marco Pivetta <ocramius@gmail.com> wrote:
> That still means that code designed for 7.0 will break while reflecting 7.0
> code (`= null` scenario) when run on 7.1, effectively preventing upgrade,
> Niklas.
>
> On 21 Sep 2016 15:27, "Niklas Keller" <me@kelunik.com> wrote:
>
>> 2016-09-21 15:14 GMT+02:00 Nikita Popov <nikita.ppv@gmail.com>:
>>
>> > On Wed, Sep 21, 2016 at 10:55 AM, Nicolas Grekas <
>> nicolas.grekas@gmail.com
>> > >
>> > wrote:
>> >
>> > > Hello,
>> > >
>> > > our turn to be hit by this 7.1 BC break in Symfony:
>> > > `ReflectionType::__toString()` now adds a `?` in front of type hints
>> > where
>> > > `null` is allowed by their default values.
>> > >
>> > > But since `zend_bool` is really an `unsigned char`, we have plenty of
>> > room
>> > > to store and forward this semantic subtlety (nullable being set through
>> > > `?Foo $arg` vs `Foo $arg = null`) so that
>> `ReflectionType::__toString()`
>> > > can be accurate again, thus BC.
>> > >
>> > > See https://github.com/php/php-src/pull/2136
>> > >
>> > > Thank for you consideration of this patch.
>> > >
>> >
>> > I don't like this. The way I see it, either ReflectionType::__toString()
>> > gives you a complete representation of the type (which includes "?" no
>> > matter how you happen to write it in the source code) or it does not --
>> in
>> > which case this function is fundamentally broken and needs to go away. In
>> > other words, either we stick with the behavior currently implemented in
>> PHP
>> > 7.1, or we go back to the PHP 7 behavior (there will never be a leading
>> ?,
>> > it just returns the name) **and we deprecate the method**.
>> >
>> > I consider the last part important because going to the PHP 7 behavior
>> > fundamentally breaks the method (it is not actually a string
>> representation
>> > of the type). Furthermore, this forces consumers of ReflectionType to
>> > switch to the new API based on getName() and isNullable().
>> >
>> > Nikita
>> >
>>
>> As Nikita said, starting with PHP 7.1, we'll have ReflectionType::getName()
>> which does exactly what you want. You can use `__toString()` on previous
>> versions and use the proper `getName()` starting with PHP 7.1.
>>
>> Regards, Niklas
>>