From d7557cf975728744101111300a2d6c51248f5c57 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 23 Mar 2017 23:53:25 +0100 Subject: [PATCH] [DI] Add hints to exceptions thrown by AutowiringPass --- .../Compiler/AutowirePass.php | 96 +++++++++++++------ .../Tests/Compiler/AutowirePassTest.php | 45 +++++++-- 2 files changed, 103 insertions(+), 38 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 026c56aab450..6930d632de70 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -50,13 +50,13 @@ public function process(ContainerBuilder $container) continue; } - $classOrInterface = class_exists($type) ? 'class' : 'interface'; - $matchingServices = implode(', ', $this->ambiguousServiceTypes[$type]); + $this->container = $container; + $classOrInterface = class_exists($type, false) ? 'class' : 'interface'; - throw new RuntimeException(sprintf('Unable to autowire argument of type "%s" for the service "%s". Multiple services exist for this %s (%s).', $type, $id, $classOrInterface, $matchingServices)); + throw new RuntimeException(sprintf('Cannot autowire service "%s": multiple candidate services exist for %s "%s".%s', $id, $classOrInterface, $type, $this->createTypeAlternatives($type))); } } finally { - // Free memory + $this->container = null; $this->definedTypes = array(); $this->types = null; $this->ambiguousServiceTypes = array(); @@ -108,7 +108,12 @@ protected function processValue($value, $isRoot = false) $this->currentDefinition = $value; try { - if (!$value->isAutowired() || !$reflectionClass = $this->container->getReflectionClass($value->getClass())) { + if (!$value->isAutowired() || $value->isAbstract() || !$value->getClass()) { + return parent::processValue($value, $isRoot); + } + if (!$reflectionClass = $this->container->getReflectionClass($value->getClass())) { + $this->container->log($this, sprintf('Skipping service "%s": Class or interface "%s" does not exist.', $this->currentId, $value->getClass())); + return parent::processValue($value, $isRoot); } @@ -117,8 +122,6 @@ protected function processValue($value, $isRoot = false) 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)); } $methodCalls = $this->autowireCalls($reflectionClass, $methodCalls, $autowiredMethods); @@ -203,11 +206,13 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC 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)); + $class = $reflectionClass->name; + throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s() does not exist.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $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)); + $class = $reflectionClass->name; + throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s() must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method)); } } @@ -222,10 +227,13 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC if (!$reflectionMethod->getNumberOfParameters()) { continue; // skip getters } + $method = $reflectionMethod->name; + if (!$reflectionMethod->isPublic()) { - throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s::%s() must be public.', $this->currentId, $reflectionClass->name, $reflectionMethod->name)); + $class = $reflectionClass->name; + throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s() must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method)); } - $methodCalls[] = array($reflectionMethod->name, $this->autowireMethod($reflectionMethod, array())); + $methodCalls[] = array($method, $this->autowireMethod($reflectionMethod, array())); } return $methodCalls; @@ -244,9 +252,11 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC private function autowireMethod(\ReflectionMethod $reflectionMethod, array $arguments) { $isConstructor = $reflectionMethod->isConstructor(); + $class = $reflectionMethod->class; + $method = $reflectionMethod->name; if (!$isConstructor && !$arguments && !$reflectionMethod->getNumberOfRequiredParameters()) { - throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s::%s() has only optional arguments, thus must be wired explicitly.', $this->currentId, $reflectionMethod->class, $reflectionMethod->name)); + throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s() has only optional arguments, thus must be wired explicitly.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method)); } foreach ($reflectionMethod->getParameters() as $index => $parameter) { @@ -265,7 +275,7 @@ private function autowireMethod(\ReflectionMethod $reflectionMethod, array $argu if (!$type) { // no default value? Then fail if (!$parameter->isOptional()) { - 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)); + throw new RuntimeException(sprintf('Cannot autowire service "%s": argument $%s of method %s() must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method)); } if (!array_key_exists($index, $arguments)) { @@ -279,7 +289,7 @@ private function autowireMethod(\ReflectionMethod $reflectionMethod, array $argu if ($value = $this->getAutowiredReference($type)) { $this->usedTypes[$type] = $this->currentId; } else { - $failureMessage = $this->createTypeNotFoundMessage($type, 'argument $'.$parameter->name.' of method '.$reflectionMethod->class.'::'.$reflectionMethod->name.'()'); + $failureMessage = $this->createTypeNotFoundMessage($type, sprintf('argument $%s of method %s()', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method)); if ($parameter->isDefaultValueAvailable()) { $value = $parameter->getDefaultValue(); @@ -315,14 +325,17 @@ private function autowireOverridenGetters(array $overridenGetters, array $autowi if (isset($overridenGetters[$lcMethod]) || $reflectionMethod->getNumberOfParameters() || $reflectionMethod->isConstructor()) { continue; } + $class = $reflectionMethod->class; + $method = $reflectionMethod->name; + if (!$type = InheritanceProxyHelper::getTypeHint($reflectionMethod, null, true)) { $type = InheritanceProxyHelper::getTypeHint($reflectionMethod); - throw new RuntimeException(sprintf('Cannot autowire service "%s": getter %s::%s() must%s have its return value be configured explicitly.', $this->currentId, $reflectionMethod->class, $reflectionMethod->name, $type ? '' : ' have a return-type hint or')); + throw new RuntimeException(sprintf('Cannot autowire service "%s": getter %s() must%s have its return value be configured explicitly.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method, $type ? '' : ' have a return-type hint or')); } if (!$typeRef = $this->getAutowiredReference($type)) { - $this->container->log($this, $this->createTypeNotFoundMessage($type, 'return value of method '.$reflectionMethod->class.'::'.$reflectionMethod->name.'()')); + $this->container->log($this, $this->createTypeNotFoundMessage($type, sprintf('return value of method %s()', $class !== $this->currentId ? $class.'::'.$method : $method))); continue; } @@ -446,15 +459,14 @@ private function set($type, $id) */ private function createAutowiredDefinition(\ReflectionClass $typeHint) { - if (isset($this->ambiguousServiceTypes[$typeHint->name])) { - $classOrInterface = $typeHint->isInterface() ? 'interface' : 'class'; - $matchingServices = implode(', ', $this->ambiguousServiceTypes[$typeHint->name]); + if (isset($this->ambiguousServiceTypes[$type = $typeHint->name])) { + $classOrInterface = class_exists($type) ? 'class' : 'interface'; - 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)); + throw new RuntimeException(sprintf('Cannot autowire service "%s": multiple candidate services exist for %s "%s".%s', $this->currentId, $classOrInterface, $type, $this->createTypeAlternatives($type))); } if (!$typeHint->isInstantiable()) { - $this->container->log($this, sprintf('Type "%s" is not instantiable thus cannot be auto-registered for service "%s".', $typeHint->name, $this->currentId)); + $this->container->log($this, sprintf('Type "%s" is not instantiable thus cannot be auto-registered for service "%s".', $type, $this->currentId)); return; } @@ -463,8 +475,8 @@ private function createAutowiredDefinition(\ReflectionClass $typeHint) $currentDefinition = $this->currentDefinition; $definitions = $this->container->getDefinitions(); $currentId = $this->currentId; - $this->currentId = $argumentId = sprintf('autowired.%s', $typeHint->name); - $this->currentDefinition = $argumentDefinition = new Definition($typeHint->name); + $this->currentId = $argumentId = sprintf('autowired.%s', $type); + $this->currentDefinition = $argumentDefinition = new Definition($type); $argumentDefinition->setPublic(false); $argumentDefinition->setAutowired(true); @@ -475,7 +487,7 @@ private function createAutowiredDefinition(\ReflectionClass $typeHint) $this->container->setDefinition($argumentId, $argumentDefinition); } catch (RuntimeException $e) { // revert any changes done to our internal state - unset($this->types[$typeHint->name]); + unset($this->types[$type]); $this->ambiguousServiceTypes = $ambiguousServiceTypes; $this->container->setDefinitions($definitions); $this->container->log($this, $e->getMessage()); @@ -486,7 +498,7 @@ private function createAutowiredDefinition(\ReflectionClass $typeHint) $this->currentDefinition = $currentDefinition; } - $this->container->log($this, sprintf('Type "%s" has been auto-registered for service "%s".', $typeHint->name, $this->currentId)); + $this->container->log($this, sprintf('Type "%s" has been auto-registered for service "%s".', $type, $this->currentId)); return new Reference($argumentId); } @@ -494,11 +506,39 @@ private function createAutowiredDefinition(\ReflectionClass $typeHint) private function createTypeNotFoundMessage($type, $label) { if (!$classOrInterface = class_exists($type, false) ? 'class' : (interface_exists($type, false) ? 'interface' : null)) { - return sprintf('Cannot autowire %s for service "%s": Class or interface "%s" does not exist.', $label, $this->currentId, $type); + return sprintf('Cannot autowire service "%s": %s has type "%s" but this class does not exist.', $this->currentId, $label, $type); + } + $message = sprintf('no services were found matching the "%s" %s and it cannot be auto-registered for %s.', $type, $classOrInterface, $label); + + return sprintf('Cannot autowire service "%s": %s', $this->currentId, $message); + } + + private function createTypeAlternatives($type) + { + $message = ' This type-hint could be aliased to '; + + if (isset($this->ambiguousServiceTypes[$type])) { + $message .= sprintf('one of these existing services: "%s"', implode('", "', $this->ambiguousServiceTypes[$type])); + } elseif (isset($this->types[$type])) { + $message .= sprintf('the existing "%s" service', $this->types[$type]); + } else { + return; + } + $aliases = array(); + + foreach (class_parents($type) + class_implements($type) as $parent) { + if ($this->container->has($parent)) { + $aliases[] = $parent; + } + } + + if (1 < count($aliases)) { + $message .= sprintf('; or be updated to one of the following: "%s"', implode('", "', $aliases)); + } elseif ($aliases) { + $message .= sprintf('; or be updated to "%s"', $aliases[0]); } - $message = sprintf('No services were found matching the "%s" %s and it cannot be auto-registered', $type, $classOrInterface); - return sprintf('Cannot autowire %s for service "%s": %s.', $label, $this->currentId, $message); + return $message.'.'; } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index b365ba8838e4..e3740c48b485 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -125,7 +125,7 @@ public function testCompleteExistingDefinitionWithNotDefinedArguments() /** * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMessage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" for the service "a". Multiple services exist for this interface (c1, c2, c3). + * @expectedExceptionMessage Cannot autowire service "a": multiple candidate services exist for interface "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface". This type-hint could be aliased to one of these existing services: "c1", "c2", "c3". */ public function testTypeCollision() { @@ -143,7 +143,7 @@ public function testTypeCollision() /** * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMessage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\Foo" for the service "a". Multiple services exist for this class (a1, a2). + * @expectedExceptionMessage Cannot autowire service "a": multiple candidate services exist for class "Symfony\Component\DependencyInjection\Tests\Compiler\Foo". This type-hint could be aliased to one of these existing services: "a1", "a2". */ public function testTypeNotGuessable() { @@ -160,7 +160,7 @@ public function testTypeNotGuessable() /** * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMessage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\A" for the service "a". Multiple services exist for this class (a1, a2). + * @expectedExceptionMessage Cannot autowire service "a": multiple candidate services exist for class "Symfony\Component\DependencyInjection\Tests\Compiler\A". This type-hint could be aliased to one of these existing services: "a1", "a2". */ public function testTypeNotGuessableWithSubclass() { @@ -177,7 +177,7 @@ public function testTypeNotGuessableWithSubclass() /** * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMessage Cannot autowire argument $collision of method Symfony\Component\DependencyInjection\Tests\Compiler\CannotBeAutowired::__construct() for service "a": No services were found matching the "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" interface and it cannot be auto-registered. + * @expectedExceptionMessage Cannot autowire service "a": no services were found matching the "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" interface and it cannot be auto-registered for argument $collision of method Symfony\Component\DependencyInjection\Tests\Compiler\CannotBeAutowired::__construct(). */ public function testTypeNotGuessableNoServicesFound() { @@ -295,7 +295,7 @@ public function testDontTriggerAutowiring() /** * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMessage Cannot autowire argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadTypeHintedArgument::__construct() for service "a": Class or interface "Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass" does not exist. + * @expectedExceptionMessage Cannot autowire service "a": argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadTypeHintedArgument::__construct() has type "Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass" but this class does not exist. */ public function testClassNotFoundThrowsException() { @@ -310,7 +310,7 @@ public function testClassNotFoundThrowsException() /** * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMessage Cannot autowire argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadParentTypeHintedArgument::__construct() for service "a": Class or interface "Symfony\Component\DependencyInjection\Tests\Compiler\OptionalServiceClass" does not exist. + * @expectedExceptionMessage Cannot autowire service "a": argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadParentTypeHintedArgument::__construct() has type "Symfony\Component\DependencyInjection\Tests\Compiler\OptionalServiceClass" but this class does not exist. */ public function testParentClassNotFoundThrowsException() { @@ -563,7 +563,7 @@ public function testGetterOverriding() /** * @requires PHP 7.1 * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMessage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\Foo" for the service "getter_overriding". Multiple services exist for this class (a1, a2). + * @expectedExceptionMessage Cannot autowire service "getter_overriding": multiple candidate services exist for class "Symfony\Component\DependencyInjection\Tests\Compiler\Foo". This type-hint could be aliased to one of these existing services: "a1", "a2". */ public function testGetterOverridingWithAmbiguousServices() { @@ -631,7 +631,7 @@ public function testIgnoreServiceWithClassNotExisting() /** * @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). + * @expectedExceptionMessage Cannot autowire service "setter_injection_collision": multiple candidate services exist for interface "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface". This type-hint could be aliased to one of these existing services: "c1", "c2". */ public function testSetterInjectionCollisionThrowsException() { @@ -666,7 +666,7 @@ public function testEmptyStringIsKept() * @dataProvider provideAutodiscoveredAutowiringOrder * * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMEssage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" for the service "a". Multiple services exist for this interface (autowired.Symfony\Component\DependencyInjection\Tests\Compiler\CollisionA, autowired.Symfony\Component\DependencyInjection\Tests\Compiler\CollisionB). + * @expectedExceptionMEssage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" for service "a". Multiple services exist for this interface: autowired.Symfony\Component\DependencyInjection\Tests\Compiler\CollisionA, autowired.Symfony\Component\DependencyInjection\Tests\Compiler\CollisionB. */ public function testAutodiscoveredAutowiringOrder($class) { @@ -716,7 +716,7 @@ public function testNotWireableCalls($method, $expectedMsg) public function provideNotWireableCalls() { return array( - array('setNotAutowireable', 'Cannot autowire argument $n of method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setNotAutowireable() for service "foo": Class or interface "Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass" does not exist.'), + array('setNotAutowireable', 'Cannot autowire service "foo": argument $n of method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setNotAutowireable() has type "Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass" but this class does not exist.'), array('setBar', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setBar() has only optional arguments, thus must be wired explicitly.'), array('setOptionalNotAutowireable', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalNotAutowireable() has only optional arguments, thus must be wired explicitly.'), array('setOptionalNoTypeHint', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalNoTypeHint() has only optional arguments, thus must be wired explicitly.'), @@ -737,6 +737,24 @@ public function testAutoregisterRestoresStateOnFailure() $this->assertSame(array('service_container', 'e'), array_keys($container->getDefinitions())); } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException + * @expectedExceptionMessage Cannot autowire service "j": multiple candidate services exist for class "Symfony\Component\DependencyInjection\Tests\Compiler\I". This type-hint could be aliased to one of these existing services: "f", "i"; or be updated to "Symfony\Component\DependencyInjection\Tests\Compiler\IInterface". + */ + public function testAlternatives() + { + $container = new ContainerBuilder(); + + $container->setAlias(IInterface::class, 'i'); + $container->register('f', F::class); + $container->register('i', I::class); + $container->register('j', J::class) + ->setAutowired(true); + + $pass = new AutowirePass(); + $pass->process($container); + } } class Foo @@ -813,6 +831,13 @@ public function __construct(D $d = null) } } +class J +{ + public function __construct(I $i) + { + } +} + interface CollisionInterface { }