feature #22256 [DI] Reduce complexity of autowiring (nicolas-grekas)
This PR was merged into the 3.3-dev branch.
Discussion
----------
[DI] Reduce complexity of autowiring
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no (tweaking existing ones)
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
- optional args are autowired
- methods with only optional args are autowired
- default values of required args when possible
Both first changes remove the behavior delta between constructors and setters, and is expected to me now that things are more explicit. This reduces the "know-how" requirements for using autowiring and is easier to get correct intuitively. The 3rd change plays nice with named args.
Commits
-------
146f074
[DI] Reduce complexity of autowiring
This commit is contained in:
commit
4a5f22bc1e
@ -230,13 +230,8 @@ class AutowirePass extends AbstractRecursivePass
|
||||
*/
|
||||
private function autowireMethod(\ReflectionMethod $reflectionMethod, array $arguments)
|
||||
{
|
||||
$isConstructor = $reflectionMethod->isConstructor();
|
||||
$class = $reflectionMethod->class;
|
||||
$method = $reflectionMethod->name;
|
||||
|
||||
if (!$isConstructor && !$arguments && !$reflectionMethod->getNumberOfRequiredParameters()) {
|
||||
throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s() has only optional arguments, thus must be wired explicitly.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
|
||||
}
|
||||
$parameters = $reflectionMethod->getParameters();
|
||||
if (method_exists('ReflectionMethod', 'isVariadic') && $reflectionMethod->isVariadic()) {
|
||||
array_pop($parameters);
|
||||
@ -246,9 +241,6 @@ class AutowirePass extends AbstractRecursivePass
|
||||
if (array_key_exists($index, $arguments) && '' !== $arguments[$index]) {
|
||||
continue;
|
||||
}
|
||||
if (!$isConstructor && $parameter->isOptional() && !array_key_exists($index, $arguments)) {
|
||||
break;
|
||||
}
|
||||
|
||||
$type = ProxyHelper::getTypeHint($reflectionMethod, $parameter, true);
|
||||
|
||||
@ -258,7 +250,7 @@ class AutowirePass extends AbstractRecursivePass
|
||||
}
|
||||
|
||||
// no default value? Then fail
|
||||
if (!$parameter->isOptional()) {
|
||||
if (!$parameter->isDefaultValueAvailable()) {
|
||||
throw new RuntimeException(sprintf('Cannot autowire service "%s": argument $%s of method %s() must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
|
||||
}
|
||||
|
||||
|
@ -386,21 +386,19 @@ class AutowirePassTest extends TestCase
|
||||
$container->getDefinition('arg_no_type_hint');
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
|
||||
* @expectedExceptionMessage Cannot autowire service "not_really_optional_scalar": argument $foo of method Symfony\Component\DependencyInjection\Tests\Compiler\MultipleArgumentsOptionalScalarNotReallyOptional::__construct() must have a type-hint or be given a value explicitly.
|
||||
*/
|
||||
public function testOptionalScalarNotReallyOptionalThrowException()
|
||||
public function testOptionalScalarNotReallyOptionalUsesDefaultValue()
|
||||
{
|
||||
$container = new ContainerBuilder();
|
||||
|
||||
$container->register('a', __NAMESPACE__.'\A');
|
||||
$container->register('lille', __NAMESPACE__.'\Lille');
|
||||
$container->register('not_really_optional_scalar', __NAMESPACE__.'\MultipleArgumentsOptionalScalarNotReallyOptional')
|
||||
$definition = $container->register('not_really_optional_scalar', __NAMESPACE__.'\MultipleArgumentsOptionalScalarNotReallyOptional')
|
||||
->setAutowired(true);
|
||||
|
||||
$pass = new AutowirePass();
|
||||
$pass->process($container);
|
||||
|
||||
$this->assertSame('default_val', $definition->getArgument(1));
|
||||
}
|
||||
|
||||
public function testOptionalScalarArgsDontMessUpOrder()
|
||||
@ -637,7 +635,12 @@ class AutowirePassTest extends TestCase
|
||||
{
|
||||
$container = new ContainerBuilder();
|
||||
|
||||
$foo = $container->register('foo', NotWireable::class)->setAutowired(true);
|
||||
$foo = $container->register('foo', NotWireable::class)->setAutowired(true)
|
||||
->addMethodCall('setBar', array())
|
||||
->addMethodCall('setOptionalNotAutowireable', array())
|
||||
->addMethodCall('setOptionalNoTypeHint', array())
|
||||
->addMethodCall('setOptionalArgNoAutowireable', array())
|
||||
;
|
||||
|
||||
if ($method) {
|
||||
$foo->addMethodCall($method, array());
|
||||
@ -659,10 +662,6 @@ class AutowirePassTest extends TestCase
|
||||
{
|
||||
return array(
|
||||
array('setNotAutowireable', 'Cannot autowire service "foo": argument $n of method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setNotAutowireable() has type "Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass" but this class does not exist.'),
|
||||
array('setBar', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setBar() has only optional arguments, thus must be wired explicitly.'),
|
||||
array('setOptionalNotAutowireable', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalNotAutowireable() has only optional arguments, thus must be wired explicitly.'),
|
||||
array('setOptionalNoTypeHint', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalNoTypeHint() has only optional arguments, thus must be wired explicitly.'),
|
||||
array('setOptionalArgNoAutowireable', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalArgNoAutowireable() has only optional arguments, thus must be wired explicitly.'),
|
||||
array(null, 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setProtectedMethod() must be public.'),
|
||||
);
|
||||
}
|
||||
|
Reference in New Issue
Block a user