[DI] Simplify using DI attributes with ServiceLocator/Iterator's

kbond · nicolas-grekas · commit c75b0753c93a · 2023-10-12T14:36:18.000+02:00
diff --git a/Attribute/AutowireIterator.php b/Attribute/AutowireIterator.php
@@ -11,67 +11,25 @@
 
 namespace Symfony\Component\DependencyInjection\Attribute;
 
-use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
-use Symfony\Component\DependencyInjection\ContainerInterface;
-use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
-use Symfony\Component\DependencyInjection\TypedReference;
-use Symfony\Contracts\Service\Attribute\SubscribedService;
-use Symfony\Contracts\Service\ServiceSubscriberInterface;
 
 /**
- * Autowires an iterator of services based on a tag name or an explicit list of key => service-type pairs.
+ * Autowires an iterator of services based on a tag name.
  */
 #[\Attribute(\Attribute::TARGET_PARAMETER)]
 class AutowireIterator extends Autowire
 {
     /**
-     * @see ServiceSubscriberInterface::getSubscribedServices()
-     *
-     * @param string|array<string|SubscribedService> $services A tag name or an explicit list of services
-     * @param string|string[]                        $exclude  A service or a list of services to exclude
+     * @param string|string[] $exclude A service or a list of services to exclude
      */
     public function __construct(
-        string|array $services,
+        string $tag,
         string $indexAttribute = null,
         string $defaultIndexMethod = null,
         string $defaultPriorityMethod = null,
         string|array $exclude = [],
         bool $excludeSelf = true,
     ) {
-        if (\is_string($services)) {
-            parent::__construct(new TaggedIteratorArgument($services, $indexAttribute, $defaultIndexMethod, false, $defaultPriorityMethod, (array) $exclude, $excludeSelf));
-
-            return;
-        }
-
-        $references = [];
-
-        foreach ($services as $key => $type) {
-            $attributes = [];
-
-            if ($type instanceof SubscribedService) {
-                $key = $type->key ?? $key;
-                $attributes = $type->attributes;
-                $type = ($type->nullable ? '?' : '').($type->type ?? throw new InvalidArgumentException(sprintf('When "%s" is used, a type must be set.', SubscribedService::class)));
-            }
-
-            if (!\is_string($type) || !preg_match('/(?(DEFINE)(?<cn>[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+))(?(DEFINE)(?<fqcn>(?&cn)(?:\\\\(?&cn))*+))^\??(?&fqcn)(?:(?:\|(?&fqcn))*+|(?:&(?&fqcn))*+)$/', $type)) {
-                throw new InvalidArgumentException(sprintf('"%s" is not a PHP type for key "%s".', \is_string($type) ? $type : get_debug_type($type), $key));
-            }
-            $optionalBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE;
-            if ('?' === $type[0]) {
-                $type = substr($type, 1);
-                $optionalBehavior = ContainerInterface::IGNORE_ON_INVALID_REFERENCE;
-            }
-            if (\is_int($name = $key)) {
-                $key = $type;
-                $name = null;
-            }
-
-            $references[$key] = new TypedReference($type, $type, $optionalBehavior, $name, $attributes);
-        }
-
-        parent::__construct(new IteratorArgument($references));
+        parent::__construct(new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod, false, $defaultPriorityMethod, (array) $exclude, $excludeSelf));
     }
 }
diff --git a/Attribute/AutowireLocator.php b/Attribute/AutowireLocator.php
@@ -11,9 +11,11 @@
 
 namespace Symfony\Component\DependencyInjection\Attribute;
 
-use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
+use Symfony\Component\DependencyInjection\TypedReference;
 use Symfony\Contracts\Service\Attribute\SubscribedService;
 use Symfony\Contracts\Service\ServiceSubscriberInterface;
 
