Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95270
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.43 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <E4EE6269-8D50-4529-9A21-10B40D4D3B4A@trowski.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>
Date: Wed, 17 Aug 2016 19:25:24 +0200
Message-ID: <CADyq6sK22PQaji6pa727k6VS-pK6HzQhM5ccBORhC303_O7JgA@mail.gmail.com>
To: Aaron Piotrowski <aaron@trowski.com>
Cc: internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=089e01160dfa607211053a47c148
Subject: Re: [PHP-DEV] ReflectionType::__toString() prepending \ to class names
From: ocramius@gmail.com (Marco Pivetta)

--089e01160dfa607211053a47c148
Content-Type: text/plain; charset=UTF-8

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/

--089e01160dfa607211053a47c148--