8000 [DI] Reduce complexity of autowiring · symfony/symfony@b7c449c · GitHub
[go: up one dir, main page]
Skip to content

Commit b7c449c

Browse files
nicolas-grekasnicolas-grekas
committed
[DI] Reduce complexity of autowiring
1 parent 329b181 commit b7c449c

File tree

2 files changed

+53
-89
lines changed

2 files changed

+53
-89
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php

Lines changed: 43 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class AutowirePass extends AbstractRecursivePass
3030
private $definedTypes = array();
3131
private $types;
3232
private $ambiguousServiceTypes = array();
33-
private $usedTypes = array();
33+
private $autowired = array();
3434
private $currentDefinition;
3535

3636
/**
@@ -40,27 +40,11 @@ public function process(ContainerBuilder $container)
4040
{
4141
try {
4242
parent::process($container);
43-
44-
foreach ($this->usedTypes as $type => $id) {
45-
if (!isset($this->usedTypes[$type]) || !isset($this->ambiguousServiceTypes[$type])) {
46-
continue;
47-
}
48-
49-
if ($container->has($type) && !$container->findDefinition($type)->isAbstract()) {
50-
continue;
51-
}
52-
53-
$this->container = $container;
54-
$classOrInterface = class_exists($type, false) ? 'class' : 'interface';
55-
56-
throw new RuntimeException(sprintf('Cannot autowire service "%s": multiple candidate services exist for %s "%s".%s', $id, $classOrInterface, $type, $this->createTypeAlternatives($type)));
57-
}
5843
} finally {
59-
$this->container = null;
6044
$this->definedTypes = array();
6145
$this->types = null;
6246
$this->ambiguousServiceTypes = array();
63-
$this->usedTypes = array();
47+
$this->autowired = array();
6448
}
6549
}
6650

@@ -243,44 +227,37 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
243227
*/
244228
private function autowireMethod(\ReflectionMethod $reflectionMethod, array $arguments)
245229
{
246-
$isConstructor = $reflectionMethod->isConstructor();
247230
$class = $reflectionMethod->class;
248231
$method = $reflectionMethod->name;
249-
250-
if (!$isConstructor && !$arguments && !$reflectionMethod->getNumberOfRequiredParameters()) {
251-
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));
232+
$parameters = $reflectionMethod->getParameters();
233+
if (method_exists('ReflectionMethod', 'isVariadic') && $reflectionMethod->isVariadic()) {
234+
array_pop($parameters);
252235
}
253236

