bug #21797 [DI] Simplify AutowirePass and other master-only cleanups (nicolas-grekas)
This PR was merged into the 3.3-dev branch.
Discussion
----------
[DI] Simplify AutowirePass and other master-only cleanups
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
A few minor cleanups and fixes, and an overall simplification of AutowirePass.
Commits
-------
34e5cc7698
[DI] Simplify AutowirePass and other master-only cleanups
This commit is contained in:
commit
adc6ba5d9d
@ -399,14 +399,14 @@ class JsonDescriptor extends Descriptor
|
||||
);
|
||||
}
|
||||
|
||||
if ($value instanceof Definition) {
|
||||
return $this->getContainerDefinitionData($value, $omitTags, $showArguments);
|
||||
}
|
||||
|
||||
if ($value instanceof ArgumentInterface) {
|
||||
return $this->describeValue($value->getValues(), $omitTags, $showArguments);
|
||||
}
|
||||
|
||||
if ($value instanceof Definition) {
|
||||
return $this->getContainerDefinitionData($value, $omitTags, $showArguments);
|
||||
}
|
||||
|
||||
return $value;
|
||||
}
|
||||
}
|
||||
|
@ -330,8 +330,6 @@ class TextDescriptor extends Descriptor
|
||||
foreach ($arguments as $argument) {
|
||||
if ($argument instanceof Reference) {
|
||||
$argumentsInformation[] = sprintf('Service(%s)', (string) $argument);
|
||||
} elseif ($argument instanceof Definition) {
|
||||
$argumentsInformation[] = 'Inlined Service';
|
||||
} elseif ($argument instanceof IteratorArgument) {
|
||||
$argumentsInformation[] = sprintf('Iterator (%d element(s))', count($argument->getValues()));
|
||||
} elseif ($argument instanceof ServiceLocatorArgument) {
|
||||
@ -339,6 +337,8 @@ class TextDescriptor extends Descriptor
|
||||
} elseif ($argument instanceof ClosureProxyArgument) {
|
||||
list($reference, $method) = $argument->getValues();
|
||||
$argumentsInformation[] = sprintf('ClosureProxy(Service(%s)::%s())', $reference, $method);
|
||||
} elseif ($argument instanceof Definition) {
|
||||
$argumentsInformation[] = 'Inlined Service';
|
||||
} else {
|
||||
$argumentsInformation[] = is_array($argument) ? sprintf('Array (%d element(s))', count($argument)) : $argument;
|
||||
}
|
||||
|
@ -440,8 +440,6 @@ class XmlDescriptor extends Descriptor
|
||||
if ($argument instanceof Reference) {
|
||||
$argumentXML->setAttribute('type', 'service');
|
||||
$argumentXML->setAttribute('id', (string) $argument);
|
||||
} elseif ($argument instanceof Definition) {
|
||||
$argumentXML->appendChild($dom->importNode($this->getContainerDefinitionDocument($argument, null, false, true)->childNodes->item(0), true));
|
||||
} elseif ($argument instanceof IteratorArgument) {
|
||||
$argumentXML->setAttribute('type', 'iterator');
|
||||
|
||||
@ -459,6 +457,8 @@ class XmlDescriptor extends Descriptor
|
||||
$argumentXML->setAttribute('type', 'closure-proxy');
|
||||
$argumentXML->setAttribute('id', (string) $reference);
|
||||
$argumentXML->setAttribute('method', $method);
|
||||
} elseif ($argument instanceof Definition) {
|
||||
$argumentXML->appendChild($dom->importNode($this->getContainerDefinitionDocument($argument, null, false, true)->childNodes->item(0), true));
|
||||
} elseif (is_array($argument)) {
|
||||
$argumentXML->setAttribute('type', 'collection');
|
||||
|
||||
|
@ -74,7 +74,7 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe
|
||||
|
||||
if ($value instanceof ArgumentInterface) {
|
||||
$this->lazy = true;
|
||||
parent::processValue($value);
|
||||
parent::processValue($value->getValues());
|
||||
$this->lazy = $lazy;
|
||||
|
||||
return $value;
|
||||
|
@ -15,6 +15,7 @@ use Symfony\Component\DependencyInjection\Config\AutowireServiceResource;
|
||||
use Symfony\Component\DependencyInjection\ContainerBuilder;
|
||||
use Symfony\Component\DependencyInjection\Definition;
|
||||
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
|
||||
use Symfony\Component\DependencyInjection\LazyProxy\InheritanceProxyHelper;
|
||||
use Symfony\Component\DependencyInjection\Reference;
|
||||
|
||||
/**
|
||||
@ -252,13 +253,7 @@ class AutowirePass extends AbstractRecursivePass
|
||||
continue;
|
||||
}
|
||||
|
||||
if (method_exists($parameter, 'getType')) {
|
||||
if ($typeName = $parameter->getType()) {
|
||||
$typeName = $typeName->isBuiltin() ? null : ($typeName instanceof \ReflectionNamedType ? $typeName->getName() : $typeName->__toString());
|
||||
}
|
||||
} elseif (preg_match('/^(?:[^ ]++ ){4}([a-zA-Z_\x7F-\xFF][^ ]++)/', $parameter, $typeName)) {
|
||||
$typeName = 'callable' === $typeName[1] || 'array' === $typeName[1] ? null : $typeName[1];
|
||||
}
|
||||
$typeName = InheritanceProxyHelper::getTypeHint($reflectionMethod, $parameter, true);
|
||||
|
||||
if (!$typeName) {
|
||||
// no default value? Then fail
|
||||
@ -278,52 +273,23 @@ class AutowirePass extends AbstractRecursivePass
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($this->container->has($typeName) && !$this->container->findDefinition($typeName)->isAbstract()) {
|
||||
$arguments[$index] = new Reference($typeName);
|
||||
$didAutowire = true;
|
||||
|
||||
continue;
|
||||
}
|
||||
|
||||
if (null === $this->types) {
|
||||
$this->populateAvailableTypes();
|
||||
}
|
||||
|
||||
if (isset($this->types[$typeName])) {
|
||||
$value = new Reference($this->types[$typeName]);
|
||||
if ($value = $this->getAutowiredReference($typeName)) {
|
||||
$didAutowire = true;
|
||||
$this->usedTypes[$typeName] = $this->currentId;
|
||||
} elseif ($typeHint = $this->container->getReflectionClass($typeName, true)) {
|
||||
try {
|
||||
$value = $this->createAutowiredDefinition($typeHint);
|
||||
$didAutowire = true;
|
||||
$this->usedTypes[$typeName] = $this->currentId;
|
||||
} 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 an optional setter
|
||||
if (1 === $e->getCode() || self::MODE_REQUIRED === $mode) {
|
||||
throw $e;
|
||||
}
|
||||
|
||||
return array();
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Typehint against a non-existing class
|
||||
|
||||
if (!$parameter->isDefaultValueAvailable()) {
|
||||
if (self::MODE_REQUIRED === $mode) {
|
||||
throw new RuntimeException(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));
|
||||
}
|
||||
|
||||
return array();
|
||||
}
|
||||
|
||||
} elseif ($parameter->isDefaultValueAvailable()) {
|
||||
$value = $parameter->getDefaultValue();
|
||||
} elseif ($parameter->allowsNull()) {
|
||||
$value = null;
|
||||
} elseif (self::MODE_REQUIRED === $mode) {
|
||||
if ($classOrInterface = class_exists($typeName, false) ? 'class' : (interface_exists($typeName, false) ? 'interface' : null)) {
|
||||
$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);
|
||||
} else {
|
||||
$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);
|
||||
}
|
||||
|
||||
throw new RuntimeException($message);
|
||||
} else {
|
||||
return array();
|
||||
}
|
||||
|
||||
$arguments[$index] = $value;
|
||||
@ -356,44 +322,41 @@ class AutowirePass extends AbstractRecursivePass
|
||||
|| 0 !== $reflectionMethod->getNumberOfParameters()
|
||||
|| $reflectionMethod->isFinal()
|
||||
|| $reflectionMethod->returnsReference()
|
||||
|| !$returnType = $reflectionMethod->getReturnType()
|
||||
|| !($typeName = InheritanceProxyHelper::getTypeHint($reflectionMethod, null, true))
|
||||
|| !($typeRef = $this->getAutowiredReference($typeName))
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
$typeName = $returnType instanceof \ReflectionNamedType ? $returnType->getName() : $returnType->__toString();
|
||||
|
||||
if ($this->container->has($typeName) && !$this->container->findDefinition($typeName)->isAbstract()) {
|
||||
$overridenGetters[$lcMethod] = new Reference($typeName);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (null === $this->types) {
|
||||
$this->populateAvailableTypes();
|
||||
}
|
||||
|
||||
if (isset($this->types[$typeName])) {
|
||||
$value = new Reference($this->types[$typeName]);
|
||||
} elseif ($returnType = $this->container->getReflectionClass($typeName, true)) {
|
||||
try {
|
||||
$value = $this->createAutowiredDefinition($returnType);
|
||||
} catch (RuntimeException $e) {
|
||||
if (1 === $e->getCode()) {
|
||||
throw $e;
|
||||
}
|
||||
|
||||
continue;
|
||||
}
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
|
||||
$overridenGetters[$lcMethod] = $value;
|
||||
$overridenGetters[$lcMethod] = $typeRef;
|
||||
$this->usedTypes[$typeName] = $this->currentId;
|
||||
}
|
||||
|
||||
return $overridenGetters;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return Reference|null A reference to the service matching the given type, if any
|
||||
*/
|
||||
private function getAutowiredReference($typeName, $autoRegister = true)
|
||||
{
|
||||
if ($this->container->has($typeName) && !$this->container->findDefinition($typeName)->isAbstract()) {
|
||||
return new Reference($typeName);
|
||||
}
|
||||
|
||||
if (null === $this->types) {
|
||||
$this->populateAvailableTypes();
|
||||
}
|
||||
|
||||
if (isset($this->types[$typeName])) {
|
||||
return new Reference($this->types[$typeName]);
|
||||
}
|
||||
|
||||
if ($autoRegister && $class = $this->container->getReflectionClass($typeName, true)) {
|
||||
return $this->createAutowiredDefinition($class);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Populates the list of available types.
|
||||
*/
|
||||
@ -477,7 +440,7 @@ class AutowirePass extends AbstractRecursivePass
|
||||
*
|
||||
* @param \ReflectionClass $typeHint
|
||||
*
|
||||
* @return Reference A reference to the registered definition
|
||||
* @return Reference|null A reference to the registered definition
|
||||
*
|
||||
* @throws RuntimeException
|
||||
*/
|
||||
@ -487,12 +450,11 @@ class AutowirePass extends AbstractRecursivePass
|
||||
$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, $this->currentId, $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, $this->currentId, $classOrInterface, $matchingServices));
|
||||
}
|
||||
|
||||
if (!$typeHint->isInstantiable()) {
|
||||
$classOrInterface = $typeHint->isInterface() ? 'interface' : 'class';
|
||||
throw new RuntimeException(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.', $typeHint->name, $this->currentId, $classOrInterface));
|
||||
return;
|
||||
}
|
||||
|
||||
$currentId = $this->currentId;
|
||||
@ -504,14 +466,8 @@ class AutowirePass extends AbstractRecursivePass
|
||||
|
||||
$this->populateAvailableType($argumentId, $argumentDefinition);
|
||||
|
||||
try {
|
||||
$this->processValue($argumentDefinition);
|
||||
$this->currentId = $currentId;
|
||||
} catch (RuntimeException $e) {
|
||||
$classOrInterface = $typeHint->isInterface() ? 'interface' : 'class';
|
||||
$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.', $typeHint->name, $this->currentId, $classOrInterface);
|
||||
throw new RuntimeException($message, 0, $e);
|
||||
}
|
||||
$this->processValue($argumentDefinition);
|
||||
$this->currentId = $currentId;
|
||||
|
||||
return new Reference($argumentId);
|
||||
}
|
||||
|
@ -53,7 +53,9 @@ class ResolveInvalidReferencesPass implements CompilerPassInterface
|
||||
*/
|
||||
private function processValue($value, $rootLevel = 0, $level = 0)
|
||||
{
|
||||
if ($value instanceof Definition) {
|
||||
if ($value instanceof ArgumentInterface) {
|
||||
$value->setValues($this->processValue($value->getValues(), $rootLevel, 1 + $level));
|
||||
} elseif ($value instanceof Definition) {
|
||||
if ($value->isSynthetic() || $value->isAbstract()) {
|
||||
return $value;
|
||||
}
|
||||
@ -87,8 +89,6 @@ class ResolveInvalidReferencesPass implements CompilerPassInterface
|
||||
if (false !== $i) {
|
||||
$value = array_values($value);
|
||||
}
|
||||
} elseif ($value instanceof ArgumentInterface) {
|
||||
$value->setValues($this->processValue($value->getValues(), $rootLevel, 1 + $level));
|
||||
} elseif ($value instanceof Reference) {
|
||||
$id = (string) $value;
|
||||
|
||||
|
@ -11,8 +11,8 @@
|
||||
|
||||
namespace Symfony\Component\DependencyInjection\Compiler;
|
||||
|
||||
use Symfony\Component\DependencyInjection\Argument\ArgumentInterface;
|
||||
use Symfony\Component\DependencyInjection\Alias;
|
||||
use Symfony\Component\DependencyInjection\Argument\ArgumentInterface;
|
||||
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
|
||||
use Symfony\Component\DependencyInjection\Reference;
|
||||
use Symfony\Component\DependencyInjection\ContainerBuilder;
|
||||
|
@ -1127,6 +1127,9 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
|
||||
$parameterBag = $this->getParameterBag();
|
||||
$services = array();
|
||||
foreach ($value->getValues() as $k => $v) {
|
||||
if ($v->getInvalidBehavior() === ContainerInterface::IGNORE_ON_INVALID_REFERENCE && !$this->has((string) $v)) {
|
||||
continue;
|
||||
}
|
||||
$services[$k] = function () use ($v, $parameterBag) {
|
||||
return $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($v)));
|
||||
};
|
||||
|
@ -11,6 +11,7 @@
|
||||
|
||||
namespace Symfony\Component\DependencyInjection\Dumper;
|
||||
|
||||
use Symfony\Component\DependencyInjection\Argument\ArgumentInterface;
|
||||
use Symfony\Component\DependencyInjection\Argument\ClosureProxyArgument;
|
||||
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
|
||||
use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
|
||||
@ -1286,6 +1287,8 @@ EOF;
|
||||
foreach ($parameters as $key => $value) {
|
||||
if (is_array($value)) {
|
||||
$value = $this->exportParameters($value, $path.'/'.$key, $indent + 4);
|
||||
} elseif ($value instanceof ArgumentInterface) {
|
||||
throw new InvalidArgumentException(sprintf('You cannot dump a container with parameters that contain special arguments. "%s" found in "%s".', get_class($value), $path.'/'.$key));
|
||||
} elseif ($value instanceof Variable) {
|
||||
throw new InvalidArgumentException(sprintf('You cannot dump a container with parameters that contain variable references. Variable "%s" found in "%s".', $value, $path.'/'.$key));
|
||||
} elseif ($value instanceof Definition) {
|
||||
@ -1458,6 +1461,8 @@ EOF;
|
||||
foreach ($arguments as $argument) {
|
||||
if (is_array($argument)) {
|
||||
$definitions = array_merge($definitions, $this->getDefinitionsFromArguments($argument));
|
||||
} elseif ($argument instanceof ArgumentInterface) {
|
||||
$definitions = array_merge($definitions, $this->getDefinitionsFromArguments($argument->getValues()));
|
||||
} elseif ($argument instanceof Definition) {
|
||||
$definitions = array_merge(
|
||||
$definitions,
|
||||
|
@ -63,12 +63,7 @@ class InheritanceProxyHelper
|
||||
if ($p->isPassedByReference()) {
|
||||
$k = '&'.$k;
|
||||
}
|
||||
if (method_exists($p, 'getType')) {
|
||||
$type = $p->getType();
|
||||
} elseif (preg_match('/^(?:[^ ]++ ){4}([a-zA-Z_\x7F-\xFF][^ ]++)/', $p, $type)) {
|
||||
$type = $type[1];
|
||||
}
|
||||
if ($type && $type = self::getTypeHint($type, $r)) {
|
||||
if ($type = self::getTypeHint($r, $p)) {
|
||||
$k = $type.' '.$k;
|
||||
}
|
||||
if ($type && $p->allowsNull()) {
|
||||
@ -91,46 +86,55 @@ class InheritanceProxyHelper
|
||||
}
|
||||
$call = ($r->isClosure() ? '' : $r->name).'('.implode(', ', $call).')';
|
||||
|
||||
if ($type = method_exists($r, 'getReturnType') ? $r->getReturnType() : null) {
|
||||
$type = ': '.($type->allowsNull() ? '?' : '').self::getTypeHint($type, $r);
|
||||
if ($type = self::getTypeHint($r)) {
|
||||
$type = ': '.($r->getReturnType()->allowsNull() ? '?' : '').$type;
|
||||
}
|
||||
|
||||
return ($r->returnsReference() ? '&' : '').($r->isClosure() ? '' : $r->name).'('.implode(', ', $signature).')'.$type;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param $type \ReflectionType|string $type As returned by ReflectionParameter::getType() - or string on PHP 5
|
||||
*
|
||||
* @return string|null The FQCN or builtin name of the type hint, or null when the type hint references an invalid self|parent context
|
||||
*/
|
||||
public static function getTypeHint($type, \ReflectionFunctionAbstract $r)
|
||||
public static function getTypeHint(\ReflectionFunctionAbstract $r, \ReflectionParameter $p = null, $noBuiltin = false)
|
||||
{
|
||||
if (is_string($type)) {
|
||||
$name = $type;
|
||||
if ($p instanceof \ReflectionParameter) {
|
||||
if (method_exists($p, 'getType')) {
|
||||
$type = $p->getType();
|
||||
} elseif (preg_match('/^(?:[^ ]++ ){4}([a-zA-Z_\x7F-\xFF][^ ]++)/', $p, $type)) {
|
||||
$name = $type = $type[1];
|
||||
|
||||
if ('callable' === $name || 'array' === $name) {
|
||||
return $name;
|
||||
if ('callable' === $name || 'array' === $name) {
|
||||
return $noBuiltin ? null : $name;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
$type = method_exists($r, 'getReturnType') ? $r->getReturnType() : null;
|
||||
}
|
||||
if (!$type) {
|
||||
return;
|
||||
}
|
||||
if (!is_string($type)) {
|
||||
$name = $type instanceof \ReflectionNamedType ? $type->getName() : $type->__toString();
|
||||
|
||||
if ($type->isBuiltin()) {
|
||||
return $name;
|
||||
return $noBuiltin ? null : $name;
|
||||
}
|
||||
}
|
||||
$lcName = strtolower($name);
|
||||
$prefix = $noBuiltin ? '' : '\\';
|
||||
|
||||
if ('self' !== $lcName && 'parent' !== $lcName) {
|
||||
return '\\'.$name;
|
||||
return $prefix.$name;
|
||||
}
|
||||
if (!$r instanceof \ReflectionMethod) {
|
||||
return;
|
||||
}
|
||||
if ('self' === $lcName) {
|
||||
return '\\'.$r->getDeclaringClass()->name;
|
||||
return $prefix.$r->getDeclaringClass()->name;
|
||||
}
|
||||
if ($parent = $r->getDeclaringClass()->getParentClass()) {
|
||||
return '\\'.$parent->name;
|
||||
return $prefix.$parent->name;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -12,6 +12,7 @@
|
||||
namespace Symfony\Component\DependencyInjection\Loader;
|
||||
|
||||
use Symfony\Component\DependencyInjection\Alias;
|
||||
use Symfony\Component\DependencyInjection\Argument\ArgumentInterface;
|
||||
use Symfony\Component\DependencyInjection\Argument\ClosureProxyArgument;
|
||||
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
|
||||
use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
|
||||
|
@ -638,7 +638,6 @@ class AutowirePassTest extends TestCase
|
||||
/**
|
||||
* @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()
|
||||
{
|
||||
|
@ -454,7 +454,7 @@ class ContainerBuilderTest extends TestCase
|
||||
|
||||
$this->assertInstanceOf(ServiceLocator::class, $locator);
|
||||
$this->assertInstanceOf('stdClass', $locator->get('bar'));
|
||||
$this->assertNull($locator->get('invalid'));
|
||||
$this->assertFalse($locator->has('invalid'));
|
||||
$this->assertSame($locator->get('bar'), $locator('bar'), '->get() should be used when invoking ServiceLocator');
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user