One reflector to rule them all

[kukulich]

One reflector to rule them all
one reflector to find them
one reflector to bring them all
and in the reflection bind them

[kukulich]

This is PoC of big BC break :)

Motivation:

<?php

namespace Foo;

const SOME_CONSTANT = 0;

class Boo
{
     public function method($parameter = SOME_CONSTANT)
     {}
}

It's not possible with ClassReflector and StringSourceLocator/SingleFileSourceLocator to resolve default value of the parameter.
ClassReflector can create the ReflectionClass but it's not possible to get the default value because it has to be resolved by ConstantReflector and there's no ConstantReflector in the ReflectionClass. It only partly works thanks to CompileNodeToValue and global functions defined and constant.

I propose to have one Reflector that can resolve everything based on the SourceLocator.

It can also improve the types. There are a lot of places where current interface Reflector is as the parameter but we almost everytime expect that it's ClassReflector.

I've also removed the ugly circular reference in https://github.com/Roave/BetterReflection/blob/5.0.x/src/BetterReflection.php#L89

---

There's one class Reflector in the PR but it should be interface and there should be one implementation, eg. BetterReflector, OneReflector (as 1Password) or any other name. Or the interface can be eg. ReflectorInterface`.

[kukulich]

@Ocramius Please look at this PR (when you have time) and tell me if it will be mergeable :) If yes, please also add the opinion about the names :) I’ll then make the build green.

[asgrim]

The concept here seems fine; there is a finite number of "types of symbols to reflect on" so growth is reasonably controlled, I think our concern originally (why we split into ClassReflector/FunctionReflector) is uncertainty over how many symbol types we'd want to reflect about. Now the library is much more defined, it seems reasonable.

I do also like that it solves the circular dependency issue 👍

[asgrim]

There's one class Reflector in the PR but it should be interface and there should be one implementation, eg. BetterReflector, OneReflector (as 1Password) or any other name. Or the interface can be eg. ReflectorInterface`.

I'd say keep the interface named Reflector (this becomes clear where the BC break is then - because the interface is changed, rather than changing interface into a class). For the implementation name I'm not worried about, maybe BetterReflector is fine, but I'll let @Ocramius opine later on that 😁

[Ocramius]

@kukulich from my PoV, approach is very much valid! It's a major BC break, but easy to migrate to it

[kukulich]

@Ocramius what do you think about the names of interface and the class?

[kukulich]

@Ocramius @asgrim Could you please give me a clue about the class and interface names? :) This PR blocks other my PR.

[Ocramius]

@kukulich Reflector for the interface, DefaultReflector for the implementation? WDYT?

DefaultReflector could also be SourceLocatorBasedReflector or such, but IMO not that important there.

[kukulich]

@Ocramius @asgrim Ready to review :)

[Ocramius]

One last change needed :D

[Ocramius]

🚢

[Ocramius]

Awesome, thanks @kukulich!