254-
foreach ($reflectionMethod->getParameters() as $index => $parameter) {
237+
foreach ($parameters as $index => $parameter) {
255238
if (array_key_exists($index, $arguments) && '' !== $arguments[$index]) {
256239
continue;
257240
}
258-
if (!$isConstructor && $parameter->isOptional() && !array_key_exists($index, $arguments)) {
259-
break;
260-
}
261-
if (method_exists($parameter, 'isVariadic') && $parameter->isVariadic()) {
262-
continue;
263-
}
264241

265242
$type = ProxyHelper::getTypeHint($reflectionMethod, $parameter, true);
266243

267244
if (!$type) {
245+
if (isset($arguments[$index])) {
246+
continue;
247+
}
248+
268249
// no default value? Then fail
269-
if (!$parameter->isOptional()) {
250+
if (!$parameter->isDefaultValueAvailable()) {
270251
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));
271252
}
272253

273-
if (!array_key_exists($index, $arguments)) {
274-
// specifically pass the default value
275-
$arguments[$index] = $parameter->getDefaultValue();
276-
}
254+
// specifically pass the default value
255+
$arguments[$index] = $parameter->getDefaultValue();
277256

278257
continue;
279258
}
280259

281-
if ($value = $this->getAutowiredReference($type)) {
282-
$this->usedTypes[$type] = $this->currentId;
283-
} else {
260+
if (!$value = $this->getAutowiredReference($type)) {
284261
$failureMessage = $this->createTypeNotFoundMessage($type, sprintf('argument $%s of method %s()', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
285262

286263
if ($parameter->isDefaultValueAvailable()) {
@@ -296,6 +273,16 @@ private function autowireMethod(\ReflectionMethod $reflectionMethod, array $argu
296273
$arguments[$index] = $value;
297274
}
298275

276+
if ($parameters && !isset($arguments[++$index])) {
277+
while (0 <= --$index) {
278+
$parameter = $parameters[$index];
279+
if (!$parameter->isDefaultValueAvailable() || $parameter->getDefaultValue() !== $arguments[$index]) {
280+
break;
281+
}
282+
unset($arguments[$index]);
283+
}
284+
}
285+
299286
// it's possible index 1 was set, then index 0, then 2, etc
300287
// make sure that we re-order so they're injected as expected
301288
ksort($arguments);
@@ -305,6 +292,8 @@ private function autowireMethod(\ReflectionMethod $reflectionMethod, array $argu
305292

306293
/**
307294
* @return Reference|null A reference to the service matching the given type, if any
295+
*
296+
* @throws RuntimeException
308297
*/
309298
private function getAutowiredReference($type, $autoRegister = true)
310299
{
@@ -316,6 +305,10 @@ private function getAutowiredReference($type, $autoRegister = true)
316305
return;
317306
}
318307

308+
if (isset($this->autowired[$type])) {
309+
return $this->autowired[$type] ? new Reference($this->autowired[$type]) : null;
310+
}
311+
319312
if (null === $this->types) {
320313
$this->populateAvailableTypes();
321314
}
@@ -326,8 +319,14 @@ private function getAutowiredReference($type, $autoRegister = true)
326319
return new Reference($this->types[$type]);
327320
}
328321

329-
if ($autoRegister && $class = $this->container->getReflectionClass($type, true)) {
330-
return $this->createAutowiredDefinition($class);
322+
if (isset($this->ambiguousServiceTypes[$type])) {
323+
$classOrInterface = class_exists($type, false) ? 'class' : 'interface';
324+
325+
throw new RuntimeException(sprintf('Cannot autowire service "%s": multiple candidate services exist for %s "%s".%s', $this->currentId, $classOrInterface, $type, $this->createTypeAlternatives($type)));
326+
}
327+
328+
if ($autoRegister) {
329+
return $this->createAutowiredDefinition($type);
331330
}
332331
}
333332

@@ -412,45 +411,28 @@ private function set($type, $id)
412411
/**
413412
* Registers a definition for the type if possible or throws an exception.
414413
*
415-
* @param \ReflectionClass $typeHint
414+
* @param string $type
416415
*
417416
* @return Reference|null A reference to the registered definition
418-
*
419-
* @throws RuntimeException
420417
*/
421-
private function createAutowiredDefinition(\ReflectionClass $typeHint)
418+
private function createAutowiredDefinition($type)
422419
{
423-
if (isset($this->ambiguousServiceTypes[$type = $typeHint->name])) {
424-
$classOrInterface = class_exists($type) ? 'class' : 'interface';
425-
426-
throw new RuntimeException(sprintf('Cannot autowire service "%s": multiple candidate services exist for %s "%s".%s', $this->currentId, $classOrInterface, $type, $this->createTypeAlternatives($type)));
427-
}
428-
429-
if (!$typeHint->isInstantiable()) {
430-
$this->container->log($this, sprintf('Type "%s" is not instantiable thus cannot be auto-registered for service "%s".', $type, $this->currentId));
431-
420+
if (!($typeHint = $this->container->getReflectionClass($type, true)) || !$typeHint->isInstantiable()) {
432421
return;
433422
}
434423

435-
$ambiguousServiceTypes = $this->ambiguousServiceTypes;
436424
$currentDefinition = $this->currentDefinition;
437-
$definitions = $this->container->getDefinitions();
438425
$currentId = $this->currentId;
439-
$this->currentId = $argumentId = sprintf('autowired.%s', $type);
426+
$this->currentId = $this->autowired[$type] = $argumentId = sprintf('autowired.%s', $type);
440427
$this->currentDefinition = $argumentDefinition = new Definition($type);
441428
$argumentDefinition->setPublic(false);
442429
$argumentDefinition->setAutowired(true);
443430

444-
$this->populateAvailableType($argumentId, $argumentDefinition);
445-
446431
try {
447432
$this->processValue($argumentDefinition, true);
448433
$this->container->setDefinition($argumentId, $argumentDefinition);
449434
} catch (RuntimeException $e) {
450-
// revert any changes done to our internal state
451-
unset($this->types[$type]);
452-
$this->ambiguousServiceTypes = $ambiguousServiceTypes;
453-
$this->container->setDefinitions($definitions);
435+
$this->autowired[$type] = false;
454436
$this->container->log($this, $e->getMessage());
455437

456438
return;

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -386,21 +386,19 @@ public function testScalarArgsCannotBeAutowired()
386386
$container->getDefinition('arg_no_type_hint');
387387
}
388388

389-
/**
390-
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
391-
* @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.
392-
*/
393-
public function testOptionalScalarNotReallyOptionalThrowException()
389+
public function testOptionalScalarNotReallyOptionalUsesDefaultValue()
394390
{
395391
$container = new ContainerBuilder();
396392

397393
$container->register('a', __NAMESPACE__.'\A');
398394
$container->register('lille', __NAMESPACE__.'\Lille');
399-
$container->register('not_really_optional_scalar', __NAMESPACE__.'\MultipleArgumentsOptionalScalarNotReallyOptional')
395+
$definition = $container->register('not_really_optional_scalar', __NAMESPACE__.'\MultipleArgumentsOptionalScalarNotReallyOptional')
400396
->setAutowired(true);
401397

402398
$pass = new AutowirePass();
403399
$pass->process($container);
400+
401+
$this->assertSame('default_val', $definition->getArgument(1));
404402
}
405403

406404
public function testOptionalScalarArgsDontMessUpOrder()
@@ -444,10 +442,6 @@ public function testOptionalScalarArgsNotPassedIfLast()
444442
array(
445443
new Reference('a'),
446444
new Reference('lille'),
447-
// third arg shouldn't *need* to be passed
448-
// but that's hard to "pull of" with autowiring, so
449-
// this assumes passing the default val is ok
450-
'some_val',
451445
),
452446
$definition->getArguments()
453447
);
@@ -650,7 +644,12 @@ public function testNotWireableCalls($method, $expectedMsg)
650644
{
651645
$container = new ContainerBuilder();
652646

653-
$foo = $container->register('foo', NotWireable::class)->setAutowired(true);
647+
$foo = $container->register('foo', NotWireable::class)->setAutowired(true)
648+
->addMethodCall('setBar', array())
649+
->addMethodCall('setOptionalNotAutowireable', array())
650+
->addMethodCall('setOptionalNoTypeHint', array())
651+
->addMethodCall('setOptionalArgNoAutowireable', array())
652+
;
654653

655654
if ($method) {
656655
$foo->addMethodCall($method, array());
@@ -672,27 +671,10 @@ public function provideNotWireableCalls()
672671
{
673672
return array(
674673
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.'),
675-
array('setBar', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setBar() has only optional arguments, thus must be wired explicitly.'),
676-
array('setOptionalNotAutowireable', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalNotAutowireable() has only optional arguments, thus must be wired explicitly.'),
677-
array('setOptionalNoTypeHint', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalNoTypeHint() has only optional arguments, thus must be wired explicitly.'),
678-
array('setOptionalArgNoAutowireable', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalArgNoAutowireable() has only optional arguments, thus must be wired explicitly.'),
679674
array(null, 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setProtectedMethod() must be public.'),
680675
);
681676
}
682677

683-
public function testAutoregisterRestoresStateOnFailure()
684-
{
685-
$container = new ContainerBuilder();
686-
687-
$container->register('e', E::class)
688-
->setAutowired(true);
689-
690-
$pass = new AutowirePass();
691-
$pass->process($container);
692-
693-
$this->assertSame(array('service_container', 'e'), array_keys($container->getDefinitions()));
694-
}
695-
696678
/**
697679
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
698680
* @expectedExceptionMessage Cannot autowire service "j": multiple candidate services exist for class "Symfony\Component\DependencyInjection\Tests\Compiler\I". This type-hint could be aliased to one of these existing services: "f", "i"; or be updated to "Symfony\Component\DependencyInjection\Tests\Compiler\IInterface".

0 commit comments

Comments
 (0)
0