symfony · fabpot · Dec 10, 2020 · Nov 27, 2020
diff --git a/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php b/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php
@@ -41,9 +41,9 @@ public function __construct(string $tag, string $indexAttribute = null, string $
 
         $this->tag = $tag;
         $this->indexAttribute = $indexAttribute;
-        $this->defaultIndexMethod = $defaultIndexMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Name');
+        $this->defaultIndexMethod = $defaultIndexMethod ?: ($indexAttribute ? 'getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute))).'Name' : null);
         $this->needsIndexes = $needsIndexes;
-        $this->defaultPriorityMethod = $defaultPriorityMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Priority');
+        $this->defaultPriorityMethod = $defaultPriorityMethod ?: ($indexAttribute ? 'getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute))).'Priority' : null);
     }
 
     public function getTag()

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php
@@ -46,7 +46,7 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
             $indexAttribute = $tagName->getIndexAttribute();
             $defaultIndexMethod = $tagName->getDefaultIndexMethod();
             $needsIndexes = $tagName->needsIndexes();
-            $defaultPriorityMethod = $tagName->getDefaultPriorityMethod();
+            $defaultPriorityMethod = $tagName->getDefaultPriorityMethod() ?? 'getDefaultPriority';
             $tagName = $tagName->getTag();
         }
 
@@ -69,15 +69,15 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
                 }
                 $priority = $priority ?? $defaultPriority ?? $defaultPriority = 0;
 
