From 0b381dbe1d50a40f12115ea97b9bd0dd5a04b075 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Wed, 26 Jun 2019 17:59:51 +0200 Subject: [PATCH] improve error messages in the event dispatcher --- .../EventDispatcher/EventDispatcher.php | 10 +++---- .../LegacyEventDispatcherProxy.php | 12 ++++---- .../Tests/EventDispatcherTest.php | 27 +++++++++++++++++ .../Tests/LegacyEventDispatcherTest.php | 30 +++++++++++++++++++ 4 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/Symfony/Component/EventDispatcher/EventDispatcher.php b/src/Symfony/Component/EventDispatcher/EventDispatcher.php index 304caec1d1a79..a5f0b7eda5433 100644 --- a/src/Symfony/Component/EventDispatcher/EventDispatcher.php +++ b/src/Symfony/Component/EventDispatcher/EventDispatcher.php @@ -54,15 +54,13 @@ public function dispatch($event/*, string $eventName = null*/) if (\is_object($event)) { $eventName = $eventName ?? \get_class($event); - } else { - @trigger_error(sprintf('Calling the "%s::dispatch()" method with the event name as first argument is deprecated since Symfony 4.3, pass it second and provide the event object first instead.', EventDispatcherInterface::class), E_USER_DEPRECATED); + } elseif (\is_string($event) && (null === $eventName || $eventName instanceof Event)) { + @trigger_error(sprintf('Calling the "%s::dispatch()" method with the event name as the first argument is deprecated since Symfony 4.3, pass it as the second argument and provide the event object as the first argument instead.', EventDispatcherInterface::class), E_USER_DEPRECATED); $swap = $event; $event = $eventName ?? new Event(); $eventName = $swap; - - if (!$event instanceof Event) { - throw new \TypeError(sprintf('Argument 1 passed to "%s::dispatch()" must be an instance of %s, %s given.', EventDispatcherInterface::class, Event::class, \is_object($event) ? \get_class($event) : \gettype($event))); - } + } else { + throw new \TypeError(sprintf('Argument 1 passed to "%s::dispatch()" must be an object, %s given.', EventDispatcherInterface::class, \is_object($event) ? \get_class($event) : \gettype($event))); } if (null !== $this->optimized && null !== $eventName) { diff --git a/src/Symfony/Component/EventDispatcher/LegacyEventDispatcherProxy.php b/src/Symfony/Component/EventDispatcher/LegacyEventDispatcherProxy.php index 534eec98a5b63..afa3e988d0057 100644 --- a/src/Symfony/Component/EventDispatcher/LegacyEventDispatcherProxy.php +++ b/src/Symfony/Component/EventDispatcher/LegacyEventDispatcherProxy.php @@ -16,7 +16,7 @@ use Symfony\Contracts\EventDispatcher\EventDispatcherInterface as ContractsEventDispatcherInterface; /** - * An helper class to provide BC/FC with the legacy signature of EventDispatcherInterface::dispatch(). + * A helper class to provide BC/FC with the legacy signature of EventDispatcherInterface::dispatch(). * * This class should be deprecated in Symfony 5.1 * @@ -57,15 +57,13 @@ public function dispatch($event/*, string $eventName = null*/) if (\is_object($event)) { $eventName = $eventName ?? \get_class($event); - } else { - @trigger_error(sprintf('Calling the "%s::dispatch()" method with the event name as first argument is deprecated since Symfony 4.3, pass it second and provide the event object first instead.', ContractsEventDispatcherInterface::class), E_USER_DEPRECATED); + } elseif (\is_string($event) && (null === $eventName || $eventName instanceof Event)) { + @trigger_error(sprintf('Calling the "%s::dispatch()" method with the event name as the first argument is deprecated since Symfony 4.3, pass it as the second argument and provide the event object as the first argument instead.', ContractsEventDispatcherInterface::class), E_USER_DEPRECATED); $swap = $event; $event = $eventName ?? new Event(); $eventName = $swap; - - if (!$event instanceof Event) { - throw new \TypeError(sprintf('Argument 1 passed to "%s::dispatch()" must be an instance of %s, %s given.', ContractsEventDispatcherInterface::class, Event::class, \is_object($event) ? \get_class($event) : \gettype($event))); - } + } else { + throw new \TypeError(sprintf('Argument 1 passed to "%s::dispatch()" must be an object, %s given.', ContractsEventDispatcherInterface::class, \is_object($event) ? \get_class($event) : \gettype($event))); } $listeners = $this->getListeners($eventName); diff --git a/src/Symfony/Component/EventDispatcher/Tests/EventDispatcherTest.php b/src/Symfony/Component/EventDispatcher/Tests/EventDispatcherTest.php index e89f78cda8e89..658a94123933e 100644 --- a/src/Symfony/Component/EventDispatcher/Tests/EventDispatcherTest.php +++ b/src/Symfony/Component/EventDispatcher/Tests/EventDispatcherTest.php @@ -412,6 +412,33 @@ public function testMutatingWhilePropagationIsStopped() $this->assertTrue($testLoaded); } + + /** + * @group legacy + * @expectedDeprecation Calling the "Symfony\Component\EventDispatcher\EventDispatcherInterface::dispatch()" method with the event name as the first argument is deprecated since Symfony 4.3, pass it as the second argument and provide the event object as the first argument instead. + */ + public function testLegacySignatureWithoutEvent() + { + $this->dispatcher->dispatch('foo'); + } + + /** + * @group legacy + * @expectedDeprecation Calling the "Symfony\Component\EventDispatcher\EventDispatcherInterface::dispatch()" method with the event name as the first argument is deprecated since Symfony 4.3, pass it as the second argument and provide the event object as the first argument instead. + */ + public function testLegacySignatureWithEvent() + { + $this->dispatcher->dispatch('foo', new Event()); + } + + /** + * @expectedException \TypeError + * @expectedExceptionMessage Argument 1 passed to "Symfony\Component\EventDispatcher\EventDispatcherInterface::dispatch()" must be an object, string given. + */ + public function testLegacySignatureWithNewEventObject() + { + $this->dispatcher->dispatch('foo', new ContractsEvent()); + } } class CallableClass diff --git a/src/Symfony/Component/EventDispatcher/Tests/LegacyEventDispatcherTest.php b/src/Symfony/Component/EventDispatcher/Tests/LegacyEventDispatcherTest.php index 49aa2f9ff3f92..ffa767cf8b45c 100644 --- a/src/Symfony/Component/EventDispatcher/Tests/LegacyEventDispatcherTest.php +++ b/src/Symfony/Component/EventDispatcher/Tests/LegacyEventDispatcherTest.php @@ -14,12 +14,42 @@ use Symfony\Component\EventDispatcher\Event; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\LegacyEventDispatcherProxy; +use Symfony\Contracts\EventDispatcher\Event as ContractsEvent; /** * @group legacy */ class LegacyEventDispatcherTest extends EventDispatcherTest { + /** + * @group legacy + * @expectedDeprecation The signature of the "Symfony\Component\EventDispatcher\Tests\TestLegacyEventDispatcher::dispatch()" method should be updated to "dispatch($event, string $eventName = null)", not doing so is deprecated since Symfony 4.3. + * @expectedDeprecation Calling the "Symfony\Contracts\EventDispatcher\EventDispatcherInterface::dispatch()" method with the event name as the first argument is deprecated since Symfony 4.3, pass it as the second argument and provide the event object as the first argument instead. + */ + public function testLegacySignatureWithoutEvent() + { + $this->createEventDispatcher()->dispatch('foo'); + } + + /** + * @group legacy + * @expectedDeprecation The signature of the "Symfony\Component\EventDispatcher\Tests\TestLegacyEventDispatcher::dispatch()" method should be updated to "dispatch($event, string $eventName = null)", not doing so is deprecated since Symfony 4.3. + * @expectedDeprecation Calling the "Symfony\Contracts\EventDispatcher\EventDispatcherInterface::dispatch()" method with the event name as the first argument is deprecated since Symfony 4.3, pass it as the second argument and provide the event object as the first argument instead. + */ + public function testLegacySignatureWithEvent() + { + $this->createEventDispatcher()->dispatch('foo', new Event()); + } + + /** + * @expectedException \TypeError + * @expectedExceptionMessage Argument 1 passed to "Symfony\Contracts\EventDispatcher\EventDispatcherInterface::dispatch()" must be an object, string given. + */ + public function testLegacySignatureWithNewEventObject() + { + $this->createEventDispatcher()->dispatch('foo', new ContractsEvent()); + } + protected function createEventDispatcher() { return LegacyEventDispatcherProxy::decorate(new TestLegacyEventDispatcher());