Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ee10bf2

Browse filesBrowse files
fabpotfabpot
committed
bug #22254 [DI] Don't use auto-registered services to populate type-candidates (nicolas-grekas)
This PR was merged into the 2.8 branch. Discussion ---------- [DI] Don't use auto-registered services to populate type-candidates | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no (every bug fix is a bc break, isn't it?) | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22162, ~~#21658~~ | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> Alternative to #22170 and ~~#21665~~. The core issue fixed here is that auto-registered services should *not* be used as type-candidates. The culprit is this line: `$this->populateAvailableType($argumentId, $argumentDefinition);` Doing so creates a series of wtf-issues (the linked ones), with no simple fixes (the linked PRs are just dealing with the simplest cases, but the real fix would require a reboot of autowiring for every newly discovered types.) The other changes accommodate for the removal of the population of the `types` property, and fix a few other issues found along the way: - variadic constructors - empty strings injection - tail args removal when they match the existing defaults Commits ------- 992c677 [DI] Don't use auto-registered services to populate type-candidates
2 parents 80cea46 + 992c677 commit ee10bf2
Copy full SHA for ee10bf2

File tree

Expand file treeCollapse file tree

2 files changed

+32
-45
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+32
-45
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
+32-24Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class AutowirePass implements CompilerPassInterface
2828
private $definedTypes = array();
2929
private $types;
3030
private $notGuessableTypes = array();
31-
private $usedTypes = array();
31+
private $autowired = array();
3232

