Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95320
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.53 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CADyq6sK22PQaji6pa727k6VS-pK6HzQhM5ccBORhC303_O7JgA@mail.gmail.com>
References: <0668D584-EE51-4932-85D7-01D8BF70E409@trowski.com>
 <CADyq6s+qQEwXSqZUB6cs=zabfp28is57ZGK4nKb4TxivvhiZ8Q@mail.gmail.com>
 <15E198DD-FF66-475D-ABBC-54ECD2B6BF62@trowski.com> <CADyq6s+PEq9dtbKvv=7dWH6maePTwKTj9ki9W_25w4waC+orMw@mail.gmail.com>
 <D3DD20C1-9EF4-44EB-83E7-2DCB115C8057@trowski.com> <CADyq6s+sCY3UGACKXr-bJi8z_YNOOZwdMJuzDgA5M3VvJkyqHg@mail.gmail.com>
 <E4EE6269-8D50-4529-9A21-10B40D4D3B4A@trowski.com> <CADyq6sK22PQaji6pa727k6VS-pK6HzQhM5ccBORhC303_O7JgA@mail.gmail.com>
Date: Fri, 19 Aug 2016 19:16:03 +0200
Message-ID: <CADyq6s+_fS--RaO9zm4rxnLJt1EsCAY4evhn3SjAnyC0cT=-UQ@mail.gmail.com>
To: Aaron Piotrowski <aaron@trowski.com>
Cc: internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=047d7b87465ea01522053a6fdb5d
Subject: Re: [PHP-DEV] ReflectionType::__toString() prepending \ to class names
From: ocramius@gmail.com (Marco Pivetta)

--047d7b87465ea01522053a6fdb5d
Content-Type: text/plain; charset=UTF-8

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 <ocramius@gmail.com> wrote:

> On Wed, Aug 17, 2016 at 7:17 PM, Aaron Piotrowski <aaron@trowski.com>
> wrote:
>
>>
>> > On Aug 17, 2016, at 12:02 PM, Marco Pivetta <ocramius@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/
>
>

--047d7b87465ea01522053a6fdb5d--