From b023e4cac3d98d12078204704c6ec78a59be6c9a Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 26 Apr 2020 15:52:23 +0200 Subject: [PATCH] [DI] allow loading and dumping tags with an attribute named "name" --- .../DependencyInjection/CHANGELOG.md | 1 + .../DependencyInjection/Dumper/XmlDumper.php | 6 ++++- .../DependencyInjection/Dumper/YamlDumper.php | 4 +-- .../Loader/XmlFileLoader.php | 7 ++--- .../Loader/YamlFileLoader.php | 26 +++++++++++++------ .../schema/dic/services/services-1.0.xsd | 7 +++-- .../Fixtures/config/anonymous.expected.yml | 2 +- .../Fixtures/config/defaults.expected.yml | 4 +-- .../Fixtures/config/instanceof.expected.yml | 2 +- .../Fixtures/config/lazy_fqcn.expected.yml | 2 +- .../Fixtures/config/prototype.expected.yml | 8 +++--- .../config/prototype_array.expected.yml | 8 +++--- .../Tests/Fixtures/config/services9.php | 1 + .../Tests/Fixtures/containers/container9.php | 1 + .../Tests/Fixtures/xml/services9.xml | 1 + .../expected.yml | 8 +++--- .../Tests/Fixtures/yaml/services9.yml | 11 ++++---- .../yaml/services_with_tagged_argument.yml | 2 +- 18 files changed, 62 insertions(+), 39 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index 755156f0be..72626da76a 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/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 diff --git a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php index bf2ea990a9..2a0ee95de6 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php +++ b/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); } diff --git a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php index aeecd774d2..f46cef7846 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php +++ b/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) { diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 26928f2f3c..f4c30a3087 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/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())); diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 75ed3ad62e..d8d7c51c75 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/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)); diff --git a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd index 55c26ffdea..b50fdbdac9 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd +++ b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd @@ -187,8 +187,11 @@ - - + + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/anonymous.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/anonymous.expected.yml index c6a6820275..80bdde373c 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/anonymous.expected.yml +++ b/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 diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.expected.yml index 2b389b6945..032142029d 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/defaults.expected.yml +++ b/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']] diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml index b12a304221..fd71cfaebd 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/instanceof.expected.yml +++ b/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: diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/lazy_fqcn.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/lazy_fqcn.expected.yml index d5a272c4bf..f8f5e86187 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/lazy_fqcn.expected.yml +++ b/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 diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml index deb7abdc6e..8796091ea8 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.expected.yml +++ b/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' diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.expected.yml index deb7abdc6e..8796091ea8 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.expected.yml +++ b/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' diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/services9.php index 74dfdcfef9..2d5a1cdc93 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/services9.php +++ b/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')) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php index 7f9d8db80b..a1d71b9d94 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php +++ b/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%']]) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml index eafb839f6d..cc7a2e116e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml @@ -10,6 +10,7 @@ + foo foo diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/defaults_instanceof_importance/expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/defaults_instanceof_importance/expected.yml index e9161dccfc..f3885096f6 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/defaults_instanceof_importance/expected.yml +++ b/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 diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml index 0f6164d9ad..43694e0fa9 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml +++ b/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 diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_argument.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_argument.yml index ad36141dae..b2c05bed04 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_argument.yml +++ b/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 }]