Skip to content
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.

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

[DI] Generalize constructor autowiring to partial method calls #21404

Merged
merged 1 commit into from
Jan 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
142 changes: 95 additions & 47 deletions src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ public static function createResourceForClass(\ReflectionClass $reflectionClass)
{
$metadata = array();

if ($constructor = $reflectionClass->getConstructor()) {
$metadata['__construct'] = self::getResourceMetadataForMethod($constructor);
}

foreach ($reflectionClass->getMethods(\ReflectionMethod::IS_PUBLIC) as $reflectionMethod) {
if (!$reflectionMethod->isStatic()) {
$metadata[$reflectionMethod->name] = self::getResourceMetadataForMethod($reflectionMethod);
Expand All @@ -91,39 +87,45 @@ protected function processValue($value, $isRoot = false)
$this->container->addResource(static::createResourceForClass($reflectionClass));
}

$methodsCalled = array();
foreach ($value->getMethodCalls() as $methodCall) {
$methodsCalled[strtolower($methodCall[0])] = true;
$autowiredMethods = $this->getMethodsToAutowire($reflectionClass, $autowiredMethods);
$methodCalls = $value->getMethodCalls();

if ($constructor = $reflectionClass->getConstructor()) {
array_unshift($methodCalls, array($constructor->name, $value->getArguments()));
} elseif ($value->getArguments()) {
throw new RuntimeException(sprintf('Cannot autowire service "%s": class %s has no constructor but arguments are defined.', $this->currentId, $reflectionClass->name, $method));
}

foreach ($this->getMethodsToAutowire($reflectionClass, $autowiredMethods) as $reflectionMethod) {
if (!isset($methodsCalled[strtolower($reflectionMethod->name)])) {
$this->autowireMethod($value, $reflectionMethod);
$methodCalls = $this->autowireMethodCalls($reflectionClass, $methodCalls, $autowiredMethods);

if ($constructor) {
list(, $arguments) = array_shift($methodCalls);

if ($arguments !== $value->getArguments()) {
$value->setArguments($arguments);
}
}

if ($methodCalls !== $value->getMethodCalls()) {
$value->setMethodCalls($methodCalls);
}

return parent::processValue($value, $isRoot);
}

/**
* Gets the list of methods to autowire.
*
* @param \ReflectionClass $reflectionClass
* @param string[] $configuredAutowiredMethods
* @param string[] $autowiredMethods
*
* @return \ReflectionMethod[]
*/
private function getMethodsToAutowire(\ReflectionClass $reflectionClass, array $configuredAutowiredMethods)
private function getMethodsToAutowire(\ReflectionClass $reflectionClass, array $autowiredMethods)
{
$found = array();
$regexList = array();

// Always try to autowire the constructor
if (in_array('__construct', $configuredAutowiredMethods, true)) {
$autowiredMethods = $configuredAutowiredMethods;
} else {
$autowiredMethods = array_merge(array('__construct'), $configuredAutowiredMethods);
}
$methodsToAutowire = array();

foreach ($autowiredMethods as $pattern) {
$regexList[] = '/^'.str_replace('\*', '.*', preg_quote($pattern, '/')).'$/i';
Expand All @@ -137,36 +139,82 @@ private function getMethodsToAutowire(\ReflectionClass $reflectionClass, array $
foreach ($regexList as $k => $regex) {
if (preg_match($regex, $reflectionMethod->name)) {
$found[] = $autowiredMethods[$k];
yield $reflectionMethod;

$methodsToAutowire[strtolower($reflectionMethod->name)] = $reflectionMethod;
continue 2;
}
}

if ($reflectionMethod->isConstructor()) {
$methodsToAutowire[strtolower($reflectionMethod->name)] = $reflectionMethod;
}
}

if ($notFound = array_diff($configuredAutowiredMethods, $found)) {
if ($notFound = array_diff($autowiredMethods, $found)) {
$compiler = $this->container->getCompiler();
$compiler->addLogMessage($compiler->getLoggingFormatter()->formatUnusedAutowiringPatterns($this, $this->currentId, $notFound));
}

return $methodsToAutowire;
}

/**
* @param \ReflectionClass $reflectionClass
* @param array $methodCalls
* @param \ReflectionMethod[] $autowiredMethods
*
* @return array
*/
private function autowireMethodCalls(\ReflectionClass $reflectionClass, array $methodCalls, array $autowiredMethods)
{
$parameterBag = $this->container->getParameterBag();

foreach ($methodCalls as $i => $call) {
list($method, $arguments) = $call;
$method = $parameterBag->resolveValue($method);

if (isset($autowiredMethods[$lcMethod = strtolower($method)])) {
$reflectionMethod = $autowiredMethods[$lcMethod];
unset($autowiredMethods[$lcMethod]);
} else {
if (!$reflectionClass->hasMethod($method)) {
throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s::%s() does not exist.', $this->currentId, $reflectionClass->name, $method));
}
$reflectionMethod = $reflectionClass->getMethod($method);
if (!$reflectionMethod->isPublic()) {
throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s::%s() must be public.', $this->currentId, $reflectionClass->name, $method));
}
}

$arguments = $this->autowireMethod($reflectionMethod, $arguments, true);

if ($arguments !== $call[1]) {
$methodCalls[$i][1] = $arguments;
}
}

foreach ($autowiredMethods as $reflectionMethod) {
if ($arguments = $this->autowireMethod($reflectionMethod, array(), false)) {
$methodCalls[] = array($reflectionMethod->name, $arguments);
}
}

return $methodCalls;
}

/**
* Autowires the constructor or a setter.
*
* @param Definition $definition
* @param \ReflectionMethod $reflectionMethod
* @param array $arguments
* @param bool $mustAutowire
*
* @return array The autowired arguments
*
* @throws RuntimeException
*/
private function autowireMethod(Definition $definition, \ReflectionMethod $reflectionMethod)
private function autowireMethod(\ReflectionMethod $reflectionMethod, array $arguments, $mustAutowire)
{
if ($isConstructor = $reflectionMethod->isConstructor()) {
$arguments = $definition->getArguments();
} else {
$arguments = array();
}

$addMethodCall = false; // Whether the method should be added to the definition as a call or as arguments
$didAutowire = false; // Whether any arguments have been autowired or not
foreach ($reflectionMethod->getParameters() as $index => $parameter) {
if (array_key_exists($index, $arguments) && '' !== $arguments[$index]) {
continue;
Expand All @@ -176,11 +224,11 @@ private function autowireMethod(Definition $definition, \ReflectionMethod $refle
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, $this->currentId));
if ($mustAutowire) {
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));
}

return;
return array();
}

// specifically pass the default value
Expand All @@ -195,35 +243,35 @@ private function autowireMethod(Definition $definition, \ReflectionMethod $refle

if (isset($this->types[$typeHint->name])) {
$value = new Reference($this->types[$typeHint->name]);
$addMethodCall = true;
$didAutowire = true;
} else {
try {
$value = $this->createAutowiredDefinition($typeHint);
$addMethodCall = true;
$didAutowire = 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) {
// The exception code is set to 1 if the exception must be thrown even if it's an optional setter
if (1 === $e->getCode() || $mustAutowire) {
throw $e;
}

return;
return array();
}
}
}
} 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);
if ($mustAutowire) {
throw new RuntimeException(sprintf('Cannot autowire argument $%s of method %s::%s() for service "%s": %s.', $parameter->name, $reflectionMethod->class, $reflectionMethod->name, $this->currentId, $e->getMessage()), 0, $e);
}

return;
return array();
}

$value = $parameter->getDefaultValue();
Expand All @@ -232,15 +280,15 @@ private function autowireMethod(Definition $definition, \ReflectionMethod $refle
$arguments[$index] = $value;
}

if (!$mustAutowire && !$didAutowire) {
return array();
}

// 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);
}
return $arguments;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ public function testDontTriggerAutowiring()

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage Cannot autowire argument 2 for Symfony\Component\DependencyInjection\Tests\Compiler\BadTypeHintedArgument because the type-hinted class does not exist (Class Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass does not exist).
* @expectedExceptionMessage Cannot autowire argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadTypeHintedArgument::__construct() for service "a": Class Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass does not exist.
*/
public function testClassNotFoundThrowsException()
{
Expand All @@ -304,7 +304,7 @@ public function testClassNotFoundThrowsException()

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage Cannot autowire argument 2 for Symfony\Component\DependencyInjection\Tests\Compiler\BadParentTypeHintedArgument because the type-hinted class does not exist (Class Symfony\Component\DependencyInjection\Tests\Compiler\OptionalServiceClass does not exist).
* @expectedExceptionMessage Cannot autowire argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadParentTypeHintedArgument::__construct() for service "a": Class Symfony\Component\DependencyInjection\Tests\Compiler\OptionalServiceClass does not exist.
*/
public function testParentClassNotFoundThrowsException()
{
Expand Down Expand Up @@ -363,7 +363,7 @@ public function testSomeSpecificArgumentsAreSet()

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage Unable to autowire argument index 1 ($foo) for the service "arg_no_type_hint". If this is an object, give it a type-hint. Otherwise, specify this argument's value explicitly.
* @expectedExceptionMessage Cannot autowire service "arg_no_type_hint": argument $foo of method Symfony\Component\DependencyInjection\Tests\Compiler\MultipleArguments::__construct() must have a type-hint or be given a value explicitly.
*/
public function testScalarArgsCannotBeAutowired()
{
Expand All @@ -382,7 +382,7 @@ public function testScalarArgsCannotBeAutowired()

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage Unable to autowire argument index 1 ($foo) for the service "not_really_optional_scalar". If this is an object, give it a type-hint. Otherwise, specify this argument's value explicitly.
* @expectedExceptionMessage Cannot autowire service "not_really_optional_scalar": argument $foo of method Symfony\Component\DependencyInjection\Tests\Compiler\MultipleArgumentsOptionalScalarNotReallyOptional::__construct() must have a type-hint or be given a value explicitly.
*/
public function testOptionalScalarNotReallyOptionalThrowException()
{
Expand Down Expand Up @@ -593,6 +593,22 @@ public function testLogUnusedPatterns()

$this->assertEquals(array(AutowirePass::class.': Autowiring\'s patterns "not", "exist*" for service "foo" don\'t match any method.'), $container->getCompiler()->getLog());
}

public function testPartialMethodCalls()
{
$container = new ContainerBuilder();

$container->register('a', A::class);
$container->register('foo', Foo::class);
$definition = $container->register('bar', SetterInjection::class);
$definition->setAutowired(true);
$definition->addMethodCall('setDependencies', array(new Reference('foo')));

$pass = new AutowirePass();
$pass->process($container);

$this->assertEquals(array(array('setDependencies', array(new Reference('foo'), new Reference('a')))), $container->getDefinition('bar')->getMethodCalls());
}
}

class Foo
Expand Down