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 a6bfe1c

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[DI] Remove skipping magic for autowired methods
1 parent eb6f3f7 commit a6bfe1c
Copy full SHA for a6bfe1c

File tree

4 files changed

+103
-159
lines changed
Filter options

4 files changed

+103
-159
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
+33-45Lines changed: 33 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,6 @@
2626
*/
2727
class AutowirePass extends AbstractRecursivePass
2828
{
29-
/**
30-
* @internal
31-
*/
32-
const MODE_REQUIRED = 1;
33-
34-
/**
35-
* @internal
36-
*/
37-
const MODE_OPTIONAL = 2;
38-
3929
private $definedTypes = array();
4030
private $types;
4131
private $ambiguousServiceTypes = array();
@@ -170,13 +160,6 @@ private function getMethodsToAutowire(\ReflectionClass $reflectionClass)
170160
}
171161

172162
foreach ($reflectionClass->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED) as $reflectionMethod) {
173-
if ($reflectionMethod->isStatic()) {
174-
continue;
175-
}
176-
if ($reflectionMethod->isAbstract() && !$reflectionMethod->getNumberOfParameters()) {
177-
$methodsToAutowire[strtolower($reflectionMethod->name)] = $reflectionMethod;
178-
continue;
179-
}
180163
$r = $reflectionMethod;
181164

182165
while (true) {
@@ -225,17 +208,21 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
225208
}
226209
}
227210

228-
$arguments = $this->autowireMethod($reflectionMethod, $arguments, self::MODE_REQUIRED);
211+
$arguments = $this->autowireMethod($reflectionMethod, $arguments);
229212

230213
if ($arguments !== $call[1]) {
231214
$methodCalls[$i][1] = $arguments;
232215
}
233216
}
234217

