[DepedencyInjection] amended previous commit

* fixed coding standards
 * made class optional as it is not defined when using a factory service
 * renamed factory attributes in XML files, updated XSD
 * removed the factory-class as it does nothing more than the regular class attribute
 * moved usage of Reflection as 'class' is not defined when a factory-service is used
 * added more tests
 * fixed PHP dumper

src/Symfony/Components/DependencyInjection/Builder.php

@@ -232,7 +232,7 @@ class Builder extends Container implements AnnotatedContainerInterface
      *
      * @return Definition A Definition instance
      */
-    public function register($id, $class)
+    public function register($id, $class = null)
     {
         return $this->setDefinition($id, new Definition($class));
     }
@@ -329,20 +329,19 @@ class Builder extends Container implements AnnotatedContainerInterface
             require_once self::resolveValue($definition->getFile(), $this->getParameterBag()->all());
         }
 
-        $r = new \ReflectionClass(self::resolveValue($definition->getClass(), $this->getParameterBag()->all()));
-
         $arguments = $this->resolveServices(self::resolveValue($definition->getArguments(), $this->getParameterBag()->all()));
 
         if (null !== $definition->getFactoryMethod()) {
             if (null !== $definition->getFactoryService()) {
-                $factoryService = $this->get(self::resolveValue($definition->getFactoryService(), $this->getParameterBag()->all()));
-                $service = call_user_func_array(array($factoryService, $definition->getFactoryMethod()), $arguments);
-            } else if(null !== $definition->getFactoryClass()) {
-                $service = call_user_func_array(array(self::resolveValue($definition->getFactoryClass(), $this->getParameterBag()->all()), $definition->getFactoryMethod()), $arguments);
+                $factory = $this->get(self::resolveValue($definition->getFactoryService(), $this->getParameterBag()->all()));
             } else {
-                $service = call_user_func_array(array(self::resolveValue($definition->getClass(), $this->getParameterBag()->all()), $definition->getFactoryMethod()), $arguments);
+                $factory = self::resolveValue($definition->getClass(), $this->getParameterBag()->all());
             }
+
+            $service = call_user_func_array(array($factory, $definition->getFactoryMethod()), $arguments);
         } else {
+            $r = new \ReflectionClass(self::resolveValue($definition->getClass(), $this->getParameterBag()->all()));
+
             $service = null === $r->getConstructor() ? $r->newInstance() : $r->newInstanceArgs($arguments);
         }
 

src/Symfony/Components/DependencyInjection/Definition.php

@@ -24,7 +24,6 @@ class Definition
     protected $file;
     protected $factoryMethod;
     protected $factoryService;
-    protected $factoryClass;
     protected $shared;
     protected $arguments;
     protected $calls;
@@ -37,7 +36,7 @@ class Definition
      * @param string $class     The service class
      * @param array  $arguments An array of arguments to pass to the service constructor
      */
-    public function __construct($class, array $arguments = array())
+    public function __construct($class = null, array $arguments = array())
     {
         $this->class = $class;
         $this->arguments = $arguments;
@@ -71,46 +70,29 @@ class Definition
     }
 
     /**
-     * Set the name of the service that acts as a factory using the specified `constructor` method.
+     * Sets the name of the service that acts as a factory using the constructor method.
      *
-     * @param string
+     * @param string $factoryService The factory service id
+     *
+     * @return Definition The current instance
      */
     public function setFactoryService($factoryService)
     {
         $this->factoryService = $factoryService;
+
         return $this;
     }
 
     /**
-     * @return string
+     * Gets the factory service id.
+     *
+     * @return string The factory service id
      */
     public function getFactoryService()
     {
         return $this->factoryService;
     }
 
