bug #31463 [DI] default to service id - *not* FQCN - when building ta…

…gged locators (nicolas-grekas)

This PR was merged into the 4.3 branch.

Discussion
----------

[DI] default to service id - *not* FQCN - when building tagged locators

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

While reviewing #30926 I realized that defaulting to the FQCN is a shortcut that isn't useful enough.
Defaulting to the service id provides the same experience in practice because service ids are FQCN by default.
But when they aren't, the service id is the proper index to default to when building the locator.

Commits
-------

52e827c [DI] default to service id - *not* FQCN - when building tagged locators

src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php

@@ -21,22 +21,26 @@ class TaggedIteratorArgument extends IteratorArgument
     private $tag;
     private $indexAttribute;
     private $defaultIndexMethod;
-    private $useFqcnAsFallback = false;
+    private $needsIndexes = false;
 
     /**
      * @param string      $tag                The name of the tag identifying the target services
      * @param string|null $indexAttribute     The name of the attribute that defines the key referencing each service in the tagged collection
      * @param string|null $defaultIndexMethod The static method that should be called to get each service's key when their tag doesn't define the previous attribute
-     * @param bool        $useFqcnAsFallback  Whether the FQCN of the service should be used as index when neither the attribute nor the method are defined
+     * @param bool        $needsIndexes       Whether indexes are required and should be generated when computing the map
      */
-    public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $useFqcnAsFallback = false)
+    public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false)
     {
         parent::__construct([]);
 
+        if (null === $indexAttribute && $needsIndexes) {
+            $indexAttribute = preg_match('/[^.]++$/', $tag, $m) ? $m[0] : $tag;
+        }
+
         $this->tag = $tag;
         $this->indexAttribute = $indexAttribute;
         $this->defaultIndexMethod = $defaultIndexMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Name');
-        $this->useFqcnAsFallback = $useFqcnAsFallback;
+        $this->needsIndexes = $needsIndexes;
     }
 
     public function getTag()
@@ -54,8 +58,8 @@ public function getDefaultIndexMethod(): ?string
         return $this->defaultIndexMethod;
     }
 
-    public function useFqcnAsFallback(): bool
+    public function needsIndexes(): bool
     {
-        return $this->useFqcnAsFallback;
+        return $this->needsIndexes;
     }
 }

src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php

