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/
