Skip to content

Commit 8d7f6ed

Browse files
stofstof
committed
bug symfony#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 symfony#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
2 parents 2f19ddb + d512654 commit 8d7f6ed

File tree

5 files changed

+88
-4
lines changed

5 files changed

+88
-4
lines changed

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,17 @@ protected function processValue($value, $isRoot = false)
6868
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)));
6969
}
7070

71+
$typeFound = false;
7172
foreach ($parameters as $j => $p) {
72-
if (ProxyHelper::getTypeHint($r, $p, true) === $key) {
73+
if (!array_key_exists($j, $resolvedArguments) && ProxyHelper::getTypeHint($r, $p, true) === $key) {
7374
$resolvedArguments[$j] = $argument;
74-
75-
continue 2;
75+
$typeFound = true;
7676
}
7777
}
7878

79-
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));
79+
if (!$typeFound) {
80+
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));
81+
}
8082
}
8183

8284
if ($resolvedArguments !== $call[1]) {

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\DependencyInjection\Reference;
1818
use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
1919
use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy;
20+
use Symfony\Component\DependencyInjection\Tests\Fixtures\SimilarArgumentsDummy;
2021

2122
/**
2223
* @author Kévin Dunglas <dunglas@gmail.com>
@@ -125,6 +126,32 @@ public function testTypedArgument()
125126

126127
$this->assertEquals(array(new Reference('foo'), '123'), $definition->getArguments());
127128
}
129+
130+
public function testResolvesMultipleArgumentsOfTheSameType()
131+
{
132+
$container = new ContainerBuilder();
133+
134+
$definition = $container->register(SimilarArgumentsDummy::class, SimilarArgumentsDummy::class);
135+
$definition->setArguments(array(CaseSensitiveClass::class => new Reference('foo'), '$token' => 'qwerty'));
136+
137+
$pass = new ResolveNamedArgumentsPass();
138+
$pass->process($container);
139+
140+
$this->assertEquals(array(new Reference('foo'), 'qwerty', new Reference('foo')), $definition->getArguments());
141+
}
142+
143+
public function testResolvePrioritizeNamedOverType()
144+
{
145+
$container = new ContainerBuilder();
146+
147+
$definition = $container->register(SimilarArgumentsDummy::class, SimilarArgumentsDummy::class);
148+
$definition->setArguments(array(CaseSensitiveClass::class => new Reference('foo'), '$token' => 'qwerty', '$class1' => new Reference('bar')));
149+
150+
$pass = new ResolveNamedArgumentsPass();
151+
$pass->process($container);
152+
153+
$this->assertEquals(array(new Reference('bar'), 'qwerty', new Reference('foo')), $definition->getArguments());
154+
}
128155
}
129156

130157
class NoConstructor

‎src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
3333
use Symfony\Component\DependencyInjection\Loader\ClosureLoader;
3434
use Symfony\Component\DependencyInjection\Reference;
35+
use Symfony\Component\DependencyInjection\Tests\Fixtures\SimilarArgumentsDummy;
3536
use Symfony\Component\DependencyInjection\TypedReference;
3637
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
3738
use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag;
@@ -1270,6 +1271,30 @@ public function testParameterWithMixedCase()
12701271

12711272
$this->assertSame('bar', $container->get('foo')->foo);
12721273
}
1274+
1275+
public function testArgumentsHaveHigherPriorityThanBindings()
1276+
{
1277+
$container = new ContainerBuilder();
1278+
$container->register('class.via.bindings', CaseSensitiveClass::class)->setArguments(array(
1279+
'via-bindings',
1280+
));
1281+
$container->register('class.via.argument', CaseSensitiveClass::class)->setArguments(array(
1282+
'via-argument',
1283+
));
1284+
$container->register('foo', SimilarArgumentsDummy::class)->setPublic(true)->setBindings(array(
1285+
CaseSensitiveClass::class => new Reference('class.via.bindings'),
1286+
'$token' => '1234',
1287+
))->setArguments(array(
1288+
'$class1' => new Reference('class.via.argument'),
1289+
));
1290+
1291+
$this->assertSame(array('service_container', 'class.via.bindings', 'class.via.argument', 'foo', 'Psr\Container\ContainerInterface', 'Symfony\Component\DependencyInjection\ContainerInterface'), $container->getServiceIds());
1292+
1293+
$container->compile();
1294+
1295+
$this->assertSame('via-argument', $container->get('foo')->class1->identifier);
1296+
$this->assertSame('via-bindings', $container->get('foo')->class2->identifier);
1297+
}
12731298
}
12741299

12751300
class FooClass

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/CaseSensitiveClass.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,10 @@
1313

1414
class CaseSensitiveClass
1515
{
16+
public $identifier;
17+
18+
public function __construct($identifier = null)
19+
{
20+
$this->identifier = $identifier;
21+
}
1622
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/SimilarArgumentsDummy.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
13+
14+
class SimilarArgumentsDummy
15+
{
16+
public $class1;
17+
public $class2;
18+
19+
public function __construct(CaseSensitiveClass $class1, string $token, CaseSensitiveClass $class2)
20+
{
21+
$this->class1 = $class1;
22+
$this->class2 = $class2;
23+
}
24+
}

0 commit comments

Comments
 (0)