Re: Fix ReflectionType::__toString() BC break

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

« previous php.internals (#96062) next »