-                if (null === $indexAttribute && !$needsIndexes) {
+                if (null === $indexAttribute && !$defaultIndexMethod && !$needsIndexes) {
                     $services[] = [$priority, ++$i, null, $serviceId, null];
                     continue 2;
                 }
 
                 if (null !== $indexAttribute && isset($attribute[$indexAttribute])) {
                     $index = $attribute[$indexAttribute];
-                } elseif (null === $defaultIndex && $defaultIndexMethod && $class) {
-                    $defaultIndex = PriorityTaggedServiceUtil::getDefaultIndex($container, $serviceId, $class, $defaultIndexMethod, $tagName, $indexAttribute);
+                } elseif (null === $defaultIndex && $defaultPriorityMethod && $class) {
+                    $defaultIndex = PriorityTaggedServiceUtil::getDefaultIndex($container, $serviceId, $class, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute);
                 }
                 $index = $index ?? $defaultIndex ?? $defaultIndex = $serviceId;
 
@@ -116,24 +116,31 @@ class PriorityTaggedServiceUtil
     /**
      * Gets the index defined by the default index method.
      */
-    public static function getDefaultIndex(ContainerBuilder $container, string $serviceId, string $class, string $defaultIndexMethod, string $tagName, string $indexAttribute): ?string
+    public static function getDefaultIndex(ContainerBuilder $container, string $serviceId, string $class, string $defaultIndexMethod, string $tagName, ?string $indexAttribute): ?string
     {
         if (!($r = $container->getReflectionClass($class)) || !$r->hasMethod($defaultIndexMethod)) {
             return null;
         }
 
+        if (null !== $indexAttribute) {
+            $service = $class !== $serviceId ? sprintf('service "%s"', $serviceId) : 'on the corresponding service';
+            $message = [sprintf('Either method "%s::%s()" should ', $class, $defaultIndexMethod), sprintf(' or tag "%s" on %s is missing attribute "%s".', $tagName, $service, $indexAttribute)];
+        } else {
+            $message = [sprintf('Method "%s::%s()" should ', $class, $defaultIndexMethod), '.'];
+        }
+
         if (!($rm = $r->getMethod($defaultIndexMethod))->isStatic()) {
-            throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should be static or tag "%s" on service "%s" is missing attribute "%s".', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute));
+            throw new InvalidArgumentException(implode('be static', $message));
         }
 
         if (!$rm->isPublic()) {
-            throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should be public or tag "%s" on service "%s" is missing attribute "%s".', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute));
+            throw new InvalidArgumentException(implode('be public', $message));
         }
 
         $defaultIndex = $rm->invoke(null);
 
         if (!\is_string($defaultIndex)) {
-            throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should return a string (got "%s") or tag "%s" on service "%s" is missing attribute "%s".', $class, $defaultIndexMethod, \gettype($defaultIndex), $tagName, $serviceId, $indexAttribute));
+            throw new InvalidArgumentException(implode(sprintf('return a string (got "%s")', \gettype($defaultIndex)), $message));
         }
 
         return $defaultIndex;

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/PriorityTaggedServiceTraitTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/PriorityTaggedServiceTraitTest.php
@@ -15,8 +15,11 @@
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\BarTagClass;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\FooTagClass;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\FooTaggedForInvalidDefaultMethodClass;
 use Symfony\Component\DependencyInjection\TypedReference;
 
 class PriorityTaggedServiceTraitTest extends TestCase
@@ -132,6 +135,55 @@ public function testOnlyTheIndexedTagsAreListed()
         $this->assertSame(array_keys($expected), array_keys($services));
         $this->assertEquals($expected, $priorityTaggedServiceTraitImplementation->test($tag, $container));
     }
+
+    public function testTheIndexedTagsByDefaultIndexMethod()
+    {
+        $container = new ContainerBuilder();
+        $container->register('service1', FooTagClass::class)->addTag('my_custom_tag');
+
+        $definition = $container->register('service2', BarTagClass::class);
+        $definition->addTag('my_custom_tag', ['priority' => 100]);
+        $definition->addTag('my_custom_tag', []);
+
+        $priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation();
+
+        $tag = new TaggedIteratorArgument('my_custom_tag', 'foo', 'getFooBar');
+        $expected = [
+            'bar_tab_class_with_defaultmethod' => new TypedReference('service2', BarTagClass::class),
+            'service1' => new TypedReference('service1', FooTagClass::class),
+        ];
+        $services = $priorityTaggedServiceTraitImplementation->test($tag, $container);
+        $this->assertSame(array_keys($expected), array_keys($services));
+        $this->assertEquals($expected, $priorityTaggedServiceTraitImplementation->test($tag, $container));
+    }
+
+    /**
+     * @dataProvider provideInvalidDefaultMethods
+     */
+    public function testTheIndexedTagsByDefaultIndexMethodFailure(string $defaultIndexMethod, ?string $indexAttribute, string $expectedExceptionMessage)
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage($expectedExceptionMessage);
+
+        $container = new ContainerBuilder();
+
+        $container->register('service1', FooTaggedForInvalidDefaultMethodClass::class)->addTag('my_custom_tag');
+
+        $priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation();
+
+        $tag = new TaggedIteratorArgument('my_custom_tag', $indexAttribute, $defaultIndexMethod);
+        $priorityTaggedServiceTraitImplementation->test($tag, $container);
+    }
+
+    public function provideInvalidDefaultMethods(): iterable
+    {
+        yield ['getMethodShouldBeStatic', null, sprintf('Method "%s::getMethodShouldBeStatic()" should be static.', FooTaggedForInvalidDefaultMethodClass::class)];
+        yield ['getMethodShouldBeStatic', 'foo', sprintf('Either method "%s::getMethodShouldBeStatic()" should be static or tag "my_custom_tag" on service "service1" is missing attribute "foo".', FooTaggedForInvalidDefaultMethodClass::class)];
+        yield ['getMethodShouldBePublicInsteadProtected', null, sprintf('Method "%s::getMethodShouldBePublicInsteadProtected()" should be public.', FooTaggedForInvalidDefaultMethodClass::class)];
+        yield ['getMethodShouldBePublicInsteadProtected', 'foo', sprintf('Either method "%s::getMethodShouldBePublicInsteadProtected()" should be public or tag "my_custom_tag" on service "service1" is missing attribute "foo".', FooTaggedForInvalidDefaultMethodClass::class)];
+        yield ['getMethodShouldBePublicInsteadPrivate', null, sprintf('Method "%s::getMethodShouldBePublicInsteadPrivate()" should be public.', FooTaggedForInvalidDefaultMethodClass::class)];
+        yield ['getMethodShouldBePublicInsteadPrivate', 'foo', sprintf('Either method "%s::getMethodShouldBePublicInsteadPrivate()" should be public or tag "my_custom_tag" on service "service1" is missing attribute "foo".', FooTaggedForInvalidDefaultMethodClass::class)];
+    }
 }
 
 class PriorityTaggedServiceTraitImplementation

diff --git a/...ny/Component/DependencyInjection/Tests/Fixtures/FooTaggedForInvalidDefaultMethodClass.php b/...ny/Component/DependencyInjection/Tests/Fixtures/FooTaggedForInvalidDefaultMethodClass.php
@@ -0,0 +1,21 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+class FooTaggedForInvalidDefaultMethodClass
+{
+    public function getMethodShouldBeStatic()
+    {
+        return 'anonymous_foo_class_with_default_method';
+    }
+
+    protected static function getMethodShouldBePublicInsteadProtected()
+    {
+        return 'anonymous_foo_class_with_default_method';
+    }
+
+    private static function getMethodShouldBePublicInsteadPrivate()
+    {
+        return 'anonymous_foo_class_with_default_method';
+    }
+}