Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[DependencyInjection] Fix checking for interfaces in ContainerBuilder::getReflectionClass()#58822
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
carsonbot commentedNov 9, 2024
Hey! Thanks for your PR. You are targeting branch "7.2" but it seems your PR description refers to branch "5.4 or 7.2, TBD". Cheers! Carsonbot |
This looks like a bugfix to me. Please target 5.4. |
6f2be11 to1c3690bCompare
Let's see if there is consensus that this is a bug fix. |
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.
LGTM after some minor changes
| @@ -369,7 +369,7 @@ public function getReflectionClass(?string $class, bool $throw = true): ?\Reflec | |||
| $resource = new ClassExistenceResource($class, false); | |||
| $classReflector = $resource->isFresh(0) ? false : new \ReflectionClass($class); | |||
| } else { | |||
| $classReflector = class_exists($class) ? new \ReflectionClass($class) : false; | |||
| $classReflector =(class_exists($class) || interface_exists($class)) ? new \ReflectionClass($class) : false; | |||
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.
| $classReflector =(class_exists($class) ||interface_exists($class)) ?new \ReflectionClass($class) :false; | |
| $classReflector =class_exists($class) ||interface_exists($class,false) ?new \ReflectionClass($class) :false; |
…ainerBuilder::getReflectionClass().
1c3690b to6166e8fCompareThank you@donquixote. |
cf20e09 intosymfony:5.4Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Currently,
ContainerBuilder::getReflectionClass()supports interfaces only ifsymfony/configpackage is present.With this fix, it will do so without
symfony/config.Not sure how to do this.
All tests run in the monorepo, with all packages present, so
class_exists(ClassExistenceResource::class)will always be TRUE.To make this testable, we should make the condition dependent on resource tracking.
This is TBD.
This change would enable autowire for some services where this was previously not the case.