feature #20943 [DependencyInjection] Use current class as default cla…

…ss for factory declarations (ogizanagi)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DependencyInjection] Use current class as default class for factory declarations

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20888
| License       | MIT
| Doc PR        | Should update the notice about the "class" attribute on http://symfony.com/doc/current/service_container/factories.html

#20888 makes sense to me, considering the following sample extracted from the documentation:

```xml
<service id="app.newsletter_manager" class="AppBundle\Email\NewsletterManager">
    <factory class="AppBundle\Email\NewsletterManager" method="create" />
</service>
```

The class is used as a factory to create itself, thus it can be simplified to:

```xml
<service id="app.newsletter_manager" class="AppBundle\Email\NewsletterManager">
    <factory method="create" />
</service>
```

However, it's not possible to provide the same feature for the YAML format, because it doesn't allow to distinct a function from a method call if the class is not provided explicitly under the `factory` key, whereas the xml format use a dedicated `function` attribute.
Would this inconsistency between those two formats be a no-go for this feature?

The doc notices:
> When using a factory to create services, the value chosen for the class option has no effect on the resulting service. The actual class name only depends on the object that is returned by the factory. However, the configured class name may be used by compiler passes and therefore should be set to a sensible value.

If this is merged, it should be updated wisely in order to not confuse everyone with this feature when using the xml format.

UPDATE: The yaml format is now supported when the class is not provided in the factory array:

```yml
services:
    my_factory:
        class: Bar\Baz
        factory: [~, 'create']
```

Commits
-------

e6d8570 [DependencyInjection] Use current class as default class for factory declarations

src/Symfony/Component/DependencyInjection/CHANGELOG.md

@@ -4,6 +4,7 @@ CHANGELOG
 3.3.0
 -----
 
+ * added support for omitting the factory class name in a service definition if the definition class is set
  * deprecated case insensitivity of service identifiers
  * added "iterator" argument type for lazy iteration over a set of values and services
  * added "closure-proxy" argument type for turning services' methods into lazy callables

src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php

@@ -50,6 +50,7 @@ public function __construct()
             new ResolveDefinitionTemplatesPass(),
             new DecoratorServicePass(),
             new ResolveParameterPlaceHoldersPass(),
+            new ResolveFactoryClassPass(),
             new FactoryReturnTypePass($resolveClassPass),
             new CheckDefinitionValidityPass(),
             new ResolveReferencesToAliasesPass(),

src/Symfony/Component/DependencyInjection/Compiler/ResolveFactoryClassPass.php

@@ -0,0 +1,38 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
+
+/**
+ * @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
+ */
+class ResolveFactoryClassPass extends AbstractRecursivePass
+{
+    /**
+     * {@inheritdoc}
+     */
+    protected function processValue($value, $isRoot = false)
+    {
+        if ($value instanceof Definition && is_array($factory = $value->getFactory()) && null === $factory[0]) {
+            if (null === $class = $value->getClass()) {
+                throw new RuntimeException(sprintf('The "%s" service is defined to be created by a factory, but is missing the factory class. Did you forget to define the factory or service class?', $this->currentId));
+            }
+
+            $factory[0] = $class;
+            $value->setFactory($factory);
+        }
+
+        return parent::processValue($value, $isRoot);
+    }
+}

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

@@ -176,7 +176,9 @@ private function addService($definition, $id, \DOMElement $parent)
                 $this->addService($callable[0], null, $factory);
                 $factory->setAttribute('method', $callable[1]);
             } elseif (is_array($callable)) {
-                $factory->setAttribute($callable[0] instanceof Reference ? 'service' : 'class', $callable[0]);
+                if (null !== $callable[0]) {
+                    $factory->setAttribute($callable[0] instanceof Reference ? 'service' : 'class', $callable[0]);
+                }
                 $factory->setAttribute('method', $callable[1]);
             } else {
                 $factory->setAttribute('function', $callable);

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

@@ -257,7 +257,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =
                 } elseif ($childService = $factory->getAttribute('service')) {
                     $class = new Reference($childService, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false);
                 } else {
-                    $class = $factory->getAttribute('class');
+                    $class = $factory->hasAttribute('class') ? $factory->getAttribute('class') : null;
                 }
 
                 $definition->setFactory(array($class, $factory->getAttribute('method')));

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

@@ -466,6 +466,10 @@ private function parseCallable($callable, $parameter, $id, $file)
                 return array($this->resolveServices($callable[0]), $callable[1]);
             }
 
+            if ('factory' === $parameter && isset($callable[1]) && null === $callable[0]) {
+                return $callable;
+            }
+
             throw new InvalidArgumentException(sprintf('Parameter "%s" must contain an array with two elements for service "%s" in %s. Check your YAML syntax.', $parameter, $id, $file));
         }
 

