Skip to content

Revert "feature #17608 [DependencyInjection] Autowiring: add setter injection support (dunglas)" #20384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 2, 2016
Merged

Revert "feature #17608 [DependencyInjection] Autowiring: add setter injection support (dunglas)" #20384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CHANGELOG-3.2.md
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
}
}