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 b9b6ebd

Browse filesBrowse files
fabpotfabpot
committed
feature #21404 [DI] Generalize constructor autowiring to partial method calls (nicolas-grekas)
This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Generalize constructor autowiring to partial method calls | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - When autowiring is enabled: currently, the constructor can be partially wired, and autowiring will complete the remaining missing arguments. But there is no reason this should only apply to the constructor. This PR fixes this inconsistency by looking at all method calls, and wire missing arguments in the same way. Commits ------- 29c2fd5 [DI] Generalize constructor autowiring to partial method calls
2 parents 140e847 + 29c2fd5 commit b9b6ebd
Copy full SHA for b9b6ebd

File tree

2 files changed

+115
-51
lines changed
Filter options

2 files changed

+115
-51
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
+95-47Lines changed: 95 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,6 @@ public static function createResourceForClass(\ReflectionClass $reflectionClass)
6161
{
6262
$metadata = array();
6363

64-
if ($constructor = $reflectionClass->getConstructor()) {
65-
$metadata['__construct'] = self::getResourceMetadataForMethod($constructor);
66-
}
67-
6864
foreach ($reflectionClass->getMethods(\ReflectionMethod::IS_PUBLIC) as $reflectionMethod) {
6965
if (!$reflectionMethod->isStatic()) {
7066
$metadata[$reflectionMethod->name] = self::getResourceMetadataForMethod($reflectionMethod);
@@ -91,39 +87,45 @@ protected function processValue($value, $isRoot = false)
9187
$this->container->addResource(static::createResourceForClass($reflectionClass));
9288
}
9389

94-
$methodsCalled = array();
95-
foreach ($value->getMethodCalls() as $methodCall) {
96-
$methodsCalled[strtolower($methodCall[0])] = true;
90+
$autowiredMethods = $this->getMethodsToAutowire($reflectionClass, $autowiredMethods);
91+
$methodCalls = $value->getMethodCalls();
92+
93+
if ($constructor = $reflectionClass->getConstructor()) {
94+
array_unshift($methodCalls, array($constructor->name, $value->getArguments()));
95+
} elseif ($value->getArguments()) {
96+
throw new RuntimeException(sprintf('Cannot autowire service "%s": class %s has no constructor but arguments are defined.', $this->currentId, $reflectionClass->name, $method));
9797
}
9898

99-
foreach ($this->getMethodsToAutowire($reflectionClass, $autowiredMethods) as $reflectionMethod) {
100-
if (!isset($methodsCalled[strtolower($reflectionMethod->name)])) {
101-
$this->autowireMethod($value, $reflectionMethod);
99+
$methodCalls = $this->autowireMethodCalls($reflectionClass, $methodCalls, $autowiredMethods);
100+
101+
if ($constructor) {
102+
list(, $arguments) = array_shift($methodCalls);
103+
104+
if ($arguments !== $value->getArguments()) {
105+
$value->setArguments($arguments);
102106
}
103107
}
104108

109+
if ($methodCalls !== $value->getMethodCalls()) {
110+
$value->setMethodCalls($methodCalls);
111+
}
112+
105113
return parent::processValue($value, $isRoot);
106114
}
107115

108116
/**
109117
* Gets the list of methods to autowire.
110118
*
111119
* @param \ReflectionClass $reflectionClass
112-
* @param string[] $configuredAutowiredMethods
120+
* @param string[] $autowiredMethods
113121
*
114122
* @return \ReflectionMethod[]
115123
*/
116-
private function getMethodsToAutowire(\ReflectionClass $reflectionClass, array $configuredAutowiredMethods)
124+
private function getMethodsToAutowire(\ReflectionClass $reflectionClass, array $autowiredMethods)
117125
{
118126
$found = array();
119127
$regexList = array();
120-
121-
// Always try to autowire the constructor
122-
if (in_array('__construct', $configuredAutowiredMethods, true)) {
123-
$autowiredMethods = $configuredAutowiredMethods;
124-
} else {
125-
$autowiredMethods = array_merge(array('__construct'), $configuredAutowiredMethods);
126-
}
128+
$methodsToAutowire = array();
127129

128130
foreach ($autowiredMethods as $pattern) {
129131
$regexList[] = '/^'.str_replace('\*', '.*', preg_quote($pattern, '/')).'$/i';
@@ -137,36 +139,82 @@ private function getMethodsToAutowire(\ReflectionClass $reflectionClass, array $
137139
foreach ($regexList as $k => $regex) {
138140
if (preg_match($regex, $reflectionMethod->name)) {
139141
$found[] = $autowiredMethods[$k];
140-
yield $reflectionMethod;
141-
142+
$methodsToAutowire[strtolower($reflectionMethod->name)] = $reflectionMethod;
142143
continue 2;
143144
}
144145
}
146+
147+
if ($reflectionMethod->isConstructor()) {
148+
$methodsToAutowire[strtolower($reflectionMethod->name)] = $reflectionMethod;
149+
}
145150
}
146151

147-
if ($notFound = array_diff($configuredAutowiredMethods, $found)) {
152+
if ($notFound = array_diff($autowiredMethods, $found)) {
148153
$compiler = $this->container->getCompiler();
149154
$compiler->addLogMessage($compiler->getLoggingFormatter()->formatUnusedAutowiringPatterns($this, $this->currentId, $notFound));
150155
}
156+
157+
return $methodsToAutowire;
158+
}
159+
160+
/**
161+
* @param \ReflectionClass $reflectionClass
162+
* @param array $methodCalls
163+
* @param \ReflectionMethod[] $autowiredMethods
164+
*
165+
* @return array
166+
*/
167+
private function autowireMethodCalls(\ReflectionClass $reflectionClass, array $methodCalls, array $autowiredMethods)
168+
{
169+
$parameterBag = $this->container->getParameterBag();
170+
171+
foreach ($methodCalls as $i => $call) {
172+
list($method, $arguments) = $call;
173+
$method = $parameterBag->resolveValue($method);
174+
175+
if (isset($autowiredMethods[$lcMethod = strtolower($method)])) {
176+
$reflectionMethod = $autowiredMethods[$lcMethod];
177+
unset($autowiredMethods[$lcMethod]);
178+
} else {
179+
if (!$reflectionClass->hasMethod($method)) {
180+
throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s::%s() does not exist.', $this->currentId, $reflectionClass->name, $method));
181+
}
182+
$reflectionMethod = $reflectionClass->getMethod($method);
183+
if (!$reflectionMethod->isPublic()) {
184+
throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s::%s() must be public.', $this->currentId, $reflectionClass->name, $method));
185+
}
186+
}
187+
188+
$arguments = $this->autowireMethod($reflectionMethod, $arguments, true);
189+
190+
if ($arguments !== $call[1]) {
191+
$methodCalls[$i][1] = $arguments;
192+
}
193+
}
194+
195+
foreach ($autowiredMethods as $reflectionMethod) {
196+
if ($arguments = $this->autowireMethod($reflectionMethod, array(), false)) {
197+
$methodCalls[] = array($reflectionMethod->name, $arguments);
198+
}
199+
}
200+
201+
return $methodCalls;
151202
}
152203

153204
/**
154205
* Autowires the constructor or a setter.
155206
*
156-
* @param Definition $definition
157207
* @param \ReflectionMethod $reflectionMethod
208+
* @param array $arguments
209+
* @param bool $mustAutowire
210+
*
211+
* @return array The autowired arguments
158212
*
159213
* @throws RuntimeException
160214
*/
161-
private function autowireMethod(Definition $definition, \ReflectionMethod $reflectionMethod)
215+
private function autowireMethod(\ReflectionMethod $reflectionMethod, array $arguments, $mustAutowire)
162216
{
163-
if ($isConstructor = $reflectionMethod->isConstructor()) {
164-
$arguments = $definition->getArguments();
165-
} else {
166-
$arguments = array();
167-
}
168-
169-
$addMethodCall = false; // Whether the method should be added to the definition as a call or as arguments
217+
$didAutowire = false; // Whether any arguments have been autowired or not
170218
foreach ($reflectionMethod->getParameters() as $index => $parameter) {
171219
if (array_key_exists($index, $arguments) && '' !== $arguments[$index]) {
172220
continue;
@@ -176,11 +224,11 @@ private function autowireMethod(Definition $definition, \ReflectionMethod $refle
176224
if (!$typeHint = $parameter->getClass()) {
177225
// no default value? Then fail
178226
if (!$parameter->isOptional()) {
179-
if ($isConstructor) {
180-
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, $this->currentId));
227+
if ($mustAutowire) {
228+
throw new RuntimeException(sprintf('Cannot autowire service "%s": argument $%s of method %s::%s() must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $reflectionMethod->class, $reflectionMethod->name));
181229
}
182230

183-
return;
231+
return array();
184232
}
185233

186234
// specifically pass the default value
@@ -195,35 +243,35 @@ private function autowireMethod(Definition $definition, \ReflectionMethod $refle
195243

196244
if (isset($this->types[$typeHint->name])) {
197245
$value = new Reference($this->types[$typeHint->name]);
198-
$addMethodCall = true;
246+
$didAutowire = true;
199247
} else {
200248
try {
201249
$value = $this->createAutowiredDefinition($typeHint);
202-
$addMethodCall = true;
250+
$didAutowire = true;
203251
} catch (RuntimeException $e) {
204252
if ($parameter->allowsNull()) {
205253
$value = null;
206254
} elseif ($parameter->isDefaultValueAvailable()) {
207255
$value = $parameter->getDefaultValue();
208256
} else {
209-
// The exception code is set to 1 if the exception must be thrown even if it's a setter
210-
if (1 === $e->getCode() || $isConstructor) {
257+
// The exception code is set to 1 if the exception must be thrown even if it's an optional setter
258+
if (1 === $e->getCode() || $mustAutowire) {
211259
throw $e;
212260
}
213261

214-
return;
262+
return array();
215263
}
216264
}
217265
}
218266
} catch (\ReflectionException $e) {
219267
// Typehint against a non-existing class
220268

221269
if (!$parameter->isDefaultValueAvailable()) {
222-
if ($isConstructor) {
223-
throw new RuntimeException(sprintf('Cannot autowire argument %s for %s because the type-hinted class does not exist (%s).', $index + 1, $definition->getClass(), $e->getMessage()), 0, $e);
270+
if ($mustAutowire) {
271+
throw new RuntimeException(sprintf('Cannot autowire argument $%s of method %s::%s() for service "%s": %s.', $parameter->name, $reflectionMethod->class, $reflectionMethod->name, $this->currentId, $e->getMessage()), 0, $e);
224272
}
225273

226-
return;
274+
return array();
227275
}
228276

229277
$value = $parameter->getDefaultValue();
@@ -232,15 +280,15 @@ private function autowireMethod(Definition $definition, \ReflectionMethod $refle
232280
$arguments[$index] = $value;
233281
}
234282

283+
if (!$mustAutowire && !$didAutowire) {
284+
return array();
285+
}
286+
235287
// it's possible index 1 was set, then index 0, then 2, etc
236288
// make sure that we re-order so they're injected as expected
237289
ksort($arguments);
238290

239-
if ($isConstructor) {
240-
$definition->setArguments($arguments);
241-
} elseif ($addMethodCall) {
242-
$definition->addMethodCall($reflectionMethod->name, $arguments);
243-
}
291+
return $arguments;
244292
}
245293

246294
/**

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
+20-4Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ public function testDontTriggerAutowiring()
289289

290290
/**
291291
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
292-
* @expectedExceptionMessage Cannot autowire argument 2 for Symfony\Component\DependencyInjection\Tests\Compiler\BadTypeHintedArgument because the type-hinted class does not exist (Class Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass does not exist).
292+
* @expectedExceptionMessage Cannot autowire argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadTypeHintedArgument::__construct() for service "a": Class Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass does not exist.
293293
*/
294294
public function testClassNotFoundThrowsException()
295295
{
@@ -304,7 +304,7 @@ public function testClassNotFoundThrowsException()
304304

305305
/**
306306
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
307-
* @expectedExceptionMessage Cannot autowire argument 2 for Symfony\Component\DependencyInjection\Tests\Compiler\BadParentTypeHintedArgument because the type-hinted class does not exist (Class Symfony\Component\DependencyInjection\Tests\Compiler\OptionalServiceClass does not exist).
307+
* @expectedExceptionMessage Cannot autowire argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadParentTypeHintedArgument::__construct() for service "a": Class Symfony\Component\DependencyInjection\Tests\Compiler\OptionalServiceClass does not exist.
308308
*/
309309
public function testParentClassNotFoundThrowsException()
310310
{
@@ -363,7 +363,7 @@ public function testSomeSpecificArgumentsAreSet()
363363

364364
/**
365365
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
366-
* @expectedExceptionMessage Unable to autowire argument index 1 ($foo) for the service "arg_no_type_hint". If this is an object, give it a type-hint. Otherwise, specify this argument's value explicitly.
366+
* @expectedExceptionMessage Cannot autowire service "arg_no_type_hint": argument $foo of method Symfony\Component\DependencyInjection\Tests\Compiler\MultipleArguments::__construct() must have a type-hint or be given a value explicitly.
367367
*/
368368
public function testScalarArgsCannotBeAutowired()
369369
{
@@ -382,7 +382,7 @@ public function testScalarArgsCannotBeAutowired()
382382

383383
/**
384384
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
385-
* @expectedExceptionMessage Unable to autowire argument index 1 ($foo) for the service "not_really_optional_scalar". If this is an object, give it a type-hint. Otherwise, specify this argument's value explicitly.
385+
* @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.
386386
*/
387387
public function testOptionalScalarNotReallyOptionalThrowException()
388388
{
@@ -593,6 +593,22 @@ public function testLogUnusedPatterns()
593593

594594
$this->assertEquals(array(AutowirePass::class.': Autowiring\'s patterns "not", "exist*" for service "foo" don\'t match any method.'), $container->getCompiler()->getLog());
595595
}
596+
597+
public function testPartialMethodCalls()
598+
{
599+
$container = new ContainerBuilder();
600+
601+
$container->register('a', A::class);
602+
$container->register('foo', Foo::class);
603+
$definition = $container->register('bar', SetterInjection::class);
604+
$definition->setAutowired(true);
605+
$definition->addMethodCall('setDependencies', array(new Reference('foo')));
606+
607+
$pass = new AutowirePass();
608+
$pass->process($container);
609+
610+
$this->assertEquals(array(array('setDependencies', array(new Reference('foo'), new Reference('a')))), $container->getDefinition('bar')->getMethodCalls());
611+
}
596612
}
597613

598614
class Foo

0 commit comments

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