From bf91eda705c2083ab059e93f3aef1ef99a902b4a Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 2 Nov 2016 10:24:10 +0100 Subject: [PATCH] Revert "feature #17608 [DependencyInjection] Autowiring: add setter injection support (dunglas)" This reverts commit 7eab6b92042eee1afd32e37d9fb8583cc9dec100, reversing changes made to 35f201f9d6509f194081b1b04597ddb9d6bd54c6. --- CHANGELOG-3.2.md | 1 - .../DependencyInjection/CHANGELOG.md | 1 - .../Compiler/AutowirePass.php | 68 ++-------- .../Tests/Compiler/AutowirePassTest.php | 119 ------------------ 4 files changed, 9 insertions(+), 180 deletions(-) diff --git a/CHANGELOG-3.2.md b/CHANGELOG-3.2.md index 6335978da8..41bb9a4f3d 100644 --- a/CHANGELOG-3.2.md +++ b/CHANGELOG-3.2.md @@ -101,7 +101,6 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c * feature #19325 [FrameworkBundle] Allow to specify a domain when updating translations (antograssiot) * feature #19277 [Serializer] Argument objects (theofidry, dunglas) * feature #19322 [HttpFoundation] Add Request::isMethodIdempotent method (dunglas) - * feature #17608 [DependencyInjection] Autowiring: add setter injection support (dunglas) * feature #18510 Added a SecurityUserValueResolver for controllers (iltar) * feature #19203 [Bridge/Doctrine] Reset the EM lazy-proxy instead of the EM service (nicolas-grekas) * feature #19236 [FrameworkBundle] Deprecate the service serializer.mapping.cache.doctrine.apc (Ener-Getick) diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index e006527ce2..43f445736f 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/src/Symfony/Component/DependencyInjection/CHANGELOG.md @@ -4,7 +4,6 @@ CHANGELOG 3.2.0 ----- - * added support for setter autowiring * allowed to prioritize compiler passes by introducing a third argument to `PassConfig::addPass()`, to `Compiler::addPass` and to `ContainerBuilder::addCompilerPass()` * added support for PHP constants in YAML configuration files * deprecated the ability to set or unset a private service with the `Container::set()` method diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 09cbd4b06c..d1b1fd292c 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -97,42 +97,12 @@ class AutowirePass implements CompilerPassInterface $this->container->addResource(static::createResourceForClass($reflectionClass)); } - if ($constructor = $reflectionClass->getConstructor()) { - $this->autowireMethod($id, $definition, $constructor, true); + if (!$constructor = $reflectionClass->getConstructor()) { + return; } - $methodsCalled = array(); - foreach ($definition->getMethodCalls() as $methodCall) { - $methodsCalled[$methodCall[0]] = true; - } - - foreach (self::getSetters($reflectionClass) as $reflectionMethod) { - if (!isset($methodsCalled[$reflectionMethod->name])) { - $this->autowireMethod($id, $definition, $reflectionMethod, false); - } - } - } - - /** - * Autowires the constructor or a setter. - * - * @param string $id - * @param Definition $definition - * @param \ReflectionMethod $reflectionMethod - * @param bool $isConstructor - * - * @throws RuntimeException - */ - private function autowireMethod($id, Definition $definition, \ReflectionMethod $reflectionMethod, $isConstructor) - { - if ($isConstructor) { - $arguments = $definition->getArguments(); - } else { - $arguments = array(); - } - - $addMethodCall = false; - foreach ($reflectionMethod->getParameters() as $index => $parameter) { + $arguments = $definition->getArguments(); + foreach ($constructor->getParameters() as $index => $parameter) { if (array_key_exists($index, $arguments) && '' !== $arguments[$index]) { continue; } @@ -141,11 +111,7 @@ class AutowirePass implements CompilerPassInterface if (!$typeHint = $parameter->getClass()) { // no default value? Then fail if (!$parameter->isOptional()) { - if ($isConstructor) { - 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)); - } - - return; + 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)); } // specifically pass the default value @@ -160,23 +126,16 @@ class AutowirePass implements CompilerPassInterface if (isset($this->types[$typeHint->name])) { $value = new Reference($this->types[$typeHint->name]); - $addMethodCall = true; } else { try { $value = $this->createAutowiredDefinition($typeHint, $id); - $addMethodCall = true; } catch (RuntimeException $e) { if ($parameter->allowsNull()) { $value = null; } elseif ($parameter->isDefaultValueAvailable()) { $value = $parameter->getDefaultValue(); } else { - // The exception code is set to 1 if the exception must be thrown even if it's a setter - if (1 === $e->getCode() || $isConstructor) { - throw $e; - } - - return; + throw $e; } } } @@ -184,11 +143,7 @@ class AutowirePass implements CompilerPassInterface // Typehint against a non-existing class if (!$parameter->isDefaultValueAvailable()) { - if ($isConstructor) { - 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); - } - - return; + 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); } $value = $parameter->getDefaultValue(); @@ -200,12 +155,7 @@ class AutowirePass implements CompilerPassInterface // it's possible index 1 was set, then index 0, then 2, etc // make sure that we re-order so they're injected as expected ksort($arguments); - - if ($isConstructor) { - $definition->setArguments($arguments); - } elseif ($addMethodCall) { - $definition->addMethodCall($reflectionMethod->name, $arguments); - } + $definition->setArguments($arguments); } /** @@ -303,7 +253,7 @@ class AutowirePass implements CompilerPassInterface $classOrInterface = $typeHint->isInterface() ? 'interface' : 'class'; $matchingServices = implode(', ', $this->ambiguousServiceTypes[$typeHint->name]); - throw new RuntimeException(sprintf('Unable to autowire argument of type "%s" for the service "%s". Multiple services exist for this %s (%s).', $typeHint->name, $id, $classOrInterface, $matchingServices), 1); + throw new RuntimeException(sprintf('Unable to autowire argument of type "%s" for the service "%s". Multiple services exist for this %s (%s).', $typeHint->name, $id, $classOrInterface, $matchingServices)); } if (!$typeHint->isInstantiable()) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index 684e99b632..a8b1be3550 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -429,47 +429,6 @@ class AutowirePassTest extends \PHPUnit_Framework_TestCase ); } - public function testSetterInjection() - { - $container = new ContainerBuilder(); - $container->register('app_foo', Foo::class); - $container->register('app_a', A::class); - $container->register('app_collision_a', CollisionA::class); - $container->register('app_collision_b', CollisionB::class); - - // manually configure *one* call, to override autowiring - $container - ->register('setter_injection', SetterInjection::class) - ->setAutowired(true) - ->addMethodCall('setWithCallsConfigured', array('manual_arg1', 'manual_arg2')) - ; - - $pass = new AutowirePass(); - $pass->process($container); - - $methodCalls = $container->getDefinition('setter_injection')->getMethodCalls(); - - // grab the call method names - $actualMethodNameCalls = array_map(function ($call) { - return $call[0]; - }, $methodCalls); - $this->assertEquals( - array('setWithCallsConfigured', 'setFoo'), - $actualMethodNameCalls - ); - - // test setWithCallsConfigured args - $this->assertEquals( - array('manual_arg1', 'manual_arg2'), - $methodCalls[0][1] - ); - // test setFoo args - $this->assertEquals( - array(new Reference('app_foo')), - $methodCalls[1][1] - ); - } - /** * @dataProvider getCreateResourceTests */ @@ -517,24 +476,6 @@ class AutowirePassTest extends \PHPUnit_Framework_TestCase $this->assertTrue($container->hasDefinition('bar')); } - - /** - * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMessage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" for the service "setter_injection_collision". Multiple services exist for this interface (c1, c2). - * @expectedExceptionCode 1 - */ - public function testSetterInjectionCollisionThrowsException() - { - $container = new ContainerBuilder(); - - $container->register('c1', CollisionA::class); - $container->register('c2', CollisionB::class); - $aDefinition = $container->register('setter_injection_collision', SetterInjectionCollision::class); - $aDefinition->setAutowired(true); - - $pass = new AutowirePass(); - $pass->process($container); - } } class Foo @@ -707,69 +648,9 @@ class ClassForResource class IdenticalClassResource extends ClassForResource { } - class ClassChangedConstructorArgs extends ClassForResource { public function __construct($foo, Bar $bar, $baz) { } } - -class SetterInjection -{ - public function setFoo(Foo $foo) - { - // should be called - } - - public function setDependencies(Foo $foo, A $a) - { - // should be called - } - - public function setBar() - { - // should not be called - } - - public function setNotAutowireable(NotARealClass $n) - { - // should not be called - } - - public function setArgCannotAutowire($foo) - { - // should not be called - } - - public function setOptionalNotAutowireable(NotARealClass $n = null) - { - // should not be called - } - - public function setOptionalNoTypeHint($foo = null) - { - // should not be called - } - - public function setOptionalArgNoAutowireable($other = 'default_val') - { - // should not be called - } - - public function setWithCallsConfigured(A $a) - { - // this method has a calls configured on it - // should not be called - } -} - -class SetterInjectionCollision -{ - public function setMultipleInstancesForOneArg(CollisionInterface $collision) - { - // The CollisionInterface cannot be autowired - there are multiple - - // should throw an exception - } -}