Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[EventDispatcher] Improve method resolving when event is omitted in tag #50777

Open
wants to merge 1 commit into
base: 7.2
Choose a base branch
from
Open

[EventDispatcher] Improve method resolving when event is omitted in tag #50777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php
View file
Open in desktop
Expand Up @@ -76,7 +76,7 @@ public function process(ContainerBuilder $container)
continue;
}

$event['method'] ??= '__invoke';
$event['method'] ??= $this->resolveListenerMethodFromPublicMethod($container, $id);
$event['event'] = $this->getEventFromTypeDeclaration($container, $id, $event['method']);
}

Expand Down Expand Up @@ -182,6 +182,23 @@ private function getEventFromTypeDeclaration(ContainerBuilder $container, string

return $name;
}

/**
* Resolve the listener method by checking if the class has only one public method.
* If such method exists, return its name, if multiple public methods exist, fallback on `__invoke`.
*/
private function resolveListenerMethodFromPublicMethod(ContainerBuilder $container, string $id): string
{
if (
null === ($class = $container->getDefinition($id)->getClass())
|| !($r = $container->getReflectionClass($class, false))
|| 1 !== \count($publicMethods = $r->getMethods(\ReflectionMethod::IS_PUBLIC))
) {
return '__invoke';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept this for BC, but I'm wondering if an exception should not be thrown here 🤔

}

return $publicMethods[0]->getName();
}
}

/**
Expand Down
22 changes: 19 additions & 3 deletions ...Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php
View file
Open in desktop
Expand Up @@ -377,8 +377,9 @@ public function testOmitEventNameOnTypedListener()
{
$container = new ContainerBuilder();
$container->setParameter('event_dispatcher.event_aliases', [AliasedEvent::class => 'aliased_event']);
$container->register('foo', TypedListener::class)->addTag('kernel.event_listener', ['method' => 'onEvent']);
$container->register('bar', TypedListener::class)->addTag('kernel.event_listener');
$container->register('foo', InvokableTypedListener::class)->addTag('kernel.event_listener', ['method' => 'onEvent']);
$container->register('bar', InvokableTypedListener::class)->addTag('kernel.event_listener');
$container->register('baz', TypedListenerWithOnlyOnePublicMethod::class)->addTag('kernel.event_listener');
$container->register('event_dispatcher');

$registerListenersPass = new RegisterListenersPass();
Expand All @@ -402,6 +403,14 @@ public function testOmitEventNameOnTypedListener()
0,
],
],
[
'addListener',
[
CustomEvent::class,
[new ServiceClosureArgument(new Reference('baz')), 'onEvent'],
0,
],
],
];
$this->assertEquals($expectedCalls, $definition->getMethodCalls());
}
Expand Down Expand Up @@ -517,7 +526,7 @@ final class AliasedEvent
{
}

final class TypedListener
final class InvokableTypedListener
{
public function __invoke(AliasedEvent $event): void
{
Expand All @@ -528,6 +537,13 @@ public function onEvent(CustomEvent $event): void
}
}

final class TypedListenerWithOnlyOnePublicMethod
{
public function onEvent(CustomEvent $event): void
{
}
}

final class GenericListener
{
public function __invoke(object $event): void
Expand Down