Skip to content

Commit 4f0daa7

Browse files
committed
feature symfony#22420 [DI] Make tagged abstract services throw earlier (nicolas-grekas)
This PR was squashed before being merged into the 3.3-dev branch (closes symfony#22420). Discussion ---------- [DI] Make tagged abstract services throw earlier | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - As spotted by @stof in symfony#22388 (comment), skipping abstract tagged services removes an opportunity to report config mistakes to users. Instead of skipping them, let's throw as done before (thus reverting symfony#22039, ping @chalasr). I made `$container->findTaggedServiceIds()` accept a 2nd arg to make this more systematic. To keep the possibility to have abstract tagged services *for the purpose of tag inheritance*, `ResolveTagsInheritancePass` now resets their tags. Commits ------- 388e4b3 [DI] Make tagged abstract services throw earlier cd06c12 Revert "minor symfony#22039 Skip abstract definitions in compiler passes (chalasr)"
2 parents 64b715b + 388e4b3 commit 4f0daa7

File tree

30 files changed

+117
-90
lines changed

30 files changed

+117
-90
lines changed

src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ public function process(ContainerBuilder $container)
5555
return;
5656
}
5757

58-
$taggedSubscribers = $container->findTaggedServiceIds($this->tagPrefix.'.event_subscriber');
59-
$taggedListeners = $container->findTaggedServiceIds($this->tagPrefix.'.event_listener');
58+
$taggedSubscribers = $container->findTaggedServiceIds($this->tagPrefix.'.event_subscriber', true);
59+
$taggedListeners = $container->findTaggedServiceIds($this->tagPrefix.'.event_listener', true);
6060

6161
if (empty($taggedSubscribers) && empty($taggedListeners)) {
6262
return;
@@ -78,10 +78,6 @@ public function process(ContainerBuilder $container)
7878

7979
uasort($subscribers, $sortFunc);
8080
foreach ($subscribers as $id => $instance) {
81-
if ($container->getDefinition($id)->isAbstract()) {
82-
continue;
83-
}
84-
8581
$em->addMethodCall('addEventSubscriber', array(new Reference($id)));
8682
}
8783
}
@@ -94,10 +90,6 @@ public function process(ContainerBuilder $container)
9490

