diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 67b9008423bb2..b46635dca615a 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -71,13 +71,20 @@ private function completeDefinition($id, Definition $definition) $arguments = $definition->getArguments(); foreach ($constructor->getParameters() as $index => $parameter) { - $argumentExists = array_key_exists($index, $arguments); - if ($argumentExists && '' !== $arguments[$index]) { + if (array_key_exists($index, $arguments) && '' !== $arguments[$index]) { continue; } try { if (!$typeHint = $parameter->getClass()) { + // no default value? Then fail + if (!$parameter->isOptional()) { + throw new RuntimeException(sprintf('Unable to autowire argument index %d ($%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; } @@ -110,12 +117,13 @@ private function completeDefinition($id, Definition $definition) $value = $parameter->getDefaultValue(); } - if ($argumentExists) { - $definition->replaceArgument($index, $value); - } else { - $definition->addArgument($value); - } + $arguments[$index] = $value; } + + // it's possible index 1 was set, then index 0, then 2, etc + // make sure that we re-order so they're injected as expected + ksort($arguments); + $definition->setArguments($arguments); } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index 28d6ab6339eea..4a97456f99ba1 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -282,6 +282,121 @@ public function testDontUseAbstractServices() $arguments = $container->getDefinition('bar')->getArguments(); $this->assertSame('foo', (string) $arguments[0]); } + + public function testSomeSpecificArgumentsAreSet() + { + $container = new ContainerBuilder(); + + $container->register('foo', __NAMESPACE__.'\Foo'); + $container->register('a', __NAMESPACE__.'\A'); + $container->register('dunglas', __NAMESPACE__.'\Dunglas'); + $container->register('multiple', __NAMESPACE__.'\MultipleArguments') + ->setAutowired(true) + // set the 2nd (index 1) argument only: autowire the first and third + // args are: A, Foo, Dunglas + ->setArguments(array( + 1 => new Reference('foo'), + )); + + $pass = new AutowirePass(); + $pass->process($container); + + $definition = $container->getDefinition('multiple'); + $this->assertEquals( + array( + new Reference('a'), + new Reference('foo'), + new Reference('dunglas'), + ), + $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 @@ -406,3 +521,28 @@ public function __construct(A $k) { } } +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) + { + } +}