235218
foreach ($autowiredMethods as $lcMethod => $reflectionMethod) {
236-
if ($reflectionMethod->isPublic() && $arguments = $this->autowireMethod($reflectionMethod, array(), self::MODE_OPTIONAL)) {
237-
$methodCalls[] = array($reflectionMethod->name, $arguments);
219+
if (!$reflectionMethod->getNumberOfParameters()) {
220+
continue; // skip getters
238221
}
222+
if (!$reflectionMethod->isPublic()) {
223+
throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s::%s() must be public.', $this->currentId, $reflectionClass->name, $reflectionMethod->name));
224+
}
225+
$methodCalls[] = array($reflectionMethod->name, $this->autowireMethod($reflectionMethod, array()));
239226
}
240227

241228
return $methodCalls;
@@ -246,20 +233,24 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
246233
*
247234
* @param \ReflectionMethod $reflectionMethod
248235
* @param array $arguments
249-
* @param int $mode
250236
*
251237
* @return array The autowired arguments
252238
*
253239
* @throws RuntimeException
254240
*/
255-
private function autowireMethod(\ReflectionMethod $reflectionMethod, array $arguments, $mode)
241+
private function autowireMethod(\ReflectionMethod $reflectionMethod, array $arguments)
256242
{
257-
$didAutowire = false; // Whether any arguments have been autowired or not
243+
$isConstructor = $reflectionMethod->isConstructor();
244+
245+
if (!$isConstructor && !$arguments && !$reflectionMethod->getNumberOfRequiredParameters()) {
246+
throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s::%s() has only optional arguments, thus must be wired explicitly.', $this->currentId, $reflectionMethod->class, $reflectionMethod->name));
247+
}
248+
258249
foreach ($reflectionMethod->getParameters() as $index => $parameter) {
259250
if (array_key_exists($index, $arguments) && '' !== $arguments[$index]) {
260251
continue;
261252
}
262-
if (self::MODE_OPTIONAL === $mode && $parameter->isOptional() && !array_key_exists($index, $arguments)) {
253+
if (!$isConstructor && $parameter->isOptional() && !array_key_exists($index, $arguments)) {
263254
break;
264255
}
265256
if (method_exists($parameter, 'isVariadic') && $parameter->isVariadic()) {
@@ -271,11 +262,7 @@ private function autowireMethod(\ReflectionMethod $reflectionMethod, array $argu
271262
if (!$typeName) {
272263
// no default value? Then fail
273264
if (!$parameter->isOptional()) {
274-
if (self::MODE_REQUIRED === $mode) {
275-
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));
276-
}
277-
278-
return array();
265+
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));
279266
}
280267

281268
if (!array_key_exists($index, $arguments)) {
@@ -287,31 +274,24 @@ private function autowireMethod(\ReflectionMethod $reflectionMethod, array $argu
287274
}
288275

289276
if ($value = $this->getAutowiredReference($typeName)) {
290-
$didAutowire = true;
291277
$this->usedTypes[$typeName] = $this->currentId;
292278
} elseif ($parameter->isDefaultValueAvailable()) {
293279
$value = $parameter->getDefaultValue();
294280
} elseif ($parameter->allowsNull()) {
295281
$value = null;
296-
} elseif (self::MODE_REQUIRED === $mode) {
282+
} else {
297283
if ($classOrInterface = class_exists($typeName, false) ? 'class' : (interface_exists($typeName, false) ? 'interface' : null)) {
298284
$message = 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.', $typeName, $this->currentId, $classOrInterface);
299285
} else {
300286
$message = sprintf('Cannot autowire argument $%s of method %s::%s() for service "%s": Class %s does not exist.', $parameter->name, $reflectionMethod->class, $reflectionMethod->name, $this->currentId, $typeName);
301287
}
302288

303289
throw new RuntimeException($message);
304-
} else {
305-
return array();
306290
}
307291

308292
$arguments[$index] = $value;
309293
}
310294

311-
if (self::MODE_REQUIRED !== $mode && !$didAutowire) {
312-
return array();
313-
}
314-
315295
// it's possible index 1 was set, then index 0, then 2, etc
316296
// make sure that we re-order so they're injected as expected
317297
ksort($arguments);
@@ -330,16 +310,24 @@ private function autowireMethod(\ReflectionMethod $reflectionMethod, array $argu
330310
private function autowireOverridenGetters(array $overridenGetters, array $autowiredMethods)
331311
{
332312
foreach ($autowiredMethods as $lcMethod => $reflectionMethod) {
333-
if (isset($overridenGetters[$lcMethod])
334-
|| !method_exists($reflectionMethod, 'getReturnType')
335-
|| 0 !== $reflectionMethod->getNumberOfParameters()
336-
|| $reflectionMethod->isFinal()
337-
|| $reflectionMethod->returnsReference()
338-
|| !($typeName = InheritanceProxyHelper::getTypeHint($reflectionMethod, null, true))
339-
|| !($typeRef = $this->getAutowiredReference($typeName))
340-
) {
313+
if (isset($overridenGetters[$lcMethod]) || $reflectionMethod->getNumberOfParameters() || $reflectionMethod->isConstructor()) {
341314
continue;
342315
}
316+
if (!$typeName = InheritanceProxyHelper::getTypeHint($reflectionMethod, null, true)) {
317+
$typeName = InheritanceProxyHelper::getTypeHint($reflectionMethod);
318+
319+
throw new RuntimeException(sprintf('Cannot autowire service "%s": getter %s::%s() must%s be given a return value explicitly.', $this->currentId, $reflectionMethod->class, $reflectionMethod->name, $typeName ? '' : ' have a return-type hint or'));
320+
}
321+
322+
if (!$typeRef = $this->getAutowiredReference($typeName)) {
323+
if ($classOrInterface = class_exists($typeName, false) ? 'class' : (interface_exists($typeName, false) ? 'interface' : null)) {
324+
$message = sprintf('Unable to autowire return type "%s" for service "%s". No services were found matching this %s and it cannot be auto-registered.', $typeName, $this->currentId, $classOrInterface);
325+
} else {
326+
$message = sprintf('Cannot autowire return type of getter %s::%s() for service "%s": Class %s does not exist.', $reflectionMethod->class, $reflectionMethod->name, $this->currentId, $typeName);
327+
}
328+
329+
throw new RuntimeException($message);
330+
}
343331

344332
$overridenGetters[$lcMethod] = $typeRef;
345333
$this->usedTypes[$typeName] = $this->currentId;

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
+70-66Lines changed: 70 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\DependencyInjection\Compiler\AutowirePass;
1616
use Symfony\Component\DependencyInjection\ContainerBuilder;
17+
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
1718
use Symfony\Component\DependencyInjection\Reference;
18-
use Symfony\Component\DependencyInjection\Tests\Fixtures\AbstractGetterOverriding;
1919
use Symfony\Component\DependencyInjection\Tests\Fixtures\includes\FooVariadic;
2020
use Symfony\Component\DependencyInjection\Tests\Fixtures\GetterOverriding;
2121
use Symfony\Component\DependencyInjection\TypedReference;
@@ -535,27 +535,6 @@ public function testTypedReference()
535535
$this->assertSame(A::class, $container->getDefinition('autowired.'.A::class)->getClass());
536536
}
537537

538-
/**
539-
* @requires PHP 7.0
540-
*/
541-
public function testAbstractGetterOverriding()
542-
{
543-
$container = new ContainerBuilder();
544-
545-
$container
546-
->register('getter_overriding', AbstractGetterOverriding::class)
547-
->setAutowired(true)
548-
;
549-
550-
$pass = new AutowirePass();
551-
$pass->process($container);
552-
553-
$overridenGetters = $container->getDefinition('getter_overriding')->getOverriddenGetters();
554-
$this->assertEquals(array(
555-
'abstractgetfoo' => new Reference('autowired.Symfony\Component\DependencyInjection\Tests\Compiler\Foo'),
556-
), $overridenGetters);
557-
}
558-
559538
/**
560539
* @requires PHP 7.1
561540
*/
@@ -707,6 +686,44 @@ public function provideAutodiscoveredAutowiringOrder()
707686
array('CannotBeAutowiredReverseOrder'),
708687
);
709688
}
689+
690+
/**
691+
* @dataProvider provideNotWireableCalls
692+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
693+
*/
694+
public function testNotWireableCalls($method, $expectedMsg)
695+
{
696+
$container = new ContainerBuilder();
697+
698+
$foo = $container->register('foo', NotWireable::class)->setAutowired(true);
699+
700+
if ($method) {
701+
$foo->addMethodCall($method, array());
702+
}
703+
704+
$pass = new AutowirePass();
705+
706+
if (method_exists($this, 'expectException')) {
707+
$this->expectException(RuntimeException::class);
708+
$this->expectExceptionMessage($expectedMsg);
709+
} else {
710+
$this->setExpectedException(RuntimeException::class, $expectedMsg);
711+
}
712+
713+
$pass->process($container);
714+
}
715+
716+
public function provideNotWireableCalls()
717+
{
718+
return array(
719+
array('setNotAutowireable', 'Cannot autowire argument $n of method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setNotAutowireable() for service "foo": Class Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass does not exist.'),
720+
array('setBar', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setBar() has only optional arguments, thus must be wired explicitly.'),
721+
array('setOptionalNotAutowireable', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalNotAutowireable() has only optional arguments, thus must be wired explicitly.'),
722+
array('setOptionalNoTypeHint', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalNoTypeHint() has only optional arguments, thus must be wired explicitly.'),
723+
array('setOptionalArgNoAutowireable', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalArgNoAutowireable() has only optional arguments, thus must be wired explicitly.'),
724+
array(null, 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setProtectedMethod() must be public.'),
725+
);
726+
}
710727
}
711728

712729
class Foo
@@ -917,43 +934,6 @@ public function setDependencies(Foo $foo, A $a)
917934
// should be called
918935
}
919936

920-
/**
921-
* @required*/
922-
public function setBar()
923-
{
924-
// should not be called
925-
}
926-
927-
/** @required <tab> prefix is on purpose */
928-
public function setNotAutowireable(NotARealClass $n)
929-
{
930-
// should not be called
931-
}
932-
933-
/** @required */
934-
public function setArgCannotAutowire($foo)
935-
{
936-
// should not be called
937-
}
938-
939-
/** @required */
940-
public function setOptionalNotAutowireable(NotARealClass $n = null)
941-
{
942-
// should not be called
943-
}
944-
945-
/** @required */
946-
public function setOptionalNoTypeHint($foo = null)
947-
{
948-
// should not be called
949-
}
950-
951-
/** @required */
952-
public function setOptionalArgNoAutowireable($other = 'default_val')
953-
{
954-
// should not be called
955-
}
956-
957937
/** {@inheritdoc} */
958938
public function setWithCallsConfigured(A $a)
959939
{
@@ -965,12 +945,8 @@ public function notASetter(A $a)
965945
// should be called only when explicitly specified
966946
}
967947

968-
/** @required */
969-
protected function setProtectedMethod(A $a)
970-
{
971-
// should not be called
972-
}
973-
948+
/**
949+
* @required*/
974950
public function setChildMethodWithoutDocBlock(A $a)
975951
{
976952
}
@@ -989,7 +965,7 @@ public function notASetter(A $a)
989965
// @required should be ignored when the child does not add @inheritdoc
990966
}
991967

992-
/** @required */
968+
/** @required <tab> prefix is on purpose */
993969
public function setWithCallsConfigured(A $a)
994970
{
995971
}
@@ -1012,3 +988,31 @@ public function setMultipleInstancesForOneArg(CollisionInterface $collision)
1012988
// should throw an exception
1013989
}
1014990
}
991+
992+
class NotWireable
993+
{
994+
public function setNotAutowireable(NotARealClass $n)
995+
{
996+
}
997+
998+
public function setBar()
999+
{
1000+
}
1001+
1002+
public function setOptionalNotAutowireable(NotARealClass $n = null)
1003+
{
1004+
}
1005+
1006+
public function setOptionalNoTypeHint($foo = null)
1007+
{
1008+
}
1009+
1010+
public function setOptionalArgNoAutowireable($other = 'default_val')
1011+
{
1012+
}
1013+
1014+
/** @required */
1015+
protected function setProtectedMethod(A $a)
1016+
{
1017+
}
1018+
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Fixtures/AbstractGetterOverriding.php
-20Lines changed: 0 additions & 20 deletions
This file was deleted.

0 commit comments

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