From cd06c1297b9eaae8ff22e75778f360f4e4b4a8a3 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 13 Apr 2017 10:36:10 +0200 Subject: [PATCH] Revert "minor #22039 Skip abstract definitions in compiler passes (chalasr)" This reverts commit 207d068a40f482aaaa0ef010d6c64f4de942dc46, reversing changes made to 483600717236810021c109695b563d1ccb6abbe9. --- .../RegisterEventListenersAndSubscribersPass.php | 4 ++-- ...egisterEventListenersAndSubscribersPassTest.php | 10 +++++++--- .../Compiler/AddConsoleCommandPassTest.php | 6 ++++-- .../DependencyInjection/AddConsoleCommandPass.php | 2 +- .../AddConsoleCommandPassTest.php | 8 +++++--- .../DependencyInjection/RegisterListenersPass.php | 4 ++-- .../RegisterListenersPassTest.php | 14 +++++++++----- 7 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php b/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php index ddfacebfa4..6d53d88aaf 100644 --- a/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php +++ b/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php @@ -79,7 +79,7 @@ class RegisterEventListenersAndSubscribersPass implements CompilerPassInterface uasort($subscribers, $sortFunc); foreach ($subscribers as $id => $instance) { if ($container->getDefinition($id)->isAbstract()) { - continue; + throw new InvalidArgumentException(sprintf('The abstract service "%s" cannot be tagged as a doctrine event subscriber.', $id)); } $em->addMethodCall('addEventSubscriber', array(new Reference($id))); @@ -95,7 +95,7 @@ class RegisterEventListenersAndSubscribersPass implements CompilerPassInterface uasort($listeners, $sortFunc); foreach ($listeners as $id => $instance) { if ($container->getDefinition($id)->isAbstract()) { - continue; + throw new InvalidArgumentException(sprintf('The abstract service "%s" cannot be tagged as a doctrine event listener.', $id)); } $em->addMethodCall('addEventListener', array( diff --git a/src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php b/src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php index 8a29331b0b..25776f8669 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php @@ -18,6 +18,9 @@ use Symfony\Component\DependencyInjection\Definition; class RegisterEventListenersAndSubscribersPassTest extends TestCase { + /** + * @expectedException \InvalidArgumentException + */ public function testExceptionOnAbstractTaggedSubscriber() { $container = $this->createBuilder(); @@ -29,10 +32,12 @@ class RegisterEventListenersAndSubscribersPassTest extends TestCase $container->setDefinition('a', $abstractDefinition); $this->process($container); - $this->assertSame(array(), $container->getDefinition('doctrine.dbal.default_connection.event_manager')->getMethodCalls()); } - public function testAbstractTaggedListenerIsSkipped() + /** + * @expectedException \InvalidArgumentException + */ + public function testExceptionOnAbstractTaggedListener() { $container = $this->createBuilder(); @@ -43,7 +48,6 @@ class RegisterEventListenersAndSubscribersPassTest extends TestCase $container->setDefinition('a', $abstractDefinition); $this->process($container); - $this->assertSame(array(), $container->getDefinition('doctrine.dbal.default_connection.event_manager')->getMethodCalls()); } public function testProcessEventListenersWithPriorities() diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConsoleCommandPassTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConsoleCommandPassTest.php index edf5f6a703..ac30740397 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConsoleCommandPassTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConsoleCommandPassTest.php @@ -62,6 +62,10 @@ class AddConsoleCommandPassTest extends TestCase ); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage The service "my-command" tagged "console.command" must not be abstract. + */ public function testProcessThrowAnExceptionIfTheServiceIsAbstract() { $container = new ContainerBuilder(); @@ -73,8 +77,6 @@ class AddConsoleCommandPassTest extends TestCase $container->setDefinition('my-command', $definition); $container->compile(); - - $this->assertSame(array(), $container->getParameter('console.command.ids')); } /** diff --git a/src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php b/src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php index 24f0213e64..c455229475 100644 --- a/src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php +++ b/src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php @@ -32,7 +32,7 @@ class AddConsoleCommandPass implements CompilerPassInterface $definition = $container->getDefinition($id); if ($definition->isAbstract()) { - continue; + throw new \InvalidArgumentException(sprintf('The service "%s" tagged "console.command" must not be abstract.', $id)); } $class = $container->getParameterBag()->resolveValue($definition->getClass()); diff --git a/src/Symfony/Component/Console/Tests/DependencyInjection/AddConsoleCommandPassTest.php b/src/Symfony/Component/Console/Tests/DependencyInjection/AddConsoleCommandPassTest.php index f5199fee35..c6af56a0a1 100644 --- a/src/Symfony/Component/Console/Tests/DependencyInjection/AddConsoleCommandPassTest.php +++ b/src/Symfony/Component/Console/Tests/DependencyInjection/AddConsoleCommandPassTest.php @@ -50,7 +50,11 @@ class AddConsoleCommandPassTest extends TestCase ); } - public function testProcessSkipAbstractDefinitions() + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage The service "my-command" tagged "console.command" must not be abstract. + */ + public function testProcessThrowAnExceptionIfTheServiceIsAbstract() { $container = new ContainerBuilder(); $container->setResourceTracking(false); @@ -62,8 +66,6 @@ class AddConsoleCommandPassTest extends TestCase $container->setDefinition('my-command', $definition); $container->compile(); - - $this->assertSame(array(), $container->getParameter('console.command.ids')); } /** diff --git a/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php b/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php index aaa96fffde..431ea21a67 100644 --- a/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php +++ b/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php @@ -63,7 +63,7 @@ class RegisterListenersPass implements CompilerPassInterface foreach ($container->findTaggedServiceIds($this->listenerTag) as $id => $events) { $def = $container->getDefinition($id); if ($def->isAbstract()) { - continue; + throw new InvalidArgumentException(sprintf('The service "%s" must not be abstract as event listeners are lazy-loaded.', $id)); } foreach ($events as $event) { @@ -90,7 +90,7 @@ class RegisterListenersPass implements CompilerPassInterface foreach ($container->findTaggedServiceIds($this->subscriberTag) as $id => $attributes) { $def = $container->getDefinition($id); if ($def->isAbstract()) { - continue; + throw new InvalidArgumentException(sprintf('The service "%s" must not be abstract as event subscribers are lazy-loaded.', $id)); } // We must assume that the class value has been correctly filled, even if the service is created by a factory diff --git a/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php b/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php index 40821377a5..13810ce433 100644 --- a/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php +++ b/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php @@ -87,6 +87,10 @@ class RegisterListenersPassTest extends TestCase $registerListenersPass->process($builder); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage The service "foo" must not be abstract as event listeners are lazy-loaded. + */ public function testAbstractEventListener() { $container = new ContainerBuilder(); @@ -95,11 +99,13 @@ class RegisterListenersPassTest extends TestCase $registerListenersPass = new RegisterListenersPass(); $registerListenersPass->process($container); - - $this->assertSame(array(), $container->getDefinition('event_dispatcher')->getMethodCalls()); } - public function testAbstractEventSubscriberIsSkipped() + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage The service "foo" must not be abstract as event subscribers are lazy-loaded. + */ + public function testAbstractEventSubscriber() { $container = new ContainerBuilder(); $container->register('foo', 'stdClass')->setAbstract(true)->addTag('kernel.event_subscriber', array()); @@ -107,8 +113,6 @@ class RegisterListenersPassTest extends TestCase $registerListenersPass = new RegisterListenersPass(); $registerListenersPass->process($container); - - $this->assertSame(array(), $container->getDefinition('event_dispatcher')->getMethodCalls()); } public function testEventSubscriberResolvableClassName()