-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] Fix support for unions/intersections together with ServiceSubscriberInterface
#43930
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
ServiceSubscriberInterface
ServiceSubscriberInterface
src/Symfony/Component/DependencyInjection/Compiler/RegisterServiceSubscribersPass.php
Show resolved
Hide resolved
src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php
Show resolved
Hide resolved
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.
I think we'll also need to filter out any built-in types from union types, and to order the list of types for both union and intersection types.
We could do this in AutowirePass, when TypedReference are wired
src/Symfony/Component/DependencyInjection/Compiler/RegisterServiceSubscribersPass.php
Show resolved
Hide resolved
662c282
to
81cd56f
Compare
Sorry, I'm quite out of my element here. I'm not sure how/where to do this. |
Using pseudo-code, this is what is missing to me: --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
+++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
@@ -100,7 +100,7 @@ class AutowirePass extends AbstractRecursivePass
private function doProcessValue(mixed $value, bool $isRoot = false): mixed
{
if ($value instanceof TypedReference) {
- if ($ref = $this->getAutowiredReference($value)) {
+ if ($ref = $this->getAutowiredReference($value, true)) {
return $ref;
}
if (ContainerBuilder::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE === $value->getInvalidBehavior()) {
@@ -291,7 +291,7 @@ class AutowirePass extends AbstractRecursivePass
}
$getValue = function () use ($type, $parameter, $class, $method) {
- if (!$value = $this->getAutowiredReference($ref = new TypedReference($type, $type, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, Target::parseName($parameter)))) {
+ if (!$value = $this->getAutowiredReference($ref = new TypedReference($type, $type, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, Target::parseName($parameter)), false)) {
$failureMessage = $this->createTypeNotFoundMessageCallback($ref, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
if ($parameter->isDefaultValueAvailable()) {
@@ -346,7 +346,7 @@ class AutowirePass extends AbstractRecursivePass
/**
* Returns a reference to the service matching the given type, if any.
*/
- private function getAutowiredReference(TypedReference $reference): ?TypedReference
+ private function getAutowiredReference(TypedReference $reference, bool $filterType): ?TypedReference
{
$this->lastFailure = null;
$type = $reference->getType();
@@ -355,6 +355,13 @@ class AutowirePass extends AbstractRecursivePass
return $reference;
}
+ if ($filterType) {
+ if (is_union_type($type)) {
+ $type = remove_builtin_types($types);
+ }
+ $types = sort_types_in_union_or_intersection($types);
+ }
+
if (null !== $name = $reference->getName()) { |
And we'll also need a change on this line, since the leading
Adding |
d66eb54
to
e4f1fdd
Compare
Thank you, I've added the requested code. Still not sure how to test this. |
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.
thanks, we're almost done :)
can you try adding a few test cases that involve AutowirePass?
src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php
Show resolved
Hide resolved
src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php
Show resolved
Hide resolved
ServiceSubscriberInterface
ServiceSubscriberInterface
…ith `ServiceSubscriberInterface`
48b3262
to
123842a
Compare
Thank you @kbond. |
Continuation of #43479 per discussion with @nicolas-grekas.
Todo:
ServiceSubscriberTrait
support