Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96057
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.43 as permitted sender)
MIME-Version: 1.0
Sender: julienpauli@gmail.com
In-Reply-To: <CAF+90c9XW7G-7CtKCWt+JOm5vE1XTnC92AkHvmBe9fohTOWeKA@mail.gmail.com>
References: <CAOWwgpn1_ZHxQq9FpiwCYoqT3kVHH0k3LgO7E+eX5N2ybjDtrg@mail.gmail.com>
 <CAF+90c9XW7G-7CtKCWt+JOm5vE1XTnC92AkHvmBe9fohTOWeKA@mail.gmail.com>
Date: Wed, 21 Sep 2016 15:32:38 +0200
Message-ID: <CAMUwpuQ9qY5Es1nZ2pA0vYv+0o6g6ZzK0Xh70QYQ1_0H1xvOJA@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: Nicolas Grekas <nicolas.grekas@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: jpauli@php.net (Julien Pauli)

On Wed, Sep 21, 2016 at 3:14 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:
> 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


People used __toString() because there was no other way to "get the
name" of the param.
But __toString() should give a "string representation of the objects
it is affected to".  The fact that in 7.0 it returned the exact name
of the param, was a coincidence.

Now, in 7.1 , we changed it to represent the param more accurately,
including the nullable fact, thus adding a '?' to the type ; BUT we
also added a way to get the name of the param (like you expect it) :
the getName() method.


So now, in 7.1 , one who used __toString() should now use getName().
And __toString() changed in 7.1 , breaking BC yes.  Should the break
stay ?  It is RM to decide.

But, if we must wait for PHP 8 to be able to change so little things,
then we perhaps need to change our release management to have one
major per year ?

I mean, we need to change things, we need to evoluate , and that can't
be done without breaking BC in some parts.
That is all the challenge behind BC breaks ....

RM thoughts ?



Julien.Pauli