-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[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
ServiceSubscriberInterfaceServiceSubscriberInterface
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. |
nicolas-grekas
left a comment
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
ServiceSubscriberInterfaceServiceSubscriberInterface
…ith `ServiceSubscriberInterface`
48b3262 to
123842a
Compare
|
Thank you @kbond. |
Continuation of #43479 per discussion with @nicolas-grekas.
Todo:
ServiceSubscriberTraitsupport