9591
uasort($listeners, $sortFunc);
9692
foreach ($listeners as $id => $instance) {
97-
if ($container->getDefinition($id)->isAbstract()) {
98-
continue;
99-
}
100-
10193
$em->addMethodCall('addEventListener', array(
10294
array_unique($instance['event']),
10395
isset($instance['lazy']) && $instance['lazy'] ? $id : new Reference($id),

src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818

1919
class RegisterEventListenersAndSubscribersPassTest extends TestCase
2020
{
21+
/**
22+
* @expectedException \InvalidArgumentException
23+
*/
2124
public function testExceptionOnAbstractTaggedSubscriber()
2225
{
2326
$container = $this->createBuilder();
@@ -29,10 +32,12 @@ public function testExceptionOnAbstractTaggedSubscriber()
2932
$container->setDefinition('a', $abstractDefinition);
3033

3134
$this->process($container);
32-
$this->assertSame(array(), $container->getDefinition('doctrine.dbal.default_connection.event_manager')->getMethodCalls());
3335
}
3436

35-
public function testAbstractTaggedListenerIsSkipped()
37+
/**
38+
* @expectedException \InvalidArgumentException
39+
*/
40+
public function testExceptionOnAbstractTaggedListener()
3641
{
3742
$container = $this->createBuilder();
3843

@@ -43,7 +48,6 @@ public function testAbstractTaggedListenerIsSkipped()
4348
$container->setDefinition('a', $abstractDefinition);
4449

4550
$this->process($container);
46-
$this->assertSame(array(), $container->getDefinition('doctrine.dbal.default_connection.event_manager')->getMethodCalls());
4751
}
4852

4953
public function testProcessEventListenersWithPriorities()

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddCacheClearerPass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public function process(ContainerBuilder $container)
3232
}
3333

3434
$clearers = array();
35-
foreach ($container->findTaggedServiceIds('kernel.cache_clearer') as $id => $attributes) {
35+
foreach ($container->findTaggedServiceIds('kernel.cache_clearer', true) as $id => $attributes) {
3636
$clearers[] = new Reference($id);
3737
}
3838

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@ public function process(ContainerBuilder $container)
3030
// routing
3131
if ($container->has('router')) {
3232
$definition = $container->findDefinition('router');
33-
foreach ($container->findTaggedServiceIds('routing.expression_language_provider') as $id => $attributes) {
33+
foreach ($container->findTaggedServiceIds('routing.expression_language_provider', true) as $id => $attributes) {
3434
$definition->addMethodCall('addExpressionLanguageProvider', array(new Reference($id)));
3535
}
3636
}
3737

3838
// security
3939
if ($container->has('security.access.expression_voter')) {
4040
$definition = $container->findDefinition('security.access.expression_voter');
41-
foreach ($container->findTaggedServiceIds('security.expression_language_provider') as $id => $attributes) {
41+
foreach ($container->findTaggedServiceIds('security.expression_language_provider', true) as $id => $attributes) {
4242
$definition->addMethodCall('addExpressionLanguageProvider', array(new Reference($id)));
4343
}
4444
}

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ProfilerPass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public function process(ContainerBuilder $container)
3333

3434
$collectors = new \SplPriorityQueue();
3535
$order = PHP_INT_MAX;
36-
foreach ($container->findTaggedServiceIds('data_collector') as $id => $attributes) {
36+
foreach ($container->findTaggedServiceIds('data_collector', true) as $id => $attributes) {
3737
$priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
3838
$template = null;
3939

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TemplatingPass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public function process(ContainerBuilder $container)
3232

3333
if ($container->hasDefinition('templating.engine.php')) {
3434
$helpers = array();
35-
foreach ($container->findTaggedServiceIds('templating.helper') as $id => $attributes) {
35+
foreach ($container->findTaggedServiceIds('templating.helper', true) as $id => $attributes) {
3636
if (isset($attributes[0]['alias'])) {
3737
$helpers[$attributes[0]['alias']] = $id;
3838
}

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationDumperPass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public function process(ContainerBuilder $container)
2828

2929
$definition = $container->getDefinition('translation.writer');
3030

31-
foreach ($container->findTaggedServiceIds('translation.dumper') as $id => $attributes) {
31+
foreach ($container->findTaggedServiceIds('translation.dumper', true) as $id => $attributes) {
3232
$definition->addMethodCall('addDumper', array($attributes[0]['alias'], new Reference($id)));
3333
}
3434
}

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationExtractorPass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public function process(ContainerBuilder $container)
2929

3030
$definition = $container->getDefinition('translation.extractor');
3131

32-
foreach ($container->findTaggedServiceIds('translation.extractor') as $id => $attributes) {
32+
foreach ($container->findTaggedServiceIds('translation.extractor', true) as $id => $attributes) {
3333
if (!isset($attributes[0]['alias'])) {
3434
throw new RuntimeException(sprintf('The alias for the tag "translation.extractor" of service "%s" must be set.', $id));
3535
}

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslatorPass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public function process(ContainerBuilder $container)
2626

2727
$loaders = array();
2828
$loaderRefs = array();
29-
foreach ($container->findTaggedServiceIds('translation.loader') as $id => $attributes) {
29+
foreach ($container->findTaggedServiceIds('translation.loader', true) as $id => $attributes) {
3030
$loaderRefs[$id] = new Reference($id);
3131
$loaders[$id][] = $attributes[0]['alias'];
3232
if (isset($attributes[0]['legacy-alias'])) {

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class ValidateWorkflowsPass implements CompilerPassInterface
2525
{
2626
public function process(ContainerBuilder $container)
2727
{
28-
$taggedServices = $container->findTaggedServiceIds('workflow.definition');
28+
$taggedServices = $container->findTaggedServiceIds('workflow.definition', true);
2929
foreach ($taggedServices as $id => $tags) {
3030
$definition = $container->get($id);
3131
foreach ($tags as $tag) {

0 commit comments

Comments
 (0)