@@ -41,12 +41,12 @@ trait PriorityTaggedServiceTrait
      */
     private function findAndSortTaggedServices($tagName, ContainerBuilder $container)
     {
-        $indexAttribute = $defaultIndexMethod = $useFqcnAsFallback = null;
+        $indexAttribute = $defaultIndexMethod = $needsIndexes = null;
 
         if ($tagName instanceof TaggedIteratorArgument) {
             $indexAttribute = $tagName->getIndexAttribute();
             $defaultIndexMethod = $tagName->getDefaultIndexMethod();
-            $useFqcnAsFallback = $tagName->useFqcnAsFallback();
+            $needsIndexes = $tagName->needsIndexes();
             $tagName = $tagName->getTag();
         }
 
@@ -55,7 +55,7 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
         foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $attributes) {
             $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
 
-            if (null === $indexAttribute && !$useFqcnAsFallback) {
+            if (null === $indexAttribute && !$needsIndexes) {
                 $services[$priority][] = new Reference($serviceId);
 
                 continue;
@@ -77,8 +77,8 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
             $class = $r->name;
 
             if (!$r->hasMethod($defaultIndexMethod)) {
-                if ($useFqcnAsFallback) {
-                    $services[$priority][$class] = new TypedReference($serviceId, $class);
+                if ($needsIndexes) {
+                    $services[$priority][$serviceId] = new TypedReference($serviceId, $class);
 
                     continue;
                 }

src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php

@@ -292,22 +292,22 @@ public function testTaggedServiceWithIndexAttributeAndDefaultMethod()
     public function testTaggedServiceLocatorWithIndexAttribute()
     {
         $container = new ContainerBuilder();
-        $container->register(BarTagClass::class)
+        $container->register('bar_tag', BarTagClass::class)
             ->setPublic(true)
             ->addTag('foo_bar', ['foo' => 'bar'])
         ;
-        $container->register(FooTagClass::class)
+        $container->register('foo_tag', FooTagClass::class)
             ->setPublic(true)
             ->addTag('foo_bar')
         ;
-        $container->register(FooBarTaggedClass::class)
+        $container->register('foo_bar_tagged', FooBarTaggedClass::class)
             ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo')))
             ->setPublic(true)
         ;
 
         $container->compile();
 
-        $s = $container->get(FooBarTaggedClass::class);
+        $s = $container->get('foo_bar_tagged');
 
         /** @var ServiceLocator $serviceLocator */
         $serviceLocator = $s->getParam();
@@ -317,28 +317,28 @@ public function testTaggedServiceLocatorWithIndexAttribute()
             'bar' => $serviceLocator->get('bar'),
             'foo_tag_class' => $serviceLocator->get('foo_tag_class'),
         ];
-        $this->assertSame(['bar' => $container->get(BarTagClass::class), 'foo_tag_class' => $container->get(FooTagClass::class)], $same);
+        $this->assertSame(['bar' => $container->get('bar_tag'), 'foo_tag_class' => $container->get('foo_tag')], $same);
     }
 
     public function testTaggedServiceLocatorWithIndexAttributeAndDefaultMethod()
     {
         $container = new ContainerBuilder();
-        $container->register(BarTagClass::class)
+        $container->register('bar_tag', BarTagClass::class)
             ->setPublic(true)
             ->addTag('foo_bar')
         ;
-        $container->register(FooTagClass::class)
+        $container->register('foo_tag', FooTagClass::class)
             ->setPublic(true)
             ->addTag('foo_bar', ['foo' => 'foo'])
         ;
-        $container->register(FooBarTaggedClass::class)
+        $container->register('foo_bar_tagged', FooBarTaggedClass::class)
             ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo', 'getFooBar')))
             ->setPublic(true)
         ;
 
         $container->compile();
 
-        $s = $container->get(FooBarTaggedClass::class);
+        $s = $container->get('foo_bar_tagged');
 
         /** @var ServiceLocator $serviceLocator */
         $serviceLocator = $s->getParam();
@@ -348,33 +348,59 @@ public function testTaggedServiceLocatorWithIndexAttributeAndDefaultMethod()
             'bar_tab_class_with_defaultmethod' => $serviceLocator->get('bar_tab_class_with_defaultmethod'),
             'foo' => $serviceLocator->get('foo'),
         ];
-        $this->assertSame(['bar_tab_class_with_defaultmethod' => $container->get(BarTagClass::class), 'foo' => $container->get(FooTagClass::class)], $same);
+        $this->assertSame(['bar_tab_class_with_defaultmethod' => $container->get('bar_tag'), 'foo' => $container->get('foo_tag')], $same);
     }
 
-    public function testTaggedServiceLocatorWithFqcnFallback()
+    public function testTaggedServiceLocatorWithFallback()
     {
         $container = new ContainerBuilder();
-        $container->register(BarTagClass::class)
+        $container->register('bar_tag', BarTagClass::class)
             ->setPublic(true)
             ->addTag('foo_bar')
         ;
-        $container->register(FooBarTaggedClass::class)
+        $container->register('foo_bar_tagged', FooBarTaggedClass::class)
             ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', null, null, true)))
             ->setPublic(true)
         ;
 
         $container->compile();
 
-        $s = $container->get(FooBarTaggedClass::class);
+        $s = $container->get('foo_bar_tagged');
 
         /** @var ServiceLocator $serviceLocator */
         $serviceLocator = $s->getParam();
         $this->assertTrue($s->getParam() instanceof ServiceLocator, sprintf('Wrong instance, should be an instance of ServiceLocator, %s given', \is_object($serviceLocator) ? \get_class($serviceLocator) : \gettype($serviceLocator)));
 
-        $same = [
-            BarTagClass::class => $serviceLocator->get(BarTagClass::class),
+        $expected = [
+            'bar_tag' => $container->get('bar_tag'),
+        ];
+        $this->assertSame($expected, ['bar_tag' => $serviceLocator->get('bar_tag')]);
+    }
+
+    public function testTaggedServiceLocatorWithDefaultIndex()
+    {
+        $container = new ContainerBuilder();
+        $container->register('bar_tag', BarTagClass::class)
+            ->setPublic(true)
+            ->addTag('app.foo_bar', ['foo_bar' => 'baz'])
+        ;
+        $container->register('foo_bar_tagged', FooBarTaggedClass::class)
+            ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('app.foo_bar', null, null, true)))
+            ->setPublic(true)
+        ;
+
+        $container->compile();
+
+        $s = $container->get('foo_bar_tagged');
+
+        /** @var ServiceLocator $serviceLocator */
+        $serviceLocator = $s->getParam();
+        $this->assertTrue($s->getParam() instanceof ServiceLocator, sprintf('Wrong instance, should be an instance of ServiceLocator, %s given', \is_object($serviceLocator) ? \get_class($serviceLocator) : \gettype($serviceLocator)));
+
+        $expected = [
+            'baz' => $container->get('bar_tag'),
         ];
-        $this->assertSame([BarTagClass::class => $container->get(BarTagClass::class)], $same);
+        $this->assertSame($expected, ['baz' => $serviceLocator->get('baz')]);
     }
 }