From cd06c1297b9eaae8ff22e75778f360f4e4b4a8a3 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 13 Apr 2017 10:36:10 +0200 Subject: [PATCH 1/2] 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() From 388e4b3389f187d31781603534f8bdeb771d6c6f Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 12 Apr 2017 11:13:52 +0200 Subject: [PATCH 2/2] [DI] Make tagged abstract services throw earlier --- ...gisterEventListenersAndSubscribersPass.php | 12 ++-------- .../Compiler/AddCacheClearerPass.php | 2 +- .../AddExpressionLanguageProvidersPass.php | 4 ++-- .../Compiler/ProfilerPass.php | 2 +- .../Compiler/TemplatingPass.php | 2 +- .../Compiler/TranslationDumperPass.php | 2 +- .../Compiler/TranslationExtractorPass.php | 2 +- .../Compiler/TranslatorPass.php | 2 +- .../Compiler/ValidateWorkflowsPass.php | 2 +- .../AddConstraintValidatorsPassTest.php | 21 +++++++++++++++--- .../Compiler/RuntimeLoaderPass.php | 7 +----- .../Compiler/TwigEnvironmentPass.php | 2 +- .../Compiler/TwigLoaderPass.php | 2 +- .../AddConsoleCommandPass.php | 7 +----- .../Compiler/PriorityTaggedServiceTrait.php | 2 +- .../Compiler/ResolveTagsInheritancePass.php | 22 +++++++++++++++++++ .../DependencyInjection/ContainerBuilder.php | 8 +++++-- .../ResolveTagsInheritancePassTest.php | 4 ++-- .../RegisterListenersPass.php | 10 ++------- .../RegisterListenersPassTest.php | 4 ++-- .../Form/DependencyInjection/FormPass.php | 7 +++--- .../FragmentRendererPass.php | 6 +---- ...RegisterControllerArgumentLocatorsPass.php | 6 +---- .../RoutingResolverPass.php | 2 +- .../AddConstraintValidatorsPass.php | 6 +---- .../AddValidatorInitializersPass.php | 6 +---- .../AddConstraintValidatorsPassTest.php | 21 +++++++++++++++--- 27 files changed, 94 insertions(+), 79 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php b/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php index 6d53d88aaf..cac2100794 100644 --- a/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php +++ b/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php @@ -55,8 +55,8 @@ class RegisterEventListenersAndSubscribersPass implements CompilerPassInterface return; } - $taggedSubscribers = $container->findTaggedServiceIds($this->tagPrefix.'.event_subscriber'); - $taggedListeners = $container->findTaggedServiceIds($this->tagPrefix.'.event_listener'); + $taggedSubscribers = $container->findTaggedServiceIds($this->tagPrefix.'.event_subscriber', true); + $taggedListeners = $container->findTaggedServiceIds($this->tagPrefix.'.event_listener', true); if (empty($taggedSubscribers) && empty($taggedListeners)) { return; @@ -78,10 +78,6 @@ class RegisterEventListenersAndSubscribersPass implements CompilerPassInterface uasort($subscribers, $sortFunc); foreach ($subscribers as $id => $instance) { - if ($container->getDefinition($id)->isAbstract()) { - throw new InvalidArgumentException(sprintf('The abstract service "%s" cannot be tagged as a doctrine event subscriber.', $id)); - } - $em->addMethodCall('addEventSubscriber', array(new Reference($id))); } } @@ -94,10 +90,6 @@ class RegisterEventListenersAndSubscribersPass implements CompilerPassInterface uasort($listeners, $sortFunc); foreach ($listeners as $id => $instance) { - if ($container->getDefinition($id)->isAbstract()) { - throw new InvalidArgumentException(sprintf('The abstract service "%s" cannot be tagged as a doctrine event listener.', $id)); - } - $em->addMethodCall('addEventListener', array( array_unique($instance['event']), isset($instance['lazy']) && $instance['lazy'] ? $id : new Reference($id), diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddCacheClearerPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddCacheClearerPass.php index 27b9b465c9..88cfffd04f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddCacheClearerPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddCacheClearerPass.php @@ -32,7 +32,7 @@ class AddCacheClearerPass implements CompilerPassInterface } $clearers = array(); - foreach ($container->findTaggedServiceIds('kernel.cache_clearer') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('kernel.cache_clearer', true) as $id => $attributes) { $clearers[] = new Reference($id); } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php index 6510d02c83..d4103d8dff 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php @@ -30,7 +30,7 @@ class AddExpressionLanguageProvidersPass implements CompilerPassInterface // routing if ($container->has('router')) { $definition = $container->findDefinition('router'); - foreach ($container->findTaggedServiceIds('routing.expression_language_provider') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('routing.expression_language_provider', true) as $id => $attributes) { $definition->addMethodCall('addExpressionLanguageProvider', array(new Reference($id))); } } @@ -38,7 +38,7 @@ class AddExpressionLanguageProvidersPass implements CompilerPassInterface // security if ($container->has('security.access.expression_voter')) { $definition = $container->findDefinition('security.access.expression_voter'); - foreach ($container->findTaggedServiceIds('security.expression_language_provider') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('security.expression_language_provider', true) as $id => $attributes) { $definition->addMethodCall('addExpressionLanguageProvider', array(new Reference($id))); } } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ProfilerPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ProfilerPass.php index 046407351c..28cc83dfd3 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ProfilerPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ProfilerPass.php @@ -33,7 +33,7 @@ class ProfilerPass implements CompilerPassInterface $collectors = new \SplPriorityQueue(); $order = PHP_INT_MAX; - foreach ($container->findTaggedServiceIds('data_collector') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('data_collector', true) as $id => $attributes) { $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; $template = null; diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TemplatingPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TemplatingPass.php index 722a014373..8e2e4d0dbf 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TemplatingPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TemplatingPass.php @@ -32,7 +32,7 @@ class TemplatingPass implements CompilerPassInterface if ($container->hasDefinition('templating.engine.php')) { $helpers = array(); - foreach ($container->findTaggedServiceIds('templating.helper') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('templating.helper', true) as $id => $attributes) { if (isset($attributes[0]['alias'])) { $helpers[$attributes[0]['alias']] = $id; } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationDumperPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationDumperPass.php index f7c09748b4..541b06b31e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationDumperPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationDumperPass.php @@ -28,7 +28,7 @@ class TranslationDumperPass implements CompilerPassInterface $definition = $container->getDefinition('translation.writer'); - foreach ($container->findTaggedServiceIds('translation.dumper') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('translation.dumper', true) as $id => $attributes) { $definition->addMethodCall('addDumper', array($attributes[0]['alias'], new Reference($id))); } } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationExtractorPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationExtractorPass.php index fc8a66d24f..4dc8d3985c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationExtractorPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationExtractorPass.php @@ -29,7 +29,7 @@ class TranslationExtractorPass implements CompilerPassInterface $definition = $container->getDefinition('translation.extractor'); - foreach ($container->findTaggedServiceIds('translation.extractor') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('translation.extractor', true) as $id => $attributes) { if (!isset($attributes[0]['alias'])) { throw new RuntimeException(sprintf('The alias for the tag "translation.extractor" of service "%s" must be set.', $id)); } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslatorPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslatorPass.php index 3e2d0e24c4..29e251f274 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslatorPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslatorPass.php @@ -26,7 +26,7 @@ class TranslatorPass implements CompilerPassInterface $loaders = array(); $loaderRefs = array(); - foreach ($container->findTaggedServiceIds('translation.loader') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('translation.loader', true) as $id => $attributes) { $loaderRefs[$id] = new Reference($id); $loaders[$id][] = $attributes[0]['alias']; if (isset($attributes[0]['legacy-alias'])) { diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php index a2f6959929..8034e441e6 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php @@ -25,7 +25,7 @@ class ValidateWorkflowsPass implements CompilerPassInterface { public function process(ContainerBuilder $container) { - $taggedServices = $container->findTaggedServiceIds('workflow.definition'); + $taggedServices = $container->findTaggedServiceIds('workflow.definition', true); foreach ($taggedServices as $id => $tags) { $definition = $container->get($id); foreach ($tags as $tag) { diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConstraintValidatorsPassTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConstraintValidatorsPassTest.php index ac0a4e0cab..d9065e46d5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConstraintValidatorsPassTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConstraintValidatorsPassTest.php @@ -34,9 +34,6 @@ class AddConstraintValidatorsPassTest extends TestCase ->addTag('validator.constraint_validator', array('alias' => 'my_constraint_validator_alias1')); $container->register('my_constraint_validator_service2', Validator2::class) ->addTag('validator.constraint_validator'); - $container->register('my_abstract_constraint_validator') - ->setAbstract(true) - ->addTag('validator.constraint_validator'); $addConstraintValidatorsPass = new AddConstraintValidatorsPass(); $addConstraintValidatorsPass->process($container); @@ -49,6 +46,24 @@ class AddConstraintValidatorsPassTest extends TestCase $this->assertEquals($expected, $container->getDefinition((string) $validatorFactory->getArgument(0))); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage The service "my_abstract_constraint_validator" tagged "validator.constraint_validator" must not be abstract. + */ + public function testAbstractConstraintValidator() + { + $container = new ContainerBuilder(); + $validatorFactory = $container->register('validator.validator_factory') + ->addArgument(array()); + + $container->register('my_abstract_constraint_validator') + ->setAbstract(true) + ->addTag('validator.constraint_validator'); + + $addConstraintValidatorsPass = new AddConstraintValidatorsPass(); + $addConstraintValidatorsPass->process($container); + } + public function testThatCompilerPassIsIgnoredIfThereIsNoConstraintValidatorFactoryDefinition() { $definition = $this->getMockBuilder('Symfony\Component\DependencyInjection\Definition')->getMock(); diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/RuntimeLoaderPass.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/RuntimeLoaderPass.php index 20fda9e162..a4defc51ff 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/RuntimeLoaderPass.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/RuntimeLoaderPass.php @@ -29,13 +29,8 @@ class RuntimeLoaderPass implements CompilerPassInterface $definition = $container->getDefinition('twig.runtime_loader'); $mapping = array(); - foreach ($container->findTaggedServiceIds('twig.runtime') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('twig.runtime', true) as $id => $attributes) { $def = $container->getDefinition($id); - - if ($def->isAbstract()) { - continue; - } - $mapping[$def->getClass()] = new Reference($id); } diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/TwigEnvironmentPass.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/TwigEnvironmentPass.php index 927e4b46e1..f520ab11f0 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/TwigEnvironmentPass.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/TwigEnvironmentPass.php @@ -36,7 +36,7 @@ class TwigEnvironmentPass implements CompilerPassInterface // be registered. $calls = $definition->getMethodCalls(); $definition->setMethodCalls(array()); - foreach ($container->findTaggedServiceIds('twig.extension') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('twig.extension', true) as $id => $attributes) { $definition->addMethodCall('addExtension', array(new Reference($id))); } $definition->setMethodCalls(array_merge($definition->getMethodCalls(), $calls)); diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/TwigLoaderPass.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/TwigLoaderPass.php index fc670e7cd1..f2c1d32eec 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/TwigLoaderPass.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/TwigLoaderPass.php @@ -32,7 +32,7 @@ class TwigLoaderPass implements CompilerPassInterface $prioritizedLoaders = array(); $found = 0; - foreach ($container->findTaggedServiceIds('twig.loader') as $id => $attributes) { + foreach ($container->findTaggedServiceIds('twig.loader', true) as $id => $attributes) { $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; $prioritizedLoaders[$priority][] = $id; ++$found; diff --git a/src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php b/src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php index c455229475..c663910bda 100644 --- a/src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php +++ b/src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php @@ -25,16 +25,11 @@ class AddConsoleCommandPass implements CompilerPassInterface { public function process(ContainerBuilder $container) { - $commandServices = $container->findTaggedServiceIds('console.command'); + $commandServices = $container->findTaggedServiceIds('console.command', true); $serviceIds = array(); foreach ($commandServices as $id => $tags) { $definition = $container->getDefinition($id); - - if ($definition->isAbstract()) { - throw new \InvalidArgumentException(sprintf('The service "%s" tagged "console.command" must not be abstract.', $id)); - } - $class = $container->getParameterBag()->resolveValue($definition->getClass()); if (!$r = $container->getReflectionClass($class)) { diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php index 64c041e93f..60829884c4 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php @@ -40,7 +40,7 @@ trait PriorityTaggedServiceTrait { $services = array(); - foreach ($container->findTaggedServiceIds($tagName) as $serviceId => $attributes) { + foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $attributes) { $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; $services[$priority][] = new Reference($serviceId); } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveTagsInheritancePass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveTagsInheritancePass.php index dbb0aab261..f95e6bbd53 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveTagsInheritancePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveTagsInheritancePass.php @@ -12,6 +12,7 @@ namespace Symfony\Component\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\ChildDefinition; +use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Exception\RuntimeException; /** @@ -21,6 +22,24 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException; */ class ResolveTagsInheritancePass extends AbstractRecursivePass { + private $abstractInheritedParents = array(); + + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container) + { + try { + parent::process($container); + + foreach ($this->abstractInheritedParents as $id) { + $container->findDefinition($id)->setTags(array()); + } + } finally { + $this->abstractInheritedParents = array(); + } + } + /** * {@inheritdoc} */ @@ -36,6 +55,9 @@ class ResolveTagsInheritancePass extends AbstractRecursivePass } $parentDef = $this->container->findDefinition($parent); + if ($parentDef->isAbstract()) { + $this->abstractInheritedParents[$parent] = $parent; + } if ($parentDef instanceof ChildDefinition) { $this->processValue($parentDef); diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 63b2bc8662..86099458f8 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -1200,16 +1200,20 @@ class ContainerBuilder extends Container implements TaggedContainerInterface * } * } * - * @param string $name The tag name + * @param string $name + * @param bool $throwOnAbstract * * @return array An array of tags with the tagged service as key, holding a list of attribute arrays */ - public function findTaggedServiceIds($name) + public function findTaggedServiceIds($name, $throwOnAbstract = false) { $this->usedTags[] = $name; $tags = array(); foreach ($this->getDefinitions() as $id => $definition) { if ($definition->hasTag($name)) { + if ($throwOnAbstract && $definition->isAbstract()) { + throw new InvalidArgumentException(sprintf('The service "%s" tagged "%s" must not be abstract.', $id, $name)); + } $tags[$id] = $definition->getTag($name); } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveTagsInheritancePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveTagsInheritancePassTest.php index 532bc86030..3ed8e852af 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveTagsInheritancePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveTagsInheritancePassTest.php @@ -22,13 +22,13 @@ class ResolveTagsInheritancePassTest extends TestCase { $container = new ContainerBuilder(); $container->register('grandpa', self::class)->addTag('g'); - $container->setDefinition('parent', new ChildDefinition('grandpa'))->addTag('p')->setInheritTags(true); + $container->setDefinition('parent', new ChildDefinition('grandpa'))->addTag('p')->setInheritTags(true)->setAbstract(true); $container->setDefinition('child', new ChildDefinition('parent'))->setInheritTags(true); (new ResolveTagsInheritancePass())->process($container); $expected = array('p' => array(array()), 'g' => array(array())); - $this->assertSame($expected, $container->getDefinition('parent')->getTags()); $this->assertSame($expected, $container->getDefinition('child')->getTags()); + $this->assertSame(array(), $container->getDefinition('parent')->getTags()); } } diff --git a/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php b/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php index 431ea21a67..8b2df3ea0f 100644 --- a/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php +++ b/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php @@ -60,11 +60,8 @@ class RegisterListenersPass implements CompilerPassInterface $definition = $container->findDefinition($this->dispatcherService); - foreach ($container->findTaggedServiceIds($this->listenerTag) as $id => $events) { + foreach ($container->findTaggedServiceIds($this->listenerTag, true) as $id => $events) { $def = $container->getDefinition($id); - if ($def->isAbstract()) { - throw new InvalidArgumentException(sprintf('The service "%s" must not be abstract as event listeners are lazy-loaded.', $id)); - } foreach ($events as $event) { $priority = isset($event['priority']) ? $event['priority'] : 0; @@ -87,11 +84,8 @@ class RegisterListenersPass implements CompilerPassInterface $extractingDispatcher = new ExtractingEventDispatcher(); - foreach ($container->findTaggedServiceIds($this->subscriberTag) as $id => $attributes) { + foreach ($container->findTaggedServiceIds($this->subscriberTag, true) as $id => $attributes) { $def = $container->getDefinition($id); - if ($def->isAbstract()) { - 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 $class = $container->getParameterBag()->resolveValue($def->getClass()); diff --git a/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php b/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php index 13810ce433..f583cd04cf 100644 --- a/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php +++ b/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php @@ -89,7 +89,7 @@ class RegisterListenersPassTest extends TestCase /** * @expectedException \InvalidArgumentException - * @expectedExceptionMessage The service "foo" must not be abstract as event listeners are lazy-loaded. + * @expectedExceptionMessage The service "foo" tagged "kernel.event_listener" must not be abstract. */ public function testAbstractEventListener() { @@ -103,7 +103,7 @@ class RegisterListenersPassTest extends TestCase /** * @expectedException \InvalidArgumentException - * @expectedExceptionMessage The service "foo" must not be abstract as event subscribers are lazy-loaded. + * @expectedExceptionMessage The service "foo" tagged "kernel.event_subscriber" must not be abstract. */ public function testAbstractEventSubscriber() { diff --git a/src/Symfony/Component/Form/DependencyInjection/FormPass.php b/src/Symfony/Component/Form/DependencyInjection/FormPass.php index 9952210ced..464035607c 100644 --- a/src/Symfony/Component/Form/DependencyInjection/FormPass.php +++ b/src/Symfony/Component/Form/DependencyInjection/FormPass.php @@ -61,10 +61,9 @@ class FormPass implements CompilerPassInterface $servicesMap = array(); // Builds an array with fully-qualified type class names as keys and service IDs as values - foreach ($container->findTaggedServiceIds($this->formTypeTag) as $serviceId => $tag) { - $serviceDefinition = $container->getDefinition($serviceId); - + foreach ($container->findTaggedServiceIds($this->formTypeTag, true) as $serviceId => $tag) { // Add form type service to the service locator + $serviceDefinition = $container->getDefinition($serviceId); $servicesMap[$serviceDefinition->getClass()] = new Reference($serviceId); } @@ -98,7 +97,7 @@ class FormPass implements CompilerPassInterface private function processFormTypeGuessers(ContainerBuilder $container) { $guessers = array(); - foreach ($container->findTaggedServiceIds($this->formTypeGuesserTag) as $serviceId => $tags) { + foreach ($container->findTaggedServiceIds($this->formTypeGuesserTag, true) as $serviceId => $tags) { $guessers[] = new Reference($serviceId); } diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/FragmentRendererPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/FragmentRendererPass.php index d3c8cb3092..ac52c8732b 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/FragmentRendererPass.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/FragmentRendererPass.php @@ -46,12 +46,8 @@ class FragmentRendererPass implements CompilerPassInterface $definition = $container->getDefinition($this->handlerService); $renderers = array(); - foreach ($container->findTaggedServiceIds($this->rendererTag) as $id => $tags) { + foreach ($container->findTaggedServiceIds($this->rendererTag, true) as $id => $tags) { $def = $container->getDefinition($id); - if ($def->isAbstract()) { - continue; - } - $class = $container->getParameterBag()->resolveValue($def->getClass()); if (!$r = $container->getReflectionClass($class)) { diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php index 52a93eb56d..268d363d83 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php @@ -48,12 +48,8 @@ class RegisterControllerArgumentLocatorsPass implements CompilerPassInterface $parameterBag = $container->getParameterBag(); $controllers = array(); - foreach ($container->findTaggedServiceIds($this->controllerTag) as $id => $tags) { + foreach ($container->findTaggedServiceIds($this->controllerTag, true) as $id => $tags) { $def = $container->getDefinition($id); - - if ($def->isAbstract()) { - continue; - } $class = $def->getClass(); $autowire = $def->isAutowired(); diff --git a/src/Symfony/Component/Routing/DependencyInjection/RoutingResolverPass.php b/src/Symfony/Component/Routing/DependencyInjection/RoutingResolverPass.php index 5a31471f6b..73a8f8511d 100644 --- a/src/Symfony/Component/Routing/DependencyInjection/RoutingResolverPass.php +++ b/src/Symfony/Component/Routing/DependencyInjection/RoutingResolverPass.php @@ -39,7 +39,7 @@ class RoutingResolverPass implements CompilerPassInterface $definition = $container->getDefinition($this->resolverServiceId); - foreach ($container->findTaggedServiceIds($this->loaderTag) as $id => $attributes) { + foreach ($container->findTaggedServiceIds($this->loaderTag, true) as $id => $attributes) { $definition->addMethodCall('addLoader', array(new Reference($id))); } } diff --git a/src/Symfony/Component/Validator/DependencyInjection/AddConstraintValidatorsPass.php b/src/Symfony/Component/Validator/DependencyInjection/AddConstraintValidatorsPass.php index d50368906e..ec68bc0a8c 100644 --- a/src/Symfony/Component/Validator/DependencyInjection/AddConstraintValidatorsPass.php +++ b/src/Symfony/Component/Validator/DependencyInjection/AddConstraintValidatorsPass.php @@ -38,13 +38,9 @@ class AddConstraintValidatorsPass implements CompilerPassInterface } $validators = array(); - foreach ($container->findTaggedServiceIds($this->constraintValidatorTag) as $id => $attributes) { + foreach ($container->findTaggedServiceIds($this->constraintValidatorTag, true) as $id => $attributes) { $definition = $container->getDefinition($id); - if ($definition->isAbstract()) { - continue; - } - if (isset($attributes[0]['alias'])) { $validators[$attributes[0]['alias']] = new Reference($id); } diff --git a/src/Symfony/Component/Validator/DependencyInjection/AddValidatorInitializersPass.php b/src/Symfony/Component/Validator/DependencyInjection/AddValidatorInitializersPass.php index 627b410ce0..52be28534b 100644 --- a/src/Symfony/Component/Validator/DependencyInjection/AddValidatorInitializersPass.php +++ b/src/Symfony/Component/Validator/DependencyInjection/AddValidatorInitializersPass.php @@ -37,11 +37,7 @@ class AddValidatorInitializersPass implements CompilerPassInterface } $initializers = array(); - foreach ($container->findTaggedServiceIds($this->initializerTag) as $id => $attributes) { - if ($container->getDefinition($id)->isAbstract()) { - continue; - } - + foreach ($container->findTaggedServiceIds($this->initializerTag, true) as $id => $attributes) { $initializers[] = new Reference($id); } diff --git a/src/Symfony/Component/Validator/Tests/DependencyInjection/AddConstraintValidatorsPassTest.php b/src/Symfony/Component/Validator/Tests/DependencyInjection/AddConstraintValidatorsPassTest.php index 00f6c275dc..ba4ea36b2b 100644 --- a/src/Symfony/Component/Validator/Tests/DependencyInjection/AddConstraintValidatorsPassTest.php +++ b/src/Symfony/Component/Validator/Tests/DependencyInjection/AddConstraintValidatorsPassTest.php @@ -31,9 +31,6 @@ class AddConstraintValidatorsPassTest extends TestCase ->addTag('validator.constraint_validator', array('alias' => 'my_constraint_validator_alias1')); $container->register('my_constraint_validator_service2', Validator2::class) ->addTag('validator.constraint_validator'); - $container->register('my_abstract_constraint_validator') - ->setAbstract(true) - ->addTag('validator.constraint_validator'); $addConstraintValidatorsPass = new AddConstraintValidatorsPass(); $addConstraintValidatorsPass->process($container); @@ -46,6 +43,24 @@ class AddConstraintValidatorsPassTest extends TestCase $this->assertEquals($expected, $container->getDefinition((string) $validatorFactory->getArgument(0))); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage The service "my_abstract_constraint_validator" tagged "validator.constraint_validator" must not be abstract. + */ + public function testAbstractConstraintValidator() + { + $container = new ContainerBuilder(); + $validatorFactory = $container->register('validator.validator_factory') + ->addArgument(array()); + + $container->register('my_abstract_constraint_validator') + ->setAbstract(true) + ->addTag('validator.constraint_validator'); + + $addConstraintValidatorsPass = new AddConstraintValidatorsPass(); + $addConstraintValidatorsPass->process($container); + } + public function testThatCompilerPassIsIgnoredIfThereIsNoConstraintValidatorFactoryDefinition() { $definition = $this->getMockBuilder('Symfony\Component\DependencyInjection\Definition')->getMock();