-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] Add ReflectionHelper for native or static reflection of class #18578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
hason
commented
Apr 18, 2016
Q | A |
---|---|
Branch? | master |
Bug fix? | yes |
New feature? | maybe |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #18188, #18214 |
License | MIT |
Doc PR |
00858a5
to
cd78938
Compare
A reflection parser is not required here, and we already know how to get the type of arguments that use non-existing classes: |
@nicolas-grekas Really? This PR solves the problem with existing classes that depend on non-existing classes. That's a big difference. |
Do we really need to get information about such classes? |
Yes, see #18188 |
👎, using a static parser will be a disaster from a perf point of view for an edge case (when it is called). |
@dunglas The static parser is an optional as same as |
Ok I missed that it was in I've some concerns anyway:
|
I don't know how to catch fatal errors in PHP < 7, |
So, I played a bit with this corner case, looks like there is no other way around, even in PHP 7 where missing parent classes are hard fatal errors, not |
* | ||
* @return \ReflectionClass | ||
*/ | ||
public function getReflection($class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getReflectionClass
?
Ok +0 as there is no other solution. |
Shouldn't this be applied on 2.8 (I did not check)? |
@@ -29,6 +30,12 @@ class AutowirePass implements CompilerPassInterface | |||
private $definedTypes = array(); | |||
private $types; | |||
private $ambiguousServiceTypes = array(); | |||
private $reflectionHelper; | |||
|
|||
public function __construct(ReflectionHelper $reflectionHelper = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of injecting an object, I suggest a string:
$reflectionClassClassname = \ReflectionClass::class
Could it be made to work this way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because is neccessary to call __toString
method for GoAOP.
@hason see patch proposal here: master...nicolas-grekas:no-parent |
cd78938
to
b106b0c
Compare
@dunglas @nicolas-grekas What do you mean about a method that toggle static reflection? |
Given this covers really an edge case, I'd personally prefer not adding any new public interface for it. That's why and how I proposed the previous suggestion. |
Hello, guys! I noticed that you faced this ugly issue with unavailable parent classes. It's really bad one and sadly can be solved only with static reflection. |
@nicolas-grekas @lisachenko This feature is optional and has to by enabled by |
Looks like we are allowed to throw in the autoloader, thus work around this fatal error with a throwing autoloader. See #18600 |
Closed in favour of #18600. |