From dda43ed8ce66acf93ce75bcf898ae9f190a6e1ba Mon Sep 17 00:00:00 2001
From: Guilhem Niot <guilhem.niot@gmail.com>
Date: Tue, 4 Apr 2017 19:57:14 +0200
Subject: [PATCH] [DI] Fix anonymous factories/configurators support

---
 .../Loader/XmlFileLoader.php                  | 25 ++++++------------
 .../Tests/Loader/XmlFileLoaderTest.php        | 26 +++++++++++--------
 2 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
index 3a9dbbae2f..ae02e088d2 100644
--- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
+++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
@@ -259,11 +259,7 @@ class XmlFileLoader extends FileLoader
             if ($function = $factory->getAttribute('function')) {
                 $definition->setFactory($function);
             } else {
-                $factoryService = $this->getChildren($factory, 'service');
-
-                if (isset($factoryService[0])) {
-                    $class = $this->parseDefinition($factoryService[0], $file);
-                } elseif ($childService = $factory->getAttribute('service')) {
+                if ($childService = $factory->getAttribute('service')) {
                     $class = new Reference($childService, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE);
                 } else {
                     $class = $factory->hasAttribute('class') ? $factory->getAttribute('class') : null;
@@ -278,11 +274,7 @@ class XmlFileLoader extends FileLoader
             if ($function = $configurator->getAttribute('function')) {
                 $definition->setConfigurator($function);
             } else {
-                $configuratorService = $this->getChildren($configurator, 'service');
-
-                if (isset($configuratorService[0])) {
-                    $class = $this->parseDefinition($configuratorService[0], $file);
-                } elseif ($childService = $configurator->getAttribute('service')) {
+                if ($childService = $configurator->getAttribute('service')) {
                     $class = new Reference($childService, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE);
                 } else {
                     $class = $configurator->getAttribute('class');
@@ -379,13 +371,14 @@ class XmlFileLoader extends FileLoader
         $xpath->registerNamespace('container', self::NS);
 
         // anonymous services as arguments/properties
-        if (false !== $nodes = $xpath->query('//container:argument[@type="service"][not(@id)]|//container:property[@type="service"][not(@id)]')) {
+        if (false !== $nodes = $xpath->query('//container:argument[@type="service"][not(@id)]|//container:property[@type="service"][not(@id)]|//container:factory[not(@service)]|//container:configurator[not(@service)]')) {
             foreach ($nodes as $node) {
-                // give it a unique name
-                $id = sprintf('%d_%s', ++$count, hash('sha256', $file));
-                $node->setAttribute('id', $id);
-
                 if ($services = $this->getChildren($node, 'service')) {
+                    // give it a unique name
+                    $id = sprintf('%d_%s', ++$count, hash('sha256', $file));
+                    $node->setAttribute('id', $id);
+                    $node->setAttribute('service', $id);
+
                     $definitions[$id] = array($services[0], $file, false);
                     $services[0]->setAttribute('id', $id);
 
@@ -417,8 +410,6 @@ class XmlFileLoader extends FileLoader
                 $tmpDomElement = new \DOMElement('_services', null, self::NS);
                 $domElement->parentNode->replaceChild($tmpDomElement, $domElement);
                 $tmpDomElement->setAttribute('id', $id);
-            } else {
-                $domElement->parentNode->removeChild($domElement);
             }
         }
     }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
index a36d5fff86..35d5f60de5 100644
--- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
@@ -13,7 +13,6 @@ namespace Symfony\Component\DependencyInjection\Tests\Loader;
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
-use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
@@ -539,23 +538,28 @@ class XmlFileLoaderTest extends TestCase
         $foo = $container->getDefinition('foo');
 
         $fooFactory = $foo->getFactory();
-        $this->assertInstanceOf('Symfony\Component\DependencyInjection\Definition', $fooFactory[0]);
-        $this->assertSame('FooFactory', $fooFactory[0]->getClass());
+        $this->assertInstanceOf(Reference::class, $fooFactory[0]);
+        $this->assertTrue($container->has((string) $fooFactory[0]));
+        $fooFactoryDefinition = $container->getDefinition((string) $fooFactory[0]);
+        $this->assertSame('FooFactory', $fooFactoryDefinition->getClass());
         $this->assertSame('createFoo', $fooFactory[1]);
 
-        $fooFactoryFactory = $fooFactory[0]->getFactory();
-        $this->assertInstanceOf('Symfony\Component\DependencyInjection\Definition', $fooFactoryFactory[0]);
-        $this->assertSame('Foobar', $fooFactoryFactory[0]->getClass());
+        $fooFactoryFactory = $fooFactoryDefinition->getFactory();
+        $this->assertInstanceOf(Reference::class, $fooFactoryFactory[0]);
+        $this->assertTrue($container->has((string) $fooFactoryFactory[0]));
+        $this->assertSame('Foobar', $container->getDefinition((string) $fooFactoryFactory[0])->getClass());
         $this->assertSame('createFooFactory', $fooFactoryFactory[1]);
 
         $fooConfigurator = $foo->getConfigurator();
-        $this->assertInstanceOf('Symfony\Component\DependencyInjection\Definition', $fooConfigurator[0]);
-        $this->assertSame('Bar', $fooConfigurator[0]->getClass());
+        $this->assertInstanceOf(Reference::class, $fooConfigurator[0]);
+        $this->assertTrue($container->has((string) $fooConfigurator[0]));
+        $fooConfiguratorDefinition = $container->getDefinition((string) $fooConfigurator[0]);
+        $this->assertSame('Bar', $fooConfiguratorDefinition->getClass());
         $this->assertSame('configureFoo', $fooConfigurator[1]);
 
-        $barConfigurator = $fooConfigurator[0]->getConfigurator();
-        $this->assertInstanceOf('Symfony\Component\DependencyInjection\Definition', $barConfigurator[0]);
-        $this->assertSame('Baz', $barConfigurator[0]->getClass());
+        $barConfigurator = $fooConfiguratorDefinition->getConfigurator();
+        $this->assertInstanceOf(Reference::class, $barConfigurator[0]);
+        $this->assertSame('Baz', $container->getDefinition((string) $barConfigurator[0])->getClass());
         $this->assertSame('configureBar', $barConfigurator[1]);
     }