On Mon, Aug 22, 2016 at 2:56 PM, Dan Ackroyd <dan...@basereality.com> wrote:
> On 22 August 2016 at 06:33, Nikita Popov <nikita....@gmail.com> wrote:
> >
> > Hi.
> >
> > Now that we have made ReflectionType::__toString() be useless and not
> > represent the type, this method should be deprecated in favor of using
> > ReflectionNamedType::__toString(), ::allowsNull() and any further
> extensions
> > that will be introduced later.
> >
>
> If we want to go that route, doesn't it make sense to:
>
> * Leave ReflectionType::__toString() as it pre-7.1.0beta3 - i.e. not
> including the ?nullable marker or the slash.
>
> * Explain that ReflectionType::__toString() is a legacy function that
> isn't nullable aware and that people should use the other functions to
> get the full type information for anything that might contain
> functions that have nullable parameters.
>
> * Add a ReflectionType::isNullable() function so that can be reflected
> properly, exactly as the user wrote them rather than an approximation.
>
> Levi Morrison wrote:
> > You must attempt to generate exactly what was written in user code
>
> This is exactly why I don't think it's acceptable for the type
> information to be including a question mark to indicate nullable.
>
> I use reflection to take a function that has this signature:
>
> function foo(Bar $sc = null) {...}
>
> which is valid PHP 5.x and 7.0 code, and then generate a decorated
> function like:
>
> function loggingFoo(Bar $sc = null) {
> echo "before \n";
> foo($sc);
> echo "after \n";
> }
>
> If the type for the parameter suddenly starts having a ?nullable in
> it, it means I can't run the code generator to produce code that can
> be used on 5.x and 7.0 - even though the code being reflected on is
> valid 5.x and 7.0 code.
>
> We need to be able to generate exactly what was written in the user
> code from Reflection, not just an equivalent form that only works on
> 7.1+
>
> cheers
> Dan
> Ack
>
Let's step back for a moment here and look at some data.
We introduced ReflectionType in a major release, a dot zero. Now in the
next minor release (a dot one) we are adjusting its toString representation
because of a new feature that affects the string representation of a single
class of types in Reflection, those with a default of null. There are four
projects affected by this change if it happens and at least one affected if
it does not. If this is changed then four very recent and active projects
are affected on a *single minor release* and if we do not change it then
*at best* it is redundant and at least one project is affected.
Furthermore, if generics or callable prototypes or union or intersection
types or theoretically any number of type system changes occurs then the
toString will change, and these four mentioned projects have to be adjusted
anyway.
The choice is very clear to me. We preserve the original intention and
prepend the question mark.
---
Lastly I acknowledge that my wording earlier was too strict; we don't
generate *exactly* what the user wrote because of aliases, namespaces etc.
A better descriptor would have been "equivalent".