-    /**
-     * If service has a constructor method but no factory service, this class is the static callback.
-     *
-     * @param  string $factoryClass
-     * @return Definition
-     */
-    public function setFactoryClass($factoryClass)
-    {
-        $this->factoryClass = $factoryClass;
-        return $this;
-    }
-
-    /**
-     * Get the current static create class for this service.
-     *
-     * @return string
-     */
-    public function getFactoryClass()
-    {
-        return $this->factoryClass;
-    }
-
     /**
      * Sets the service class.
      *

src/Symfony/Components/DependencyInjection/Dumper/PhpDumper.php

@@ -94,9 +94,7 @@ EOF;
 
         if (null !== $definition->getFactoryMethod()) {
             if (null !== $definition->getFactoryService()) {
-                $code = sprintf("        \$instance = \$this->get%sService()->%s(%s);\n", $this->dumpValue($definition->getFactoryService()), $definition->getFactoryMethod(), implode(', ', $arguments));
-            } elseif (null !== $definition->getFactoryClass()) {
-                $code = sprintf("        \$instance = call_user_func(array(%s, '%s')%s);\n", $this->dumpValue($definition->getFactoryClass()), $definition->getFactoryMethod(), $arguments ? ', '.implode(', ', $arguments) : '');
+                $code = sprintf("        \$instance = %s->%s(%s);\n", $this->getServiceCall($definition->getFactoryService()), $definition->getFactoryMethod(), implode(', ', $arguments));
             } else {
                 $code = sprintf("        \$instance = call_user_func(array(%s, '%s')%s);\n", $class, $definition->getFactoryMethod(), $arguments ? ', '.implode(', ', $arguments) : '');
             }
@@ -148,8 +146,13 @@ EOF;
     protected function addService($id, $definition)
     {
         $name = Container::camelize($id);
-        $class = $definition->getClass();
-        $type = 0 === strpos($class, '%') ? 'Object' : $class;
+
+        $return = '';
+        if ($class = $definition->getClass()) {
+            $return = sprintf("@return %s A %s instance.", 0 === strpos($class, '%') ? 'Object' : $class, $class);
+        } elseif ($definition->getFactoryService()) {
+            $return = sprintf('@return Object An instance returned by %s::%s().', $definition->getFactoryService(), $definition->getFactoryMethod());
+        }
 
         $doc = '';
         if ($definition->isShared()) {
@@ -166,7 +169,7 @@ EOF;
     /**
      * Gets the '$id' service.$doc
      *
-     * @return $type A $class instance.
+     * $return
      */
     protected function get{$name}Service()
     {

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

@@ -47,12 +47,11 @@ class XmlDumper extends Dumper
 
     protected function addService($id, $definition)
     {
-        $code = sprintf("    <service id=\"%s\" class=\"%s\"%s%s%s%s>\n",
+        $code = sprintf("    <service id=\"%s\"%s%s%s%s>\n",
             $id,
-            $definition->getClass(),
+            $definition->getClass() ? sprintf(' class="%s"', $definition->getClass()) : '',
             $definition->getFactoryMethod() ? sprintf(' factory-method="%s"', $definition->getFactoryMethod()) : '',
-            $definition->getFactoryClass() ? sprintf(' factoryclass="%s"', $definition->getFactoryClass()) : '',
-            $definition->getFactoryService() ? sprintf(' constructor="%s"', $definition->getFactoryService()) : '',
+            $definition->getFactoryService() ? sprintf(' factory-service="%s"', $definition->getFactoryService()) : '',
             !$definition->isShared() ? ' shared="false"' : ''
         );
 

src/Symfony/Components/DependencyInjection/Dumper/YamlDumper.php

@@ -40,7 +40,9 @@ class YamlDumper extends Dumper
     protected function addService($id, $definition)
     {
         $code = "  $id:\n";
-        $code .= sprintf("    class: %s\n", $definition->getClass());
+        if ($definition->getClass()) {
+            $code .= sprintf("    class: %s\n", $definition->getClass());
+        }
 
         $annotationsCode = '';
         foreach ($definition->getAnnotations() as $name => $annotations) {
@@ -66,12 +68,8 @@ class YamlDumper extends Dumper
             $code .= sprintf("    factory_method: %s\n", $definition->getFactoryMethod());
         }
 
-        if ($definition->getFactoryClass()) {
-            $code .= sprintf("    factoryClass: %s\n", $definition->getFactoryClass());
-        }
-
         if ($definition->getFactoryService()) {
-            $code .= sprintf("    factoryService: %s\n", $definition->getFactoryService());
+            $code .= sprintf("    factory_service: %s\n", $definition->getFactoryService());
         }
 
         if ($definition->getArguments()) {

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

@@ -134,7 +134,7 @@ class XmlFileLoader extends FileLoader
 
         $definition = new Definition((string) $service['class']);
 
-        foreach (array('shared', 'factory-method', 'factoryservice', 'factoryclass') as $key) {
+        foreach (array('shared', 'factory-method', 'factory-service', 'factory-class') as $key) {
             if (isset($service[$key])) {
                 $method = 'set'.str_replace('-', '', $key);
                 $definition->$method((string) $service->getAttributeAsPhp($key));

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

@@ -131,7 +131,11 @@ class YamlFileLoader extends FileLoader
             return;
         }
 
-        $definition = new Definition($service['class']);
+        $definition = new Definition();
+
+        if (isset($service['class'])) {
+            $definition->setClass($service['class']);
+        }
 
         if (isset($service['shared'])) {
             $definition->setShared($service['shared']);
@@ -141,12 +145,8 @@ class YamlFileLoader extends FileLoader
             $definition->setFactoryMethod($service['factory_method']);
         }
 
-        if (isset($service['factoryClass'])) {
-            $definition->setFactoryClass($service['factoryClass']);
-        }
-
-        if (isset($service['factoryService'])) {
-            $definition->setFactoryService($service['factoryService']);
+        if (isset($service['factory_service'])) {
+            $definition->setFactoryService($service['factory_service']);
         }
 
         if (isset($service['file'])) {

src/Symfony/Components/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd

@@ -90,6 +90,7 @@
     <xsd:attribute name="class" type="xsd:string" />
     <xsd:attribute name="shared" type="boolean" />
     <xsd:attribute name="factory-method" type="xsd:string" />
+    <xsd:attribute name="factory-service" type="xsd:string" />
     <xsd:attribute name="alias" type="xsd:string" />
   </xsd:complexType>
 

tests/Symfony/Tests/Components/DependencyInjection/BuilderTest.php

@@ -17,15 +17,10 @@ use Symfony\Components\DependencyInjection\Definition;
 use Symfony\Components\DependencyInjection\Reference;
 use Symfony\Components\DependencyInjection\ParameterBag\ParameterBag;
 
+require_once __DIR__.'/Fixtures/includes/classes.php';
+
 class BuilderTest extends \PHPUnit_Framework_TestCase
 {
-    static protected $fixturesPath;
-
-    static public function setUpBeforeClass()
-    {
-        self::$fixturesPath = __DIR__.'/Fixtures/';
-    }
-
     /**
      * @covers Symfony\Components\DependencyInjection\Builder::setDefinitions
      * @covers Symfony\Components\DependencyInjection\Builder::getDefinitions
@@ -199,9 +194,9 @@ class BuilderTest extends \PHPUnit_Framework_TestCase
     public function testCreateService()
     {
         $builder = new Builder();
-        $builder->register('foo1', 'FooClass')->setFile(self::$fixturesPath.'/includes/foo.php');
+        $builder->register('foo1', 'FooClass')->setFile(__DIR__.'/Fixtures/includes/foo.php');
         $this->assertInstanceOf('\FooClass', $builder->get('foo1'), '->createService() requires the file defined by the service definition');
-        $builder->register('foo2', 'FooClass')->setFile(self::$fixturesPath.'/includes/%file%.php');
+        $builder->register('foo2', 'FooClass')->setFile(__DIR__.'/Fixtures/includes/%file%.php');
         $builder->setParameter('file', 'foo');
         $this->assertInstanceOf('\FooClass', $builder->get('foo2'), '->createService() replaces parameters in the file provided by the service definition');
     }
@@ -242,6 +237,18 @@ class BuilderTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(array('foo' => 'bar', 'bar' => 'foo', $builder->get('bar')), $builder->get('foo1')->arguments, '->createService() passes the arguments to the factory method');
     }
 
+    /**
+     * @covers Symfony\Components\DependencyInjection\Builder::createService
+     */
+    public function testCreateServiceFactoryService()
+    {
+        $builder = new Builder();
+        $builder->register('baz_service')->setFactoryService('baz_factory')->setFactoryMethod('getInstance');
+        $builder->register('baz_factory', 'BazClass');
+
+        $this->assertType('BazClass', $builder->get('baz_service'));
+    }
+
     /**
      * @covers Symfony\Components\DependencyInjection\Builder::createService
      */
@@ -259,8 +266,6 @@ class BuilderTest extends \PHPUnit_Framework_TestCase
      */
     public function testCreateServiceConfigurator()
     {
-        require_once self::$fixturesPath.'/includes/classes.php';
-
         $builder = new Builder();
         $builder->register('foo1', 'FooClass')->setConfigurator('sc_configure');
         $this->assertTrue($builder->get('foo1')->configured, '->createService() calls the configurator');
@@ -394,27 +399,4 @@ class BuilderTest extends \PHPUnit_Framework_TestCase
         ), '->findAnnotatedServiceIds() returns an array of service ids and its annotation attributes');
         $this->assertEquals(array(), $builder->findAnnotatedServiceIds('foobar'), '->findAnnotatedServiceIds() returns an empty array if there is annotated services');
     }
-
-    public function testFactories()
-    {
-        $def1 = new Definition('BazClass');
-        $def1->setFactoryClass('BazFactory');
-        $def1->setConstructor('createStatic');
-
-        $def2 = new Definition('BazClass');
-        $def2->setFactoryService('BazFactoryService');
-        $def2->setFactoryMethod('create');
-
-        $def3 = new Definition('BazFactory');
-
-        $builder = new Builder();
-        $builder->addDefinitions(array(
-            'baz_factory' => $def1,
-            'baz_service' => $def2,
-            'BazFactoryService' => $def3,
-        ));
-
-        $this->assertType('BazClass', $builder->get('baz_factory'));
-        $this->assertType('Bazclass', $builder->get('baz_service'));
-    }
 }

tests/Symfony/Tests/Components/DependencyInjection/DefinitionTest.php

@@ -37,13 +37,6 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('foo', $def->getFactoryMethod(), '->getFactoryMethod() returns the factory method name');
     }
 
-    public function testSetGetFactoryClass()
-    {
-        $def = new Definition('stdClass');
-        $this->assertSame($def, $def->setFactoryClass('stdClass2'), "->setFactoryClass() implements a fluent interface.");
-        $this->assertEquals('stdClass2', $def->getFactoryClass(), 'Overwrite default factory class method did not work.');
-    }
-
     public function testSetGetFactoryService()
     {
         $def = new Definition('stdClass');

tests/Symfony/Tests/Components/DependencyInjection/Fixtures/containers/container9.php

@@ -46,5 +46,10 @@ $container->
     addMethodCall('setBar', array(new Reference('foo', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)))->
     addMethodCall('setBar', array(new Reference('foobaz', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)))
 ;
+$container->
+    register('factory_service')->
+    setFactoryService('foo.baz')->
+    setFactoryMethod('getInstance')
+;
 
 return $container;

tests/Symfony/Tests/Components/DependencyInjection/Fixtures/graphviz/services9.dot

@@ -8,6 +8,7 @@ digraph sc {
   node_foo_baz [label="foo.baz\nBazClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
   node_foo_bar [label="foo_bar\nFooClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
   node_method_call1 [label="method_call1\nFooClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
+  node_factory_service [label="factory_service\n\n", shape=record, fillcolor="#eeeeee", style="filled"];
   node_service_container [label="service_container\nSymfony\\Components\\DependencyInjection\\Builder\n", shape=record, fillcolor="#9999ff", style="filled"];
   node_foobaz [label="foobaz\n\n", shape=record, fillcolor="#ff9999", style="filled"];
   node_foo -> node_foo_baz [label="" style="filled"];

tests/Symfony/Tests/Components/DependencyInjection/Fixtures/includes/classes.php

@@ -30,16 +30,3 @@ class BazClass
     {
     }
 }
-
-class BazFactory
-{
-    static public function createStatic()
-    {
-        return new BazClass();
-    }
-
-    public function create()
-    {
-        return new BazClass();
-    }
-}

tests/Symfony/Tests/Components/DependencyInjection/Fixtures/php/services9.php

@@ -124,6 +124,24 @@ class ProjectServiceContainer extends Container
         return $instance;
     }
 
+    /**
+     * Gets the 'factory_service' service.
+     *
+     * This service is shared.
+     * This method always returns the same instance of the service.
+     *
+     * @return Object An instance returned by foo.baz::getInstance().
+     */
+    protected function getFactoryServiceService()
+    {
+        if (isset($this->shared['factory_service'])) return $this->shared['factory_service'];
+
+        $instance = $this->getFoo_BazService()->getInstance();
+        $this->shared['factory_service'] = $instance;
+
+        return $instance;
+    }
+
     /**
      * Gets the alias_for_foo service alias.
      *

tests/Symfony/Tests/Components/DependencyInjection/Fixtures/xml/services6.xml

@@ -43,8 +43,6 @@
       </call>
     </service>
     <service id="alias_for_foo" alias="foo" />
-
-    <service id="factory_class" constructor="createStatic" factoryclass="BazFactory" />
-    <service id="factory_service" constructor="create" factoryservice="BazFactoryService" />
+    <service id="factory_service" factory-method="getInstance" factory-service="baz_factory" />
   </services>
 </container>

tests/Symfony/Tests/Components/DependencyInjection/Fixtures/xml/services9.xml

@@ -52,6 +52,8 @@
         <argument type="service" id="foobaz" on-invalid="ignore" />
       </call>
     </service>
+    <service id="factory_service" factory-method="getInstance" factory-service="foo.baz">
+    </service>
     <service id="alias_for_foo" alias="foo" />
   </services>
 </container>

tests/Symfony/Tests/Components/DependencyInjection/Fixtures/yaml/services6.yml

@@ -18,5 +18,4 @@ services:
     calls:
       - [ setBar, [ foo, @foo, [true, false] ] ]
   alias_for_foo: @foo
-  factory_class: { class: BazClass, factoryClass: BazFactory }
-  factory_service: { class: BazClass, factoryService: BazFactoryService }
+  factory_service: { class: BazClass, factory_method: getInstance, factory_service: baz_factory }

tests/Symfony/Tests/Components/DependencyInjection/Fixtures/yaml/services9.yml

@@ -36,4 +36,7 @@ services:
       - [setBar, ['@@foo']]
       - [setBar, ['@@foobaz']]
       
+  factory_service:
+    factory_method: getInstance
+    factory_service: foo.baz
   alias_for_foo: @foo

tests/Symfony/Tests/Components/DependencyInjection/Loader/XmlFileLoaderTest.php

@@ -127,8 +127,7 @@ class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(array('BazClass', 'configureStatic'), $services['configurator3']->getConfigurator(), '->load() parses the configurator tag');
         $this->assertEquals(array(array('setBar', array())), $services['method_call1']->getMethodCalls(), '->load() parses the method_call tag');
         $this->assertEquals(array(array('setBar', array('foo', new Reference('foo'), array(true, false)))), $services['method_call2']->getMethodCalls(), '->load() parses the method_call tag');
-        $this->assertEquals('BazFactory', $services['factory_class']->getFactoryClass());
-        $this->assertEquals('BazFactoryService', $services['factory_service']->getFactoryService());
+        $this->assertEquals('baz_factory', $services['factory_service']->getFactoryService());
 
         $aliases = $config->getAliases();
         $this->assertTrue(isset($aliases['alias_for_foo']), '->load() parses <service> elements');

tests/Symfony/Tests/Components/DependencyInjection/Loader/YamlFileLoaderTest.php

@@ -94,9 +94,8 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(array('BazClass', 'configureStatic'), $services['configurator3']->getConfigurator(), '->load() parses the configurator tag');
         $this->assertEquals(array(array('setBar', array())), $services['method_call1']->getMethodCalls(), '->load() parses the method_call tag');
         $this->assertEquals(array(array('setBar', array('foo', new Reference('foo'), array(true, false)))), $services['method_call2']->getMethodCalls(), '->load() parses the method_call tag');
-        $this->assertEquals('BazFactory', $services['factory_class']->getFactoryClass());
-        $this->assertEquals('BazFactoryService', $services['factory_service']->getFactoryService());
-        
+        $this->assertEquals('baz_factory', $services['factory_service']->getFactoryService());
+
         $aliases = $config->getAliases();
         $this->assertTrue(isset($aliases['alias_for_foo']), '->load() parses aliases');
         $this->assertEquals('foo', $aliases['alias_for_foo'], '->load() parses aliases');