bug #24991 [DependencyInjection] Single typed argument can be applied…

… on multiple parameters (nicolas-grekas, sroze) This PR was merged into the 3.4 branch. Discussion ---------- [DependencyInjection] Single typed argument can be applied on multiple parameters | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ø | License | MIT | Doc PR | ø I'm @nicolas-grekas' test writer today. This makes the argument resolution working when injecting the same type multiple times (sub-set of PR #24978) Commits ------- d512654 Test that named arguments are prioritized over typehinted bf7eeef Prove that change is working with tests 2176be7 [DI] Fix by-type args injection
symfony · Nov 17, 2017 · 8d7f6ed · 8d7f6ed
2 parents 2f19ddb + d512654
commit 8d7f6ed
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 4 deletions.
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php
@@ -68,15 +68,17 @@ protected function processValue($value, $isRoot = false)
                     throw new InvalidArgumentException(sprintf('Invalid service "%s": the value of argument "%s" of method "%s()" must be null, an instance of %s or an instance of %s, %s given.', $this->currentId, $key, $class !== $this->currentId ? $class.'::'.$method : $method, Reference::class, Definition::class, gettype($argument)));
                 }
 
+                $typeFound = false;
                 foreach ($parameters as $j => $p) {
-                    if (ProxyHelper::getTypeHint($r, $p, true) === $key) {
+                    if (!array_key_exists($j, $resolvedArguments) && ProxyHelper::getTypeHint($r, $p, true) === $key) {
                         $resolvedArguments[$j] = $argument;
-
-                        continue 2;
+                        $typeFound = true;
                     }
                 }
 
-                throw new InvalidArgumentException(sprintf('Invalid service "%s": method "%s()" has no argument type-hinted as "%s". Check your service definition.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method, $key));
+                if (!$typeFound) {
+                    throw new InvalidArgumentException(sprintf('Invalid service "%s": method "%s()" has no argument type-hinted as "%s". Check your service definition.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method, $key));
+                }
             }
 
             if ($resolvedArguments !== $call[1]) {

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php
@@ -17,6 +17,7 @@
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\SimilarArgumentsDummy;
 
 /**
  * @author Kévin Dunglas <dunglas@gmail.com>
@@ -125,6 +126,32 @@ public function testTypedArgument()
 
         $this->assertEquals(array(new Reference('foo'), '123'), $definition->getArguments());
     }
+
+    public function testResolvesMultipleArgumentsOfTheSameType()
+    {
+        $container = new ContainerBuilder();
+
+        $definition = $container->register(SimilarArgumentsDummy::class, SimilarArgumentsDummy::class);
+        $definition->setArguments(array(CaseSensitiveClass::class => new Reference('foo'), '$token' => 'qwerty'));
+
+        $pass = new ResolveNamedArgumentsPass();
+        $pass->process($container);
+
+        $this->assertEquals(array(new Reference('foo'), 'qwerty', new Reference('foo')), $definition->getArguments());
+    }
+
+    public function testResolvePrioritizeNamedOverType()
+    {
+        $container = new ContainerBuilder();
+
+        $definition = $container->register(SimilarArgumentsDummy::class, SimilarArgumentsDummy::class);
+        $definition->setArguments(array(CaseSensitiveClass::class => new Reference('foo'), '$token' => 'qwerty', '$class1' => new Reference('bar')));
+
+        $pass = new ResolveNamedArgumentsPass();
+        $pass->process($container);
+
+        $this->assertEquals(array(new Reference('bar'), 'qwerty', new Reference('foo')), $definition->getArguments());
+    }
 }
 
 class NoConstructor

diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
@@ -32,6 +32,7 @@
 use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
 use Symfony\Component\DependencyInjection\Loader\ClosureLoader;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\SimilarArgumentsDummy;
 use Symfony\Component\DependencyInjection\TypedReference;
 use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
 use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag;
@@ -1270,6 +1271,30 @@ public function testParameterWithMixedCase()
 
         $this->assertSame('bar', $container->get('foo')->foo);
     }
+
+    public function testArgumentsHaveHigherPriorityThanBindings()
+    {
+        $container = new ContainerBuilder();
+        $container->register('class.via.bindings', CaseSensitiveClass::class)->setArguments(array(
+            'via-bindings',
+        ));
+        $container->register('class.via.argument', CaseSensitiveClass::class)->setArguments(array(
+            'via-argument',
+        ));
+        $container->register('foo', SimilarArgumentsDummy::class)->setPublic(true)->setBindings(array(
+            CaseSensitiveClass::class => new Reference('class.via.bindings'),
+            '$token' => '1234',
+        ))->setArguments(array(
+            '$class1' => new Reference('class.via.argument'),
+        ));
+
+        $this->assertSame(array('service_container', 'class.via.bindings', 'class.via.argument', 'foo', 'Psr\Container\ContainerInterface', 'Symfony\Component\DependencyInjection\ContainerInterface'), $container->getServiceIds());
+
+        $container->compile();
+
+        $this->assertSame('via-argument', $container->get('foo')->class1->identifier);
+        $this->assertSame('via-bindings', $container->get('foo')->class2->identifier);
+    }
 }
 
 class FooClass

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CaseSensitiveClass.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CaseSensitiveClass.php
@@ -13,4 +13,10 @@
 
 class CaseSensitiveClass
 {
+    public $identifier;
+
+    public function __construct($identifier = null)
+    {
+        $this->identifier = $identifier;
+    }
 }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/SimilarArgumentsDummy.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/SimilarArgumentsDummy.php
@@ -0,0 +1,24 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+class SimilarArgumentsDummy
+{
+    public $class1;
+    public $class2;
+
+    public function __construct(CaseSensitiveClass $class1, string $token, CaseSensitiveClass $class2)
+    {
+        $this->class1 = $class1;
+        $this->class2 = $class2;
+    }
+}