Skip to content

Commit

Permalink
Revert "feature #17608 [DependencyInjection] Autowiring: add setter i…
Browse files Browse the repository at this point in the history 
…njection support (dunglas)"

This reverts commit 7eab6b9, reversing
changes made to 35f201f.
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Nov 2, 2016
1 parent 46a8ede commit bf91eda
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 180 deletions.
1 change: 0 additions & 1 deletion CHANGELOG-3.2.md
View file
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Component/DependencyInjection/CHANGELOG.md
View file
Expand Up @@ -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
Expand Down
68 changes: 9 additions & 59 deletions src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
View file
Expand Up @@ -97,42 +97,12 @@ private function completeDefinition($id, Definition $definition)
$this->container->addResource(static::createResourceForClass($reflectionClass));
}

if ($constructor = $reflectionClass->getConstructor()) {
$this->autowireMethod($id, $definition, $constructor, true);
}

$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();
if (!$constructor = $reflectionClass->getConstructor()) {
return;
}

$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;
}
Expand All @@ -141,11 +111,7 @@ private function autowireMethod($id, Definition $definition, \ReflectionMethod $
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
Expand All @@ -160,35 +126,24 @@ private function autowireMethod($id, Definition $definition, \ReflectionMethod $

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;
}
}
}
} catch (\ReflectionException $e) {
// 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();
Expand All @@ -200,12 +155,7 @@ private function autowireMethod($id, Definition $definition, \ReflectionMethod $
// 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);
}

/**
Expand Down Expand Up @@ -303,7 +253,7 @@ private function createAutowiredDefinition(\ReflectionClass $typeHint, $id)
$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()) {
Expand Down
119 changes: 0 additions & 119 deletions src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
View file
Expand Up @@ -429,47 +429,6 @@ public function testOptionalScalarArgsNotPassedIfLast()
);
}

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
*/
Expand Down Expand Up @@ -517,24 +476,6 @@ public function testIgnoreServiceWithClassNotExisting()

$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
Expand Down Expand Up @@ -707,69 +648,9 @@ public function setBar(Bar $bar)
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
}
}

0 comments on commit bf91eda

Please sign in to comment.