bug #41176 [DependencyInjection] fix dumping service-closure-arguments (nicolas-grekas)

This PR was merged into the 4.4 branch. Discussion ---------- [DependencyInjection] fix dumping service-closure-arguments | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #39259 | License | MIT | Doc PR | - 5.3 uses service closures a bit more, so that this is required to make the `lint:container` command work. Commits ------- 1aa9a249fc [DependencyInjection] fix dumping service-closure-arguments
2021-05-11 17:59:00 +02:00 · 2021-05-11 17:59:00 +02:00 · 9a53e6acc1
parent 89c1be89f0 1aa9a249fc
commit 9a53e6acc1
12 changed files with 97 additions and 5 deletions
--- a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
+++ b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
@ -264,9 +264,6 @@ class XmlDumper extends Dumper
                $element->setAttribute($keyAttribute, $key);
            }

-            if ($value instanceof ServiceClosureArgument) {
-                $value = $value->getValues()[0];
-            }
            if (\is_array($tag = $value)) {
                $element->setAttribute('type', 'collection');
                $this->convertParameters($value, $type, $element, 'key');
@ -290,8 +287,12 @@ class XmlDumper extends Dumper
            } elseif ($value instanceof ServiceLocatorArgument) {
                $element->setAttribute('type', 'service_locator');
                $this->convertParameters($value->getValues(), $type, $element, 'key');
-            } elseif ($value instanceof Reference) {
+            } elseif ($value instanceof Reference || $value instanceof ServiceClosureArgument) {
                $element->setAttribute('type', 'service');
+                if ($value instanceof ServiceClosureArgument) {
+                    $element->setAttribute('type', 'service_closure');
+                    $value = $value->getValues()[0];
+                }
                $element->setAttribute('id', (string) $value);
                $behavior = $value->getInvalidBehavior();
                if (ContainerInterface::NULL_ON_INVALID_REFERENCE == $behavior) {
--- a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php
+++ b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php
@ -234,6 +234,8 @@ class YamlDumper extends Dumper
    {
        if ($value instanceof ServiceClosureArgument) {
            $value = $value->getValues()[0];
+
+            return new TaggedValue('service_closure', $this->getServiceCall((string) $value, $value));
        }
        if ($value instanceof ArgumentInterface) {
            $tag = $value;
--- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
+++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
@ -15,6 +15,7 @@ use Symfony\Component\Config\Util\XmlUtils;
 use Symfony\Component\DependencyInjection\Alias;
 use Symfony\Component\DependencyInjection\Argument\BoundArgument;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
+use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\ChildDefinition;
@ -507,6 +508,13 @@ class XmlFileLoader extends FileLoader
                        throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="iterator" only accepts collections of type="service" references in "%s".', $name, $file));
                    }
                    break;
+                case 'service_closure':
+                    if ('' === $arg->getAttribute('id')) {
+                        throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="service_closure" has no or empty "id" attribute in "%s".', $name, $file));
+                    }
+
+                    $arguments[$key] = new ServiceClosureArgument(new Reference($arg->getAttribute('id'), $invalidBehavior));
+                    break;
                case 'service_locator':
                    $arg = $this->getArgumentsAsPhp($arg, $name, $file);
                    try {
--- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
+++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
@ -15,6 +15,7 @@ use Symfony\Component\DependencyInjection\Alias;
 use Symfony\Component\DependencyInjection\Argument\ArgumentInterface;
 use Symfony\Component\DependencyInjection\Argument\BoundArgument;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
+use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\ChildDefinition;
@ -745,6 +746,15 @@ class YamlFileLoader extends FileLoader
                    throw new InvalidArgumentException(sprintf('"!iterator" tag only accepts arrays of "@service" references in "%s".', $file));
                }
            }
+            if ('service_closure' === $value->getTag()) {
+                $argument = $this->resolveServices($argument, $file, $isParameter);
+
+                if (!$argument instanceof Reference) {
+                    throw new InvalidArgumentException(sprintf('"!service_closure" tag only accepts service references in "%s".', $file));
+                }
+
+                return new ServiceClosureArgument($argument);
+            }
            if ('service_locator' === $value->getTag()) {
                if (!\is_array($argument)) {
                    throw new InvalidArgumentException(sprintf('"!service_locator" tag only accepts maps in "%s".', $file));
--- 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
@ -267,6 +267,7 @@
      <xsd:enumeration value="constant" />
      <xsd:enumeration value="binary" />
      <xsd:enumeration value="iterator" />
+      <xsd:enumeration value="service_closure" />
      <xsd:enumeration value="service_locator" />
      <!-- "tagged" is an alias of "tagged_iterator", using "tagged_iterator" is preferred. -->
      <xsd:enumeration value="tagged" />
--- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php
@ -13,6 +13,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Dumper;

 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@ -230,6 +231,17 @@ class XmlDumperTest extends TestCase
        $this->assertStringEqualsFile(self::$fixturesPath.'/xml/services_with_tagged_arguments.xml', $dumper->dump());
    }

+    public function testServiceClosure()
+    {
+        $container = new ContainerBuilder();
+        $container->register('foo', 'Foo')
+            ->addArgument(new ServiceClosureArgument(new Reference('bar', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)))
+        ;
+
+        $dumper = new XmlDumper($container);
+        $this->assertStringEqualsFile(self::$fixturesPath.'/xml/services_with_service_closure.xml', $dumper->dump());
+    }
+
    public function testDumpAbstractServices()
    {
        $container = include self::$fixturesPath.'/containers/container_abstract.php';
--- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php
@ -13,6 +13,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Dumper;

 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@ -117,6 +118,17 @@ class YamlDumperTest extends TestCase
        $this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services_with_tagged_argument.yml', $dumper->dump());
    }

+    public function testServiceClosure()
+    {
+        $container = new ContainerBuilder();
+        $container->register('foo', 'Foo')
+            ->addArgument(new ServiceClosureArgument(new Reference('bar', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)))
+        ;
+
+        $dumper = new YamlDumper($container);
+        $this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services_with_service_closure.yml', $dumper->dump());
+    }
+
    private function assertEqualYamlStructure(string $expected, string $yaml, string $message = '')
    {
        $parser = new Parser();
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_service_closure.xml
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_service_closure.xml
@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+  <services>
+    <service id="service_container" class="Symfony\Component\DependencyInjection\ContainerInterface" public="true" synthetic="true"/>
+    <service id="foo" class="Foo">
+      <argument type="service_closure" id="bar" on-invalid="ignore"/>
+    </service>
+    <service id="Psr\Container\ContainerInterface" alias="service_container" public="false"/>
+    <service id="Symfony\Component\DependencyInjection\ContainerInterface" alias="service_container" public="false"/>
+  </services>
+</container>
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_service_closure.yml
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_service_closure.yml
@ -0,0 +1,15 @@
+
+services:
+    service_container:
+        class: Symfony\Component\DependencyInjection\ContainerInterface
+        public: true
+        synthetic: true
+    foo:
+        class: Foo
+        arguments: [!service_closure '@?bar']
+    Psr\Container\ContainerInterface:
+        alias: service_container
+        public: false
+    Symfony\Component\DependencyInjection\ContainerInterface:
+        alias: service_container
+        public: false
--- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
@ -21,6 +21,7 @@ use Symfony\Component\Config\Resource\GlobResource;
 use Symfony\Component\Config\Util\Exception\XmlParsingException;
 use Symfony\Component\DependencyInjection\Argument\BoundArgument;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
+use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass;
@ -376,6 +377,15 @@ class XmlFileLoaderTest extends TestCase
        $this->assertEquals(new ServiceLocatorArgument($taggedIterator), $container->getDefinition('foo_tagged_locator')->getArgument(0));
    }

+    public function testParseServiceClosure()
+    {
+        $container = new ContainerBuilder();
+        $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
+        $loader->load('services_with_service_closure.xml');
+
+        $this->assertEquals(new ServiceClosureArgument(new Reference('bar', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), $container->getDefinition('foo')->getArgument(0));
+    }
+
    public function testParseTagsWithoutNameThrowsException()
    {
        $this->expectException(InvalidArgumentException::class);
--- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php
@ -20,6 +20,7 @@ use Symfony\Component\Config\Resource\FileResource;
 use Symfony\Component\Config\Resource\GlobResource;
 use Symfony\Component\DependencyInjection\Argument\BoundArgument;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
+use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass;
@ -364,6 +365,15 @@ class YamlFileLoaderTest extends TestCase
        }
    }

+    public function testParseServiceClosure()
+    {
+        $container = new ContainerBuilder();
+        $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
+        $loader->load('services_with_service_closure.yml');
+
+        $this->assertEquals(new ServiceClosureArgument(new Reference('bar', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), $container->getDefinition('foo')->getArgument(0));
+    }
+
    public function testNameOnlyTagsAreAllowedAsString()
    {
        $container = new ContainerBuilder();
--- a/src/Symfony/Component/DependencyInjection/composer.json
+++ b/src/Symfony/Component/DependencyInjection/composer.json
@ -21,7 +21,7 @@
        "symfony/service-contracts": "^1.1.6|^2"
    },
    "require-dev": {
-        "symfony/yaml": "^3.4|^4.0|^5.0",
+        "symfony/yaml": "^4.4|^5.0",
        "symfony/config": "^4.3",
        "symfony/expression-language": "^3.4|^4.0|^5.0"
    },