From 27470de62a7202f29e9f4589a9cfbfef88e1c152 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 4 Apr 2017 15:48:08 +0200 Subject: [PATCH] [DI] Add "factory" support to named args and autowiring --- .../Compiler/AbstractRecursivePass.php | 90 ++++++++++++++++++- .../Compiler/AutowirePass.php | 39 ++++---- .../Compiler/ResolveNamedArgumentsPass.php | 47 ++-------- .../Tests/Compiler/AutowirePassTest.php | 16 ++-- .../ResolveNamedArgumentsPassTest.php | 24 ++++- 5 files changed, 141 insertions(+), 75 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php index 11a2197d04b8e..16d0d741ae086 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php @@ -12,8 +12,10 @@ namespace Symfony\Component\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\Argument\ArgumentInterface; -use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Exception\RuntimeException; +use Symfony\Component\DependencyInjection\Reference; /** * @author Nicolas Grekas @@ -73,4 +75,90 @@ protected function processValue($value, $isRoot = false) return $value; } + + /** + * @param Definition $definition + * @param bool $required + * + * @return \ReflectionFunctionAbstract|null + * + * @throws RuntimeException + */ + protected function getConstructor(Definition $definition, $required) + { + if (is_string($factory = $definition->getFactory())) { + if (!function_exists($factory)) { + throw new RuntimeException(sprintf('Unable to resolve service "%s": function "%s" does not exist.', $this->currentId, $factory)); + } + $r = new \ReflectionFunction($factory); + if (false !== $r->getFileName() && file_exists($r->getFileName())) { + $this->container->fileExists($r->getFileName()); + } + + return $r; + } + + if ($factory) { + list($class, $method) = $factory; + if ($class instanceof Reference) { + $class = $this->container->findDefinition((string) $class)->getClass(); + } elseif (null === $class) { + $class = $definition->getClass(); + } + if ('__construct' === $method) { + throw new RuntimeException(sprintf('Unable to resolve service "%s": "__construct()" cannot be used as a factory method.', $this->currentId)); + } + + return $this->getReflectionMethod(new Definition($class), $method); + } + + $class = $definition->getClass(); + + if (!$r = $this->container->getReflectionClass($class)) { + throw new RuntimeException(sprintf('Unable to resolve service "%s": class "%s" does not exist.', $this->currentId, $class)); + } + if (!$r = $r->getConstructor()) { + if ($required) { + throw new RuntimeException(sprintf('Unable to resolve service "%s": class%s has no constructor.', $this->currentId, sprintf($class !== $this->currentId ? ' "%s"' : '', $class))); + } + } elseif (!$r->isPublic()) { + throw new RuntimeException(sprintf('Unable to resolve service "%s": %s must be public.', $this->currentId, sprintf($class !== $this->currentId ? 'constructor of class "%s"' : 'its constructor', $class))); + } + + return $r; + } + + /** + * @param Definition $definition + * @param string $method + * + * @throws RuntimeException + * + * @return \ReflectionFunctionAbstract + */ + protected function getReflectionMethod(Definition $definition, $method) + { + if ('__construct' === $method) { + return $this->getConstructor($definition, true); + } + + if (!$class = $definition->getClass()) { + throw new RuntimeException(sprintf('Unable to resolve service "%s": the class is not set.', $this->currentId)); + } + + if (!$r = $this->container->getReflectionClass($class)) { + throw new RuntimeException(sprintf('Unable to resolve service "%s": class "%s" does not exist.', $this->currentId, $class)); + } + + if (!$r->hasMethod($method)) { + throw new RuntimeException(sprintf('Unable to resolve service "%s": method "%s()" does not exist.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method)); + } + + $r = $r->getMethod($method); + if (!$r->isPublic()) { + throw new RuntimeException(sprintf('Unable to resolve service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method)); + } + + return $r; + } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 5443c6e1deb55..ef77cb745e0ee 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -95,9 +95,6 @@ protected function processValue($value, $isRoot = false) if (!$value->isAutowired() || $value->isAbstract() || !$value->getClass()) { return parent::processValue($value, $isRoot); } - if ($value->getFactory()) { - throw new RuntimeException(sprintf('Service "%s" can use either autowiring or a factory, not both.', $this->currentId)); - } 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())); @@ -107,8 +104,8 @@ protected function processValue($value, $isRoot = false) $autowiredMethods = $this->getMethodsToAutowire($reflectionClass); $methodCalls = $value->getMethodCalls(); - if ($constructor = $reflectionClass->getConstructor()) { - array_unshift($methodCalls, array($constructor->name, $value->getArguments())); + if ($constructor = $this->getConstructor($value, false)) { + array_unshift($methodCalls, array($constructor, $value->getArguments())); } $methodCalls = $this->autowireCalls($reflectionClass, $methodCalls, $autowiredMethods); @@ -143,13 +140,13 @@ private function getMethodsToAutowire(\ReflectionClass $reflectionClass) $found = array(); $methodsToAutowire = array(); - if ($reflectionMethod = $reflectionClass->getConstructor()) { - $methodsToAutowire[strtolower($reflectionMethod->name)] = $reflectionMethod; - } - foreach ($reflectionClass->getMethods() as $reflectionMethod) { $r = $reflectionMethod; + if ($r->isConstructor()) { + continue; + } + while (true) { if (false !== $doc = $r->getDocComment()) { if (false !== stripos($doc, '@required') && preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@required(?:\s|\*/$)#i', $doc)) { @@ -183,19 +180,13 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC foreach ($methodCalls as $i => $call) { list($method, $arguments) = $call; - if (isset($autowiredMethods[$lcMethod = strtolower($method)]) && $autowiredMethods[$lcMethod]->isPublic()) { + if ($method instanceof \ReflectionFunctionAbstract) { + $reflectionMethod = $method; + } elseif (isset($autowiredMethods[$lcMethod = strtolower($method)]) && $autowiredMethods[$lcMethod]->isPublic()) { $reflectionMethod = $autowiredMethods[$lcMethod]; unset($autowiredMethods[$lcMethod]); } else { - if (!$reflectionClass->hasMethod($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()) { - $class = $reflectionClass->name; - throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s() must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method)); - } + $reflectionMethod = $this->getReflectionMethod($this->currentDefinition, $method); } $arguments = $this->autowireMethod($reflectionMethod, $arguments); @@ -210,7 +201,7 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC if (!$reflectionMethod->isPublic()) { $class = $reflectionClass->name; - throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s() must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method)); + throw new RuntimeException(sprintf('Cannot autowire service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method)); } $methodCalls[] = array($method, $this->autowireMethod($reflectionMethod, array())); } @@ -221,16 +212,16 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC /** * Autowires the constructor or a method. * - * @param \ReflectionMethod $reflectionMethod - * @param array $arguments + * @param \ReflectionFunctionAbstract $reflectionMethod + * @param array $arguments * * @return array The autowired arguments * * @throws RuntimeException */ - private function autowireMethod(\ReflectionMethod $reflectionMethod, array $arguments) + private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, array $arguments) { - $class = $reflectionMethod->class; + $class = $reflectionMethod instanceof \ReflectionMethod ? $reflectionMethod->class : $this->currentId; $method = $reflectionMethod->name; $parameters = $reflectionMethod->getParameters(); if (method_exists('ReflectionMethod', 'isVariadic') && $reflectionMethod->isVariadic()) { diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php index d35775e6d09ad..9f7b835100263 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php @@ -30,18 +30,11 @@ protected function processValue($value, $isRoot = false) return parent::processValue($value, $isRoot); } - $parameterBag = $this->container->getParameterBag(); - - if ($class = $value->getClass()) { - $class = $parameterBag->resolveValue($class); - } - $calls = $value->getMethodCalls(); $calls[] = array('__construct', $value->getArguments()); foreach ($calls as $i => $call) { list($method, $arguments) = $call; - $method = $parameterBag->resolveValue($method); $parameters = null; $resolvedArguments = array(); @@ -51,10 +44,14 @@ protected function processValue($value, $isRoot = false) continue; } if ('' === $key || '$' !== $key[0]) { - throw new InvalidArgumentException(sprintf('Invalid key "%s" found in arguments of method "%s" for service "%s": only integer or $named arguments are allowed.', $key, $method, $this->currentId)); + throw new InvalidArgumentException(sprintf('Invalid key "%s" found in arguments of method "%s()" for service "%s": only integer or $named arguments are allowed.', $key, $method, $this->currentId)); } - $parameters = null !== $parameters ? $parameters : $this->getParameters($class, $method); + if (null === $parameters) { + $r = $this->getReflectionMethod($value, $method); + $class = $r instanceof \ReflectionMethod ? $r->class : $this->currentId; + $parameters = $r->getParameters(); + } foreach ($parameters as $j => $p) { if ($key === '$'.$p->name) { @@ -64,7 +61,7 @@ protected function processValue($value, $isRoot = false) } } - throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": method "%s::%s" has no argument named "%s". Check your service definition.', $this->currentId, $class, $method, $key)); + throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": method "%s()" has no argument named "%s". Check your service definition.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method, $key)); } if ($resolvedArguments !== $call[1]) { @@ -84,34 +81,4 @@ protected function processValue($value, $isRoot = false) return parent::processValue($value, $isRoot); } - - /** - * @param string|null $class - * @param string $method - * - * @throws InvalidArgumentException - * - * @return array - */ - private function getParameters($class, $method) - { - if (!$class) { - throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": the class is not set.', $this->currentId)); - } - - if (!$r = $this->container->getReflectionClass($class)) { - throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": class "%s" does not exist.', $this->currentId, $class)); - } - - if (!$r->hasMethod($method)) { - throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": method "%s::%s" does not exist.', $this->currentId, $class, $method)); - } - - $method = $r->getMethod($method); - if (!$method->isPublic()) { - throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": method "%s::%s" must be public.', $this->currentId, $class, $method->name)); - } - - return $method->getParameters(); - } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index ae88a63df653b..d48b1d840916b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -611,20 +611,19 @@ public function testEmptyStringIsKept() $this->assertEquals(array(new Reference('a'), '', new Reference('lille')), $container->getDefinition('foo')->getArguments()); } - /** - * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException - * @expectedExceptionMessage Service "a" can use either autowiring or a factory, not both. - */ public function testWithFactory() { $container = new ContainerBuilder(); - $container->register('a', __NAMESPACE__.'\A') - ->setFactory('foo') + $container->register('foo', Foo::class); + $definition = $container->register('a', A::class) + ->setFactory(array(A::class, 'create')) ->setAutowired(true); $pass = new AutowirePass(); $pass->process($container); + + $this->assertEquals(array(new Reference('foo')), $definition->getArguments()); } /** @@ -662,7 +661,7 @@ public function provideNotWireableCalls() { return array( 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(null, 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setProtectedMethod() must be public.'), + array(null, 'Cannot autowire service "foo": method "Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setProtectedMethod()" must be public.'), ); } @@ -745,6 +744,9 @@ public function __construct(Foo $foo) class A { + public static function create(Foo $foo) + { + } } class B extends A diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php index 78fdbd09cd03a..8069840fe3626 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php @@ -37,8 +37,23 @@ public function testProcess() $this->assertEquals(array(array('setApiKey', array('123'))), $definition->getMethodCalls()); } + public function testWithFactory() + { + $container = new ContainerBuilder(); + + $container->register('factory', NoConstructor::class); + $definition = $container->register('foo', NoConstructor::class) + ->setFactory(array(new Reference('factory'), 'create')) + ->setArguments(array('$apiKey' => '123')); + + $pass = new ResolveNamedArgumentsPass(); + $pass->process($container); + + $this->assertSame(array(0 => '123'), $definition->getArguments()); + } + /** - * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException */ public function testClassNull() { @@ -52,7 +67,7 @@ public function testClassNull() } /** - * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException */ public function testClassNotExist() { @@ -66,7 +81,7 @@ public function testClassNotExist() } /** - * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException */ public function testClassNoConstructor() { @@ -96,4 +111,7 @@ public function testArgumentNotFound() class NoConstructor { + public static function create($apiKey) + { + } }