bug symfony#22279 [DI] Fix anonymous factories/configurators support (GuilhemN)

This PR was squashed before being merged into the 3.3-dev branch (closes symfony#22279).

Discussion
----------

[DI] Fix anonymous factories/configurators support

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | symfony#21999 (comment)
| License       | MIT
| Doc PR        |

Using prototypes / instanceof conditionals, anonymous factories are inlined using `Definition`, so a new instance will be created for every service created from the prototype / conditional which is inconsistent with the way other anonymous services are managed.

Commits
-------

dda43ed [DI] Fix anonymous factories/configurators support

Author: fabpot

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

@@ -259,11 +259,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =
             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 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =
             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 @@ private function processAnonymousServices(\DOMDocument $xml, $file)
         $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 @@ private function processAnonymousServices(\DOMDocument $xml, $file)
                 $tmpDomElement = new \DOMElement('_services', null, self::NS);
                 $domElement->parentNode->replaceChild($tmpDomElement, $domElement);
                 $tmpDomElement->setAttribute('id', $id);
-            } else {
-                $domElement->parentNode->removeChild($domElement);
             }
         }
     }

src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php

@@ -13,7 +13,6 @@
 
 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 @@ public function testLoadInlinedServices()
         $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]);
     }