Hey Aaron,
I am currently going through the changes, and just figured that 7.1
implements https://wiki.php.net/rfc/reflectiontypeimprovements, even though
the RFC was declined:
./sapi/cli/php -r 'class Foo { public function bar() : ?Foo {} }
var_dump((new ReflectionMethod("Foo", "bar"))->getReturnType());'
object(ReflectionNamedType)#2 (0) {
}
Was there a newer RFC that I missed?
Marco Pivetta
http://twitter.com/Ocramius
http://ocramius.github.com/
On Wed, Aug 17, 2016 at 7:25 PM, Marco Pivetta <ocram...@gmail.com> wrote:
> On Wed, Aug 17, 2016 at 7:17 PM, Aaron Piotrowski <aa...@trowski.com>
> wrote:
>
>>
>> > On Aug 17, 2016, at 12:02 PM, Marco Pivetta <ocram...@gmail.com> wrote:
>> >
>> > That would have been a headache anyway. We saw it coming, and it will
>> be fixed on our end, but please don't try to outsmart it.
>> > I know that there is good intention on your side, but this is really
>> going to just make it an issue.
>>
>> Looks like this problem is more complicated than I thought. I thought
>> prepending the \ would mean little work on your end, but it appears I was
>> wrong.
>>
>> I'm still confused as to what's going on and what the best solution is...
>> Currently Doctrine is manually prepending \ to class names. Obviously your
>> logic would have to change between 7.0 and 7.1, but then going forward you
>> could rely on ReflectionType::__toString() to return a syntax-valid type
>> name, rather than modifying it. Or perhaps rather than relying on casting
>> to a string and examining the string, Doctrine should be using
>> ReflectionNamedType::getName() and ReflectionType::allowsNull() for 7.1 and
>> beyond. (Just a suggestion, I'd have to dig into the code to really
>> understand what's going on, and I don't have a ton of time to do so at the
>> moment.)
>>
>
> The problem is that we're not talking about 1 library, but a few (and I'm
> only talking about the ones I know of).
> Changing behavior is going to cause issues.
>
>
>> > From the codegen-side (I do write a lot of code generators), having `\`
>> prepended in front of stuff makes things just more complex to deal with,
>> since I have to strip it and re-introduce it anyway in multiple locations
>> in the code, while it should just be attached in the final output-logic bit.
>> > Instead, please keep the reflector on-spot: reflecting things, telling
>> us what they are. What the code generator does with the definitions is up
>> to the code generator after that.
>> >
>> > We have to adjust the code for `void` and `?` anyway, so this is just
>> more changes to keep track of, and it would break existing code.
>>
>> It sounds like you'd prefer the ? was not prepended to the string as
>> well, is that correct? Again it sounds like it would be better to use
>> methods other than __toString(). I understand __toString() was the only way
>> to get the type name before, but now that this has been fixed perhaps it
>> should be avoided in your use-cases.
>
>
> I think that adding the `?` would be semantically correct, from a
> reflector perspective (remember, we are only reflecting: please completely
> ignore the idea of using this for codegen, it is a separate domain).
>
> I can't tell you for sure right now, but I will check on Friday.
>
> Libraries that directly affect me personally are doctrine/common,
> zendframework/zend-code and ocramius/proxy-manager, so I am only talking
> about these 3 for now.
> If I remember correctly, in all three a `(string)` cast is being used for
> discovering scalar types, although I am not sure.
>
> Can you please poke me at EOD on Friday, so maybe we look at this together?
>
> Cheers,
>
> Marco Pivetta
>
> http://twitter.com/Ocramius
>
> http://ocramius.github.com/
>
>