@@ -37,14 +39,44 @@ public function __construct(
         string|array $exclude = [],
         bool $excludeSelf = true,
     ) {
-        $iterator = (new AutowireIterator($services, $indexAttribute, $defaultIndexMethod, $defaultPriorityMethod, (array) $exclude, $excludeSelf))->value;
+        if (\is_string($services)) {
+            parent::__construct(new ServiceLocatorArgument(new TaggedIteratorArgument($services, $indexAttribute, $defaultIndexMethod, true, $defaultPriorityMethod, (array) $exclude, $excludeSelf)));
 
-        if ($iterator instanceof TaggedIteratorArgument) {
-            $iterator = new TaggedIteratorArgument($iterator->getTag(), $iterator->getIndexAttribute(), $iterator->getDefaultIndexMethod(), true, $iterator->getDefaultPriorityMethod(), $iterator->getExclude(), $iterator->excludeSelf());
-        } elseif ($iterator instanceof IteratorArgument) {
-            $iterator = $iterator->getValues();
+            return;
         }
 
-        parent::__construct(new ServiceLocatorArgument($iterator));
+        $references = [];
+
+        foreach ($services as $key => $type) {
+            $attributes = [];
+
+            if ($type instanceof Autowire) {
+                $references[$key] = $type;
+                continue;
+            }
+
+            if ($type instanceof SubscribedService) {
+                $key = $type->key ?? $key;
+                $attributes = $type->attributes;
+                $type = ($type->nullable ? '?' : '').($type->type ?? throw new InvalidArgumentException(sprintf('When "%s" is used, a type must be set.', SubscribedService::class)));
+            }
+
+            if (!\is_string($type) || !preg_match('/(?(DEFINE)(?<cn>[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+))(?(DEFINE)(?<fqcn>(?&cn)(?:\\\\(?&cn))*+))^\??(?&fqcn)(?:(?:\|(?&fqcn))*+|(?:&(?&fqcn))*+)$/', $type)) {
+                throw new InvalidArgumentException(sprintf('"%s" is not a PHP type for key "%s".', \is_string($type) ? $type : get_debug_type($type), $key));
+            }
+            $optionalBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE;
+            if ('?' === $type[0]) {
+                $type = substr($type, 1);
+                $optionalBehavior = ContainerInterface::IGNORE_ON_INVALID_REFERENCE;
+            }
+            if (\is_int($name = $key)) {
+                $key = $type;
+                $name = null;
+            }
+
+            $references[$key] = new TypedReference($type, $type, $optionalBehavior, $name, $attributes);
+        }
+
+        parent::__construct(new ServiceLocatorArgument($references));
     }
 }
diff --git a/Compiler/RegisterServiceSubscribersPass.php b/Compiler/RegisterServiceSubscribersPass.php
@@ -14,6 +14,7 @@
 use Psr\Container\ContainerInterface as PsrContainerInterface;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\DependencyInjection\Argument\BoundArgument;
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
@@ -78,6 +79,11 @@ protected function processValue(mixed $value, bool $isRoot = false): mixed
         foreach ($class::getSubscribedServices() as $key => $type) {
             $attributes = [];
 
+            if (!isset($serviceMap[$key]) && $type instanceof Autowire) {
+                $subscriberMap[$key] = $type;
+                continue;
+            }
+
             if ($type instanceof SubscribedService) {
                 $key = $type->key ?? $key;
                 $attributes = $type->attributes;
diff --git a/Tests/Compiler/IntegrationTest.php b/Tests/Compiler/IntegrationTest.php
@@ -15,7 +15,6 @@
 use Psr\Container\ContainerInterface;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\Alias;
-use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\ChildDefinition;
@@ -33,7 +32,6 @@
 use Symfony\Component\DependencyInjection\Tests\Fixtures\AutoconfiguredInterface2;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\AutoconfiguredService1;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\AutoconfiguredService2;
-use Symfony\Component\DependencyInjection\Tests\Fixtures\AutowireIteratorConsumer;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\AutowireLocatorConsumer;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\BarTagClass;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\FooBarTaggedClass;
@@ -415,35 +413,7 @@ public function testLocatorConfiguredViaAttribute()
         self::assertSame($container->get(FooTagClass::class), $s->locator->get('with_key'));
         self::assertFalse($s->locator->has('nullable'));
         self::assertSame('foo', $s->locator->get('subscribed'));
-    }
-
-    public function testIteratorConfiguredViaAttribute()
-    {
-        $container = new ContainerBuilder();
-        $container->setParameter('some.parameter', 'foo');
-        $container->register(BarTagClass::class)
-            ->setPublic(true)
-        ;
-        $container->register(FooTagClass::class)
-            ->setPublic(true)
-        ;
-        $container->register(AutowireIteratorConsumer::class)
-            ->setAutowired(true)
-            ->setPublic(true)
-        ;
-
-        $container->compile();
-
-        /** @var AutowireIteratorConsumer $s */
-        $s = $container->get(AutowireIteratorConsumer::class);
-
-        self::assertInstanceOf(RewindableGenerator::class, $s->iterator);
-
-        $values = iterator_to_array($s->iterator);
-        self::assertCount(3, $values);
-        self::assertSame($container->get(BarTagClass::class), $values[BarTagClass::class]);
-        self::assertSame($container->get(FooTagClass::class), $values['with_key']);
-        self::assertSame('foo', $values['subscribed']);
+        self::assertSame('foo', $s->locator->get('subscribed1'));
     }
 
     public function testTaggedServiceWithIndexAttributeAndDefaultMethodConfiguredViaAttribute()
diff --git a/Tests/Fixtures/AutowireIteratorConsumer.php b/Tests/Fixtures/AutowireIteratorConsumer.php
diff --git a/Tests/Fixtures/AutowireLocatorConsumer.php b/Tests/Fixtures/AutowireLocatorConsumer.php
@@ -24,6 +24,7 @@ public function __construct(
             'with_key' => FooTagClass::class,
             'nullable' => '?invalid',
             'subscribed' => new SubscribedService(type: 'string', attributes: new Autowire('%some.parameter%')),
+            'subscribed1' => new Autowire('%some.parameter%'),
         ])]
         public readonly ContainerInterface $locator,
     ) {
