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

Author: ogizanagi

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

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(),

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);
+    }
+}

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);

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')));

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));
         }
 

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);
+    }
+}

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>

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']

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');