From 6f578ee51404e831209fad962c72473c5f9fd17e Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 18 Feb 2017 12:21:50 +0100 Subject: [PATCH 1/3] [DI] Bug in autowiring collisions detection --- .../Tests/Compiler/AutowirePassTest.php | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index 2d34cbff1d..7bf3ad0a30 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -459,6 +459,31 @@ class AutowirePassTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array(new Reference('a'), '', new Reference('lille')), $container->getDefinition('foo')->getArguments()); } + + /** + * @dataProvider provideAutodiscoveredAutowiringOrder + * + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException + * @expectedExceptionMEssage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" for the service "a". Multiple services exist for this interface (autowired.Symfony\Component\DependencyInjection\Tests\Compiler\CollisionA, autowired.Symfony\Component\DependencyInjection\Tests\Compiler\CollisionB). + */ + public function testAutodiscoveredAutowiringOrder($class) + { + $container = new ContainerBuilder(); + + $container->register('a', __NAMESPACE__.'\\'.$class) + ->setAutowired(true); + + $pass = new AutowirePass(); + $pass->process($container); + } + + public function provideAutodiscoveredAutowiringOrder() + { + return array( + array('CannotBeAutowiredForwardOrder'), + array('CannotBeAutowiredReverseOrder'), + ); + } } class Foo @@ -540,6 +565,20 @@ class CannotBeAutowired } } +class CannotBeAutowiredForwardOrder +{ + public function __construct(CollisionA $a, CollisionInterface $b, CollisionB $c) + { + } +} + +class CannotBeAutowiredReverseOrder +{ + public function __construct(CollisionA $a, CollisionB $c, CollisionInterface $b) + { + } +} + class Lille { } From bb70472dce4cafe2cc425e9d6b4c05cbde7cc10b Mon Sep 17 00:00:00 2001 From: Guilhem Niot Date: Sat, 18 Feb 2017 18:58:12 +0100 Subject: [PATCH 2/3] [DependencyInjection] Fix autowiring collisions detection --- .../DependencyInjection/Compiler/AutowirePass.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 09a237caad..b34717553c 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -28,6 +28,7 @@ class AutowirePass implements CompilerPassInterface private $definedTypes = array(); private $types; private $notGuessableTypes = array(); + private $usedTypes = array(); /** * {@inheritdoc} @@ -44,6 +45,15 @@ class AutowirePass implements CompilerPassInterface $this->completeDefinition($id, $definition); } } + + foreach ($this->usedTypes as $type => $id) { + if (isset($this->usedTypes[$type]) && isset($this->notGuessableTypes[$type])) { + $classOrInterface = class_exists($type) ? 'class' : 'interface'; + $matchingServices = implode(', ', $this->types[$type]); + + throw new RuntimeException(sprintf('Unable to autowire argument of type "%s" for the service "%s". Multiple services exist for this %s (%s).', $type, $id, $classOrInterface, $matchingServices)); + } + } } catch (\Exception $e) { } catch (\Throwable $e) { } @@ -56,6 +66,7 @@ class AutowirePass implements CompilerPassInterface $this->definedTypes = array(); $this->types = null; $this->notGuessableTypes = array(); + $this->usedTypes = array(); if (isset($e)) { throw $e; @@ -109,9 +120,11 @@ class AutowirePass implements CompilerPassInterface if (isset($this->types[$typeHint->name]) && !isset($this->notGuessableTypes[$typeHint->name])) { $value = new Reference($this->types[$typeHint->name]); + $this->usedTypes[$typeHint->name] = $id; } else { try { $value = $this->createAutowiredDefinition($typeHint, $id); + $this->usedTypes[$typeHint->name] = $id; } catch (RuntimeException $e) { if ($parameter->allowsNull()) { $value = null; From 59812785378b6ff1970244869538c1aa5375d37b Mon Sep 17 00:00:00 2001 From: Guilhem Niot Date: Sun, 19 Feb 2017 12:32:55 +0100 Subject: [PATCH 3/3] [DependencyInjection] Fix using autowiring types when there are more than 2 services --- .../Component/DependencyInjection/Compiler/AutowirePass.php | 1 + .../DependencyInjection/Tests/Compiler/AutowirePassTest.php | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 09a237caad..17464f2b80 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -169,6 +169,7 @@ class AutowirePass implements CompilerPassInterface foreach ($definition->getAutowiringTypes() as $type) { $this->definedTypes[$type] = true; $this->types[$type] = $id; + unset($this->notGuessableTypes[$type]); } if (!$reflectionClass = $this->getReflectionClass($id, $definition)) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index 2d34cbff1d..986965255b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -173,7 +173,8 @@ class AutowirePassTest extends \PHPUnit_Framework_TestCase $container = new ContainerBuilder(); $container->register('a1', __NAMESPACE__.'\Foo'); - $container->register('a2', __NAMESPACE__.'\Foo')->addAutowiringType(__NAMESPACE__.'\Foo'); + $container->register('a2', __NAMESPACE__.'\Foo'); + $container->register('a3', __NAMESPACE__.'\Foo')->addAutowiringType(__NAMESPACE__.'\Foo'); $aDefinition = $container->register('a', __NAMESPACE__.'\NotGuessableArgument'); $aDefinition->setAutowired(true); @@ -181,7 +182,7 @@ class AutowirePassTest extends \PHPUnit_Framework_TestCase $pass->process($container); $this->assertCount(1, $container->getDefinition('a')->getArguments()); - $this->assertEquals('a2', (string) $container->getDefinition('a')->getArgument(0)); + $this->assertEquals('a3', (string) $container->getDefinition('a')->getArgument(0)); } public function testWithTypeSet()