Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96052
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.213.169 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAOWwgpn1_ZHxQq9FpiwCYoqT3kVHH0k3LgO7E+eX5N2ybjDtrg@mail.gmail.com>
References: <CAOWwgpn1_ZHxQq9FpiwCYoqT3kVHH0k3LgO7E+eX5N2ybjDtrg@mail.gmail.com>
Date: Wed, 21 Sep 2016 15:14:47 +0200
Message-ID: <CAF+90c9XW7G-7CtKCWt+JOm5vE1XTnC92AkHvmBe9fohTOWeKA@mail.gmail.com>
To: Nicolas Grekas <nicolas.grekas@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=94eb2c14e262587140053d045459
Subject: Re: [PHP-DEV] Fix ReflectionType::__toString() BC break
From: nikita.ppv@gmail.com (Nikita Popov)

--94eb2c14e262587140053d045459
Content-Type: text/plain; charset=UTF-8

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

--94eb2c14e262587140053d045459--