symfony · fabpot · Apr 13, 2017 · Apr 13, 2017 · Apr 12, 2017 · Koc
diff --git a/...ge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php b/...ge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php
@@ -55,8 +55,8 @@ public function process(ContainerBuilder $container)
            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 @@ public function process(ContainerBuilder $container)

                uasort($subscribers, $sortFunc);
                foreach ($subscribers as $id => $instance) {
-                    if ($container->getDefinition($id)->isAbstract()) {
-                        continue;
-                    }
-
                    $em->addMethodCall('addEventSubscriber', array(new Reference($id)));
                }
            }
@@ -94,10 +90,6 @@ public function process(ContainerBuilder $container)

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

diff --git a/...e/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php b/...e/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php
@@ -18,6 +18,9 @@

 class RegisterEventListenersAndSubscribersPassTest extends TestCase
 {
+    /**
+     * @expectedException \InvalidArgumentException
+     */
    public function testExceptionOnAbstractTaggedSubscriber()
    {
        $container = $this->createBuilder();
@@ -29,10 +32,12 @@ public function testExceptionOnAbstractTaggedSubscriber()
        $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 @@ public function testAbstractTaggedListenerIsSkipped()
        $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/DependencyInjection/Compiler/AddCacheClearerPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddCacheClearerPass.php
@@ -32,7 +32,7 @@ public function process(ContainerBuilder $container)
        }

        $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/...undle/FrameworkBundle/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php b/...undle/FrameworkBundle/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php
@@ -30,15 +30,15 @@ public function process(ContainerBuilder $container)
        // 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)));
            }
        }

        // 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
@@ -33,7 +33,7 @@ public function process(ContainerBuilder $container)

        $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
@@ -32,7 +32,7 @@ public function process(ContainerBuilder $container)

        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
@@ -28,7 +28,7 @@ public function process(ContainerBuilder $container)

        $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
@@ -29,7 +29,7 @@ public function process(ContainerBuilder $container)

        $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
@@ -26,7 +26,7 @@ public function process(ContainerBuilder $container)

        $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
@@ -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/...y/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConsoleCommandPassTest.php b/...y/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConsoleCommandPassTest.php
@@ -62,6 +62,10 @@ public function visibilityProvider()
        );
    }

+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessage The service "my-command" tagged "console.command" must not be abstract.
+     */
    public function testProcessThrowAnExceptionIfTheServiceIsAbstract()
    {
        $container = new ContainerBuilder();
@@ -73,8 +77,6 @@ public function testProcessThrowAnExceptionIfTheServiceIsAbstract()
        $container->setDefinition('my-command', $definition);

        $container->compile();
-
-        $this->assertSame(array(), $container->getParameter('console.command.ids'));
    }

    /**

diff --git a/...le/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConstraintValidatorsPassTest.php b/...le/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConstraintValidatorsPassTest.php
@@ -34,9 +34,6 @@ public function testThatConstraintValidatorServicesAreProcessed()
            ->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 @@ public function testThatConstraintValidatorServicesAreProcessed()
        $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
@@ -29,13 +29,8 @@ public function process(ContainerBuilder $container)

        $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
@@ -36,7 +36,7 @@ public function process(ContainerBuilder $container)
        // 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
@@ -32,7 +32,7 @@ public function process(ContainerBuilder $container)
        $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
@@ -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()) {
-                continue;
-            }
-
            $class = $container->getParameterBag()->resolveValue($definition->getClass());

            if (!$r = $container->getReflectionClass($class)) {

diff --git a/src/Symfony/Component/Console/Tests/DependencyInjection/AddConsoleCommandPassTest.php b/src/Symfony/Component/Console/Tests/DependencyInjection/AddConsoleCommandPassTest.php
@@ -50,7 +50,11 @@ public function visibilityProvider()
        );
    }

-    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 @@ public function testProcessSkipAbstractDefinitions()
        $container->setDefinition('my-command', $definition);

        $container->compile();
-
-        $this->assertSame(array(), $container->getParameter('console.command.ids'));
    }

    /**

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php
@@ -40,7 +40,7 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
    {
        $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
@@ -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 @@
 */
 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 @@ protected function processValue($value, $isRoot = false)
        }

        $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
@@ -1200,16 +1200,20 @@ public function resolveServices($value)
     *     }
     * }
     *
-     * @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
@@ -22,13 +22,13 @@ public function testProcess()
    {
        $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
@@ -60,11 +60,8 @@ public function process(ContainerBuilder $container)

        $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()) {
-                continue;
-            }

            foreach ($events as $event) {
                $priority = isset($event['priority']) ? $event['priority'] : 0;
@@ -87,11 +84,8 @@ public function process(ContainerBuilder $container)

        $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()) {
-                continue;
-            }

            // 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());
-Original file line number
+Diff line change
@@ -1200,16 +1200,20 @@ public function resolveServices($value)
          *     }
          * }
          *
-         * @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)
             Copy link

  
      
    
  

  
      

  
    Contributor


      

  

  
    
      

      
            Koc
  

      

      

      


        Apr 13, 2017


      
  •

  
    
      
        
          edited
          
        
        
      
    
    
      
  
        
      Loading


  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  
    
  


    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

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

    
      
      


  


  
    
      what about set null for now (and consider it as false), trigger deprecation warning if no argument value provided (null), change it to true by default for 4.0?
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



  
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  Author


  

  
    
      

      
            nicolas-grekas
  

      

      

      


        Apr 13, 2017


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

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

    
      
      


  


  
    
      this is mostly internal, so not sure it's worth it

but please open a PR if you want to work on it
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
         {
             $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);
                 }
             }
-          Expand Down