3333
/**
3434
* {@inheritdoc}
@@ -45,15 +45,6 @@ public function process(ContainerBuilder $container)
4545
$this->completeDefinition($id, $definition);
4646
}
4747
}
48-
49-
foreach ($this->usedTypes as $type => $id) {
50-
if (isset($this->usedTypes[$type]) && isset($this->notGuessableTypes[$type])) {
51-
$classOrInterface = class_exists($type) ? 'class' : 'interface';
52-
$matchingServices = implode(', ', $this->types[$type]);
53-
54-
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));
55-
}
56-
}
5748
} catch (\Exception $e) {
5849
} catch (\Throwable $e) {
5950
}
@@ -66,7 +57,7 @@ public function process(ContainerBuilder $container)
6657
$this->definedTypes = array();
6758
$this->types = null;
6859
$this->notGuessableTypes = array();
69-
$this->usedTypes = array();
60+
$this->autowired = array();
7061

7162
if (isset($e)) {
7263
throw $e;
@@ -92,47 +83,56 @@ private function completeDefinition($id, Definition $definition)
9283
if (!$constructor = $reflectionClass->getConstructor()) {
9384
return;
9485
}
86+
$parameters = $constructor->getParameters();
87+
if (method_exists('ReflectionMethod', 'isVariadic') && $constructor->isVariadic()) {
88+
array_pop($parameters);
89+
}
9590

9691
$arguments = $definition->getArguments();
97-
foreach ($constructor->getParameters() as $index => $parameter) {
92+
foreach ($parameters as $index => $parameter) {
9893
if (array_key_exists($index, $arguments) && '' !== $arguments[$index]) {
9994
continue;
10095
}
10196

10297
try {
10398
if (!$typeHint = $parameter->getClass()) {
99+
if (isset($arguments[$index])) {
100+
continue;
101+
}
102+
104103
// no default value? Then fail
105104
if (!$parameter->isOptional()) {
106105
throw new RuntimeException(sprintf('Unable to autowire argument index %d ($%s) for the service "%s". If this is an object, give it a type-hint. Otherwise, specify this argument\'s value explicitly.', $index, $parameter->name, $id));
107106
}
108107

109-
if (!array_key_exists($index, $arguments)) {
110-
// specifically pass the default value
111-
$arguments[$index] = $parameter->getDefaultValue();
112-
}
108+
// specifically pass the default value
109+
$arguments[$index] = $parameter->getDefaultValue();
113110

114111
continue;
115112
}
116113

114+
if (isset($this->autowired[$typeHint->name])) {
115+
return $this->autowired[$typeHint->name] ? new Reference($this->autowired[$typeHint->name]) : null;
116+
}
117+
117118
if (null === $this->types) {
118119
$this->populateAvailableTypes();
119120
}
120121

121122
if (isset($this->types[$typeHint->name]) && !isset($this->notGuessableTypes[$typeHint->name])) {
122123
$value = new Reference($this->types[$typeHint->name]);
123-
$this->usedTypes[$typeHint->name] = $id;
124124
} else {
125125
try {
126126
$value = $this->createAutowiredDefinition($typeHint, $id);
127-
$this->usedTypes[$typeHint->name] = $id;
128127
} catch (RuntimeException $e) {
129-
if ($parameter->allowsNull()) {
130-
$value = null;
131-
} elseif ($parameter->isDefaultValueAvailable()) {
128+
if ($parameter->isDefaultValueAvailable()) {
132129
$value = $parameter->getDefaultValue();
130+
} elseif ($parameter->allowsNull()) {
131+
$value = null;
133132
} else {
134133
throw $e;
135134
}
135+
$this->autowired[$typeHint->name] = false;
136136
}
137137
}
138138
} catch (\ReflectionException $e) {
@@ -148,6 +148,16 @@ private function completeDefinition($id, Definition $definition)
148148
$arguments[$index] = $value;
149149
}
150150

151+
if ($parameters && !isset($arguments[++$index])) {
152+
while (0 <= --$index) {
153+
$parameter = $parameters[$index];
154+
if (!$parameter->isDefaultValueAvailable() || $parameter->getDefaultValue() !== $arguments[$index]) {
155+
break;
156+
}
157+
unset($arguments[$index]);
158+
}
159+
}
160+
151161
// it's possible index 1 was set, then index 0, then 2, etc
152162
// make sure that we re-order so they're injected as expected
153163
ksort($arguments);
@@ -252,13 +262,11 @@ private function createAutowiredDefinition(\ReflectionClass $typeHint, $id)
252262
throw new RuntimeException(sprintf('Unable to autowire argument of type "%s" for the service "%s". No services were found matching this %s and it cannot be auto-registered.', $typeHint->name, $id, $classOrInterface));
253263
}
254264

255-
$argumentId = sprintf('autowired.%s', $typeHint->name);
265+
$this->autowired[$typeHint->name] = $argumentId = sprintf('autowired.%s', $typeHint->name);
256266

257267
$argumentDefinition = $this->container->register($argumentId, $typeHint->name);
258268
$argumentDefinition->setPublic(false);
259269

260-
$this->populateAvailableType($argumentId, $argumentDefinition);
261-
262270
try {
263271
$this->completeDefinition($argumentId, $argumentDefinition);
264272
} catch (RuntimeException $e) {

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
-21Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,6 @@ public function testOptionalScalarArgsNotPassedIfLast()
422422
array(
423423
new Reference('a'),
424424
new Reference('lille'),
425-
// third arg shouldn't *need* to be passed
426-
// but that's hard to "pull of" with autowiring, so
427-
// this assumes passing the default val is ok
428-
'some_val',
429425
),
430426
$definition->getArguments()
431427
);
@@ -462,23 +458,6 @@ public function testEmptyStringIsKept()
462458
$this->assertEquals(array(new Reference('a'), '', new Reference('lille')), $container->getDefinition('foo')->getArguments());
463459
}
464460

465-
/**
466-
* @dataProvider provideAutodiscoveredAutowiringOrder
467-
*
468-
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
469-
* @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).
470-
*/
471-
public function testAutodiscoveredAutowiringOrder($class)
472-
{
473-
$container = new ContainerBuilder();
474-
475-
$container->register('a', __NAMESPACE__.'\\'.$class)
476-
->setAutowired(true);
477-
478-
$pass = new AutowirePass();
479-
$pass->process($container);
480-
}
481-
482461
public function provideAutodiscoveredAutowiringOrder()
483462
{
484463
return array(

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.