feature #36586 [DI] allow loading and dumping tags with an attribute named "name" (nicolas-grekas)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[DI] allow loading and dumping tags with an attribute named "name"

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This is a minor feature added for consistency: using PHP, we can already define tags with an attribute named `"name"`. But then, we cannot dump such definitions in YAML nor XML since we don't have a syntax to declare such tags in these formats.

I spotted this while looking at a dumped container: we already use an attribute named `"name"` on two tags: `cache.pool` and `workflow.definition`. Currently, the dumped XML is wrong because of this.

This PR enables the following new syntaxes (the current style still works as usual):
- in YAML, consistently with the new syntax for method calls:
  ```yaml
  tags:
    - cache.pool: { name: my_cache_pool }
  ```
- in XML:
  ```xml
  <tag name="my_cache_pool">cache.pool</tag>
  ```

Commits
-------

b023e4cac3 [DI] allow loading and dumping tags with an attribute named "name"

src/Symfony/Component/DependencyInjection/CHANGELOG.md

@@ -16,6 +16,7 @@ CHANGELOG
    configure them explicitly instead
  * added class `Symfony\Component\DependencyInjection\Dumper\Preloader` to help with preloading on PHP 7.4+
  * added tags `container.preload`/`.no_preload` to declare extra classes to preload/services to not preload
+ * allowed loading and dumping tags with an attribute named "name"
  * deprecated `Definition::getDeprecationMessage()`, use `Definition::getDeprecation()` instead
  * deprecated `Alias::getDeprecationMessage()`, use `Alias::getDeprecation()` instead
  * deprecated PHP-DSL's `inline()` function, use `service()` instead

src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php

