8000 [DI] Remove skipping magic for autowired methods · soyuka/symfony@a6bfe1c · GitHub
[go: up one dir, main page]
Skip to content

Commit a6bfe1c

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

File tree

4 files changed

+103
-159
lines changed

4 files changed

+103
-159
lines changed

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

Lines 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 getMethodsToAut ED4F owire(\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

Lines 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

Lines changed: 0 additions & 20 deletions
This file was deleted.

0 commit comments

Comments
 (0)
0