src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveFactoryClassPassTest.php

@@ -0,0 +1,87 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\DependencyInjection\Tests\Compiler;
+
+use Symfony\Component\DependencyInjection\Compiler\ResolveFactoryClassPass;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\Reference;
+
+class ResolveFactoryClassPassTest extends \PHPUnit_Framework_TestCase
+{
+    public function testProcess()
+    {
+        $container = new ContainerBuilder();
+
+        $factory = $container->register('factory', 'Foo\Bar');
+        $factory->setFactory(array(null, 'create'));
+
+        $pass = new ResolveFactoryClassPass();
+        $pass->process($container);
+
+        $this->assertSame(array('Foo\Bar', 'create'), $factory->getFactory());
+    }
+
+    public function testInlinedDefinitionFactoryIsProcessed()
+    {
+        $container = new ContainerBuilder();
+
+        $factory = $container->register('factory');
+        $factory->setFactory(array((new Definition('Baz\Qux'))->setFactory(array(null, 'getInstance')), 'create'));
+
+        $pass = new ResolveFactoryClassPass();
+        $pass->process($container);
+
+        $this->assertSame(array('Baz\Qux', 'getInstance'), $factory->getFactory()[0]->getFactory());
+    }
+
+    public function provideFulfilledFactories()
+    {
+        return array(
+            array(array('Foo\Bar', 'create')),
+            array(array(new Reference('foo'), 'create')),
+            array(array(new Definition('Baz'), 'create')),
+        );
+    }
+
+    /**
+     * @dataProvider provideFulfilledFactories
+     */
+    public function testIgnoresFulfilledFactories($factory)
+    {
+        $container = new ContainerBuilder();
+        $definition = new Definition();
+        $definition->setFactory($factory);
+
+        $container->setDefinition('factory', $definition);
+
+        $pass = new ResolveFactoryClassPass();
+        $pass->process($container);
+
+        $this->assertSame($factory, $container->getDefinition('factory')->getFactory());
+    }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
+     * @expectedExceptionMessage The "factory" service is defined to be created by a factory, but is missing the factory class. Did you forget to define the factory or service class?
+     */
+    public function testNotAnyClassThrowsException()
+    {
+        $container = new ContainerBuilder();
+
+        $factory = $container->register('factory');
+        $factory->setFactory(array(null, 'create'));
+
+        $pass = new ResolveFactoryClassPass();
+        $pass->process($container);
+    }
+}

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

@@ -58,5 +58,8 @@
     <service id="new_factory3" class="FooBarClass">
       <factory class="BazClass" method="getInstance" />
     </service>
+    <service id="new_factory4" class="BazClass">
+      <factory method="getInstance" />
+    </service>
   </services>
 </container>

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

@@ -39,4 +39,5 @@ services:
     new_factory1: { class: FooBarClass, factory: factory}
     new_factory2: { class: FooBarClass, factory: ['@baz', getClass]}
     new_factory3: { class: FooBarClass, factory: [BazClass, getInstance]}
+    new_factory4: { class: BazClass, factory: [~, getInstance]}
     with_shortcut_args: [foo, '@baz']

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

@@ -246,6 +246,7 @@ public function testLoadServices()
         $this->assertEquals('factory', $services['new_factory1']->getFactory(), '->load() parses the factory tag');
         $this->assertEquals(array(new Reference('baz', ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false), 'getClass'), $services['new_factory2']->getFactory(), '->load() parses the factory tag');
         $this->assertEquals(array('BazClass', 'getInstance'), $services['new_factory3']->getFactory(), '->load() parses the factory tag');
+        $this->assertSame(array(null, 'getInstance'), $services['new_factory4']->getFactory(), '->load() accepts factory tag without class');
 
         $aliases = $container->getAliases();
         $this->assertTrue(isset($aliases['alias_for_foo']), '->load() parses <service> elements');

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

@@ -153,6 +153,7 @@ public function testLoadServices()
         $this->assertEquals('factory', $services['new_factory1']->getFactory(), '->load() parses the factory tag');
         $this->assertEquals(array(new Reference('baz'), 'getClass'), $services['new_factory2']->getFactory(), '->load() parses the factory tag');
         $this->assertEquals(array('BazClass', 'getInstance'), $services['new_factory3']->getFactory(), '->load() parses the factory tag');
+        $this->assertSame(array(null, 'getInstance'), $services['new_factory4']->getFactory(), '->load() accepts factory tag without class');
         $this->assertEquals(array('foo', new Reference('baz')), $services['with_shortcut_args']->getArguments(), '->load() parses short service definition');
 
         $aliases = $container->getAliases();