@@ -137,7 +137,11 @@ class XmlDumper extends Dumper
         foreach ($definition->getTags() as $name => $tags) {
             foreach ($tags as $attributes) {
                 $tag = $this->document->createElement('tag');
-                $tag->setAttribute('name', $name);
+                if (!\array_key_exists('name', $attributes)) {
+                    $tag->setAttribute('name', $name);
+                } else {
+                    $tag->appendChild($this->document->createTextNode($name));
+                }
                 foreach ($attributes as $key => $value) {
                     $tag->setAttribute($key, $value);
                 }

src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php

@@ -79,9 +79,9 @@ class YamlDumper extends Dumper
                 foreach ($attributes as $key => $value) {
                     $att[] = sprintf('%s: %s', $this->dumper->dump($key), $this->dumper->dump($value));
                 }
-                $att = $att ? ', '.implode(', ', $att) : '';
+                $att = $att ? ': { '.implode(', ', $att).' }' : '';
 
-                $tagsCode .= sprintf("            - { name: %s%s }\n", $this->dumper->dump($name), $att);
+                $tagsCode .= sprintf("            - %s%s\n", $this->dumper->dump($name), $att);
             }
         }
         if ($tagsCode) {

src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php

@@ -316,8 +316,9 @@ class XmlFileLoader extends FileLoader
 
         foreach ($tags as $tag) {
             $parameters = [];
+            $tagName = $tag->nodeValue;
             foreach ($tag->attributes as $name => $node) {
-                if ('name' === $name) {
+                if ('name' === $name && '' === $tagName) {
                     continue;
                 }
 
@@ -328,11 +329,11 @@ class XmlFileLoader extends FileLoader
                 $parameters[$name] = XmlUtils::phpize($node->nodeValue);
             }
 
-            if ('' === $tag->getAttribute('name')) {
+            if ('' === $tagName && '' === $tagName = $tag->getAttribute('name')) {
                 throw new InvalidArgumentException(sprintf('The tag name for service "%s" in "%s" must be a non-empty string.', (string) $service->getAttribute('id'), $file));
             }
 
-            $definition->addTag($tag->getAttribute('name'), $parameters);
+            $definition->addTag($tagName, $parameters);
         }
 
         $definition->setTags(array_merge_recursive($definition->getTags(), $defaults->getTags()));

src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

@@ -266,11 +266,16 @@ class YamlFileLoader extends FileLoader
                     $tag = ['name' => $tag];
                 }
 
-                if (!isset($tag['name'])) {
-                    throw new InvalidArgumentException(sprintf('A "tags" entry in "_defaults" is missing a "name" key in "%s".', $file));
+                if (1 === \count($tag) && \is_array(current($tag))) {
+                    $name = key($tag);
+                    $tag = current($tag);
+                } else {
+                    if (!isset($tag['name'])) {
+                        throw new InvalidArgumentException(sprintf('A "tags" entry in "_defaults" is missing a "name" key in "%s".', $file));
+                    }
+                    $name = $tag['name'];
+                    unset($tag['name']);
                 }
-                $name = $tag['name'];
-                unset($tag['name']);
 
                 if (!\is_string($name) || '' === $name) {
                     throw new InvalidArgumentException(sprintf('The tag name in "_defaults" must be a non-empty string in "%s".', $file));
@@ -568,11 +573,16 @@ class YamlFileLoader extends FileLoader
                 $tag = ['name' => $tag];
             }
 
-            if (!isset($tag['name'])) {
-                throw new InvalidArgumentException(sprintf('A "tags" entry is missing a "name" key for service "%s" in "%s".', $id, $file));
+            if (1 === \count($tag) && \is_array(current($tag))) {
+                $name = key($tag);
+                $tag = current($tag);
+            } else {
+                if (!isset($tag['name'])) {
+                    throw new InvalidArgumentException(sprintf('A "tags" entry is missing a "name" key for service "%s" in "%s".', $id, $file));
+                }
+                $name = $tag['name'];
+                unset($tag['name']);
             }
-            $name = $tag['name'];
-            unset($tag['name']);
 
             if (!\is_string($name) || '' === $name) {
                 throw new InvalidArgumentException(sprintf('The tag name for service "%s" in "%s" must be a non-empty string.', $id, $file));

src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd

@@ -187,8 +187,11 @@
   </xsd:complexType>
 
   <xsd:complexType name="tag">
-    <xsd:attribute name="name" type="xsd:string" use="required" />
-    <xsd:anyAttribute namespace="##any" processContents="lax" />
+    <xsd:simpleContent>
+      <xsd:extension base="xsd:string">
+        <xsd:anyAttribute namespace="##any" processContents="lax" />
+      </xsd:extension>
+    </xsd:simpleContent>
   </xsd:complexType>
 
   <xsd:complexType name="deprecated">

src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/anonymous.expected.yml

@@ -12,7 +12,7 @@ services:
         class: stdClass
         public: false
         tags:
-            - { name: listener }
+            - listener
     decorated:
         class: Symfony\Component\DependencyInjection\Tests\Fixtures\StdClassDecorator
         public: true

src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.expected.yml

@@ -12,7 +12,7 @@ services:
         class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo
         public: true
         tags:
-            - { name: t, a: b }
+            - t: { a: b }
         autowire: true
         autoconfigure: true
         arguments: ['@bar']
@@ -20,7 +20,7 @@ services:
         class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo
         public: true
         tags:
-            - { name: t, a: b }
+            - t: { a: b }
         autowire: true
         calls:
             - [setFoo, ['@bar']]

src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml

@@ -8,7 +8,7 @@ services:
         class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo
         public: true
         tags:
-            - { name: tag, k: v }
+            - tag: { k: v }
         lazy: true
         properties: { p: 1 }
         calls:

src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/lazy_fqcn.expected.yml

@@ -8,5 +8,5 @@ services:
         class: stdClass
         public: true
         tags:
-            - { name: proxy, interface: SomeInterface }
+            - proxy: { interface: SomeInterface }
         lazy: true

src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml

@@ -8,8 +8,8 @@ services:
         class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo
         public: true
         tags:
-            - { name: foo }
-            - { name: baz }
+            - foo
+            - baz
         deprecated:
             package: vendor/package
             version: '1.1'
@@ -20,8 +20,8 @@ services:
         class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar
         public: true
         tags:
-            - { name: foo }
-            - { name: baz }
+            - foo
+            - baz
         deprecated:
             package: vendor/package
             version: '1.1'

src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.expected.yml

@@ -8,8 +8,8 @@ services:
         class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo
         public: true
         tags:
-            - { name: foo }
-            - { name: baz }
+            - foo
+            - baz
         deprecated:
             package: vendor/package
             version: '1.1'
@@ -20,8 +20,8 @@ services:
         class: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar
         public: true
         tags:
-            - { name: foo }
-            - { name: baz }
+            - foo
+            - baz
         deprecated:
             package: vendor/package
             version: '1.1'

src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/services9.php

@@ -22,6 +22,7 @@ return function (ContainerConfigurator $c) {
         ->class(FooClass::class)
         ->tag('foo', ['foo' => 'foo'])
         ->tag('foo', ['bar' => 'bar', 'baz' => 'baz'])
+        ->tag('foo', ['name' => 'bar', 'baz' => 'baz'])
         ->factory([FooClass::class, 'getInstance'])
         ->property('foo', 'bar')
         ->property('moo', ref('foo.baz'))

src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php

@@ -16,6 +16,7 @@ $container
     ->register('foo', '\Bar\FooClass')
     ->addTag('foo', ['foo' => 'foo'])
     ->addTag('foo', ['bar' => 'bar', 'baz' => 'baz'])
+    ->addTag('foo', ['name' => 'bar', 'baz' => 'baz'])
     ->setFactory(['Bar\\FooClass', 'getInstance'])
     ->setArguments(['foo', new Reference('foo.baz'), ['%foo%' => 'foo is %foo%', 'foobar' => '%foo%'], true, new Reference('service_container')])
     ->setProperties(['foo' => 'bar', 'moo' => new Reference('foo.baz'), 'qux' => ['%foo%' => 'foo is %foo%', 'foobar' => '%foo%']])

src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml

@@ -10,6 +10,7 @@
     <service id="foo" class="Bar\FooClass" public="true">
       <tag name="foo" foo="foo"/>
       <tag name="foo" bar="bar" baz="baz"/>
+      <tag name="bar" baz="baz">foo</tag>
       <argument>foo</argument>
       <argument type="service" id="foo.baz"/>
       <argument type="collection">

src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/defaults_instanceof_importance/expected.yml

@@ -12,11 +12,11 @@ services:
         public: true
 
         tags:
-            - { name: foo_tag, tag_option: from_service }
+            - foo_tag: { tag_option: from_service }
             # these 2 are from instanceof
-            - { name: foo_tag, tag_option: from_instanceof }
-            - { name: bar_tag }
-            - { name: from_defaults }
+            - foo_tag: { tag_option: from_instanceof }
+            - bar_tag
+            - from_defaults
         # calls from instanceof are kept, but this comes later
         calls:
             # first call is from instanceof

src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml

@@ -11,8 +11,9 @@ services:
     foo:
         class: Bar\FooClass
         tags:
-            - { name: foo, foo: foo }
-            - { name: foo, bar: bar, baz: baz }
+            - foo: { foo: foo }
+            - foo: { bar: bar, baz: baz }
+            - foo: { name: bar, baz: baz }
         arguments: [foo, '@foo.baz', { '%foo%': 'foo is %foo%', foobar: '%foo%' }, true, '@service_container']
         properties: { foo: bar, moo: '@foo.baz', qux: { '%foo%': 'foo is %foo%', foobar: '%foo%' } }
         calls:
@@ -158,7 +159,7 @@ services:
     tagged_iterator_foo:
         class: Bar
         tags:
-            - { name: foo }
+            - foo
         public: false
     tagged_iterator:
         class: Bar
@@ -194,6 +195,6 @@ services:
     preload_sidekick:
         class: stdClass
         tags:
-            - {name: container.preload, class: 'Some\Sidekick1'}
-            - {name: container.preload, class: 'Some\Sidekick2'}
+            - container.preload: { class: 'Some\Sidekick1' }
+            - container.preload: { class: 'Some\Sidekick2' }
         public: true

src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_argument.yml

@@ -7,7 +7,7 @@ services:
     foo_service:
         class: Foo
         tags:
-            - { name: foo }
+            - foo
     foo_service_tagged_iterator:
         class: Bar
         arguments: [!tagged_iterator { tag: foo, index_by: barfoo, default_index_method: foobar, default_priority_method: getPriority }]