[#17878] Fixing a bug where scalar values caused invalid ordering

weaverryan · weaverryan · commit 2a8bd955c984 · 2016-02-21T20:17:09.000-05:00
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
@@ -78,6 +78,14 @@ private function completeDefinition($id, Definition $definition)
 
             try {
                 if (!$typeHint = $parameter->getClass()) {
+                    // no default value? Then fail
+                    if (!$parameter->isOptional()) {
+                        throw new RuntimeException(sprintf('Unable to autowire argument index %s ($%s) for the service "%s". If this is an object, give it a type-hint. Otherwise, specify this argument\'s value explicitly.', $index, $parameter->name, $id));
+                    }
+
+                    // specifically pass the default value
+                    $arguments[$index] = $parameter->getDefaultValue();
+
                     continue;
                 }
 
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
@@ -301,7 +301,6 @@ public function testSomeSpecificArgumentsAreSet()
         $pass->process($container);
 
         $definition = $container->getDefinition('multiple');
-        // takes advantage of Reference's __toString
         $this->assertEquals(
             array(
                 new Reference('a'),
@@ -311,6 +310,92 @@ public function testSomeSpecificArgumentsAreSet()
             $definition->getArguments()
         );
     }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
+     * @expectedExceptionMessage Unable to autowire argument index 1 ($foo) for the service "arg_no_type_hint". If this is an object, give it a type-hint. Otherwise, specify this argument's value explicitly.
+     */
+    public function testScalarArgsCannotBeAutowired()
+    {
+        $container = new ContainerBuilder();
+
+        $container->register('a', __NAMESPACE__ . '\A');
+        $container->register('dunglas', __NAMESPACE__.'\Dunglas');
+        $container->register('arg_no_type_hint', __NAMESPACE__ . '\MultipleArguments')
+            ->setAutowired(true);
+
+        $pass = new AutowirePass();
+        $pass->process($container);
+
+        $container->getDefinition('arg_no_type_hint');
+    }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
+     * @expectedExceptionMessage Unable to autowire argument index 1 ($foo) for the service "not_really_optional_scalar". If this is an object, give it a type-hint. Otherwise, specify this argument's value explicitly.
+     */
+    public function testOptionalScalarNotReallyOptionalThrowException()
+    {
+        $container = new ContainerBuilder();
+
+        $container->register('a', __NAMESPACE__ . '\A');
+        $container->register('lille', __NAMESPACE__ . '\Lille');
+        $container->register('not_really_optional_scalar', __NAMESPACE__ . '\MultipleArgumentsOptionalScalarNotReallyOptional')
+            ->setAutowired(true);
+
+        $pass = new AutowirePass();
+        $pass->process($container);
+    }
+
+    public function testOptionalScalarArgsDontMessUpOrder()
+    {
+        $container = new ContainerBuilder();
+
+        $container->register('a', __NAMESPACE__ . '\A');
+        $container->register('lille', __NAMESPACE__ . '\Lille');
+        $container->register('with_optional_scalar', __NAMESPACE__ . '\MultipleArgumentsOptionalScalar')
+            ->setAutowired(true);
+
+        $pass = new AutowirePass();
+        $pass->process($container);
+
+        $definition = $container->getDefinition('with_optional_scalar');
+        $this->assertEquals(
+            array(
+                new Reference('a'),
+                // use the default value
+                'default_val',
+                new Reference('lille'),
+            ),
+            $definition->getArguments()
+        );
+    }
+
+    public function testOptionalScalarArgsNotPassedIfLast()
+    {
+        $container = new ContainerBuilder();
+
+        $container->register('a', __NAMESPACE__ . '\A');
+        $container->register('lille', __NAMESPACE__ . '\Lille');
+        $container->register('with_optional_scalar_last', __NAMESPACE__ . '\MultipleArgumentsOptionalScalarLast')
+            ->setAutowired(true);
+
+        $pass = new AutowirePass();
+        $pass->process($container);
+
+        $definition = $container->getDefinition('with_optional_scalar_last');
+        $this->assertEquals(
+            array(
+                new Reference('a'),
+                new Reference('lille'),
+                // third arg shouldn't *need* to be passed
+                // but that's hard to "pull of" with autowiring, so
+                // this assumes passing the default val is ok
+                'some_val'
+            ),
+            $definition->getArguments()
+        );
+    }
 }
 
 class Foo
@@ -432,4 +517,23 @@ class MultipleArguments
     public function __construct(A $k, $foo, Dunglas $dunglas)
     {
     }
+}
+
+class MultipleArgumentsOptionalScalar
+{
+    public function __construct(A $a, $foo = 'default_val', Lille $lille = null)
+    {
+    }
+}
+class MultipleArgumentsOptionalScalarLast
+{
+    public function __construct(A $a, Lille $lille, $foo = 'some_val')
+    {
+    }
+}
+class MultipleArgumentsOptionalScalarNotReallyOptional
+{
+    public function __construct(A $a, $foo = 'default_val', Lille $lille)
+    {
+    }
 }
