Re: Fix ReflectionType::__toString() BC break
| From: | Nikita Popov | Date: | Wed, 21 Sep 2016 13:14:47 +0000 |
| Subject: | Re: Fix ReflectionType::__toString() BC break | ||
| References: | 1 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-96052@lists.php.net to get a copy of this message | ||
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