From 6e501296f914e903621ccaa02c6619034e2bcc63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 23 Jan 2017 23:09:54 +0100 Subject: [PATCH] [DependencyInjection] Add support for named arguments --- .../Compiler/PassConfig.php | 1 + .../Compiler/ResolveNamedArgumentsPass.php | 113 ++++++++++++++++++ .../DependencyInjection/Definition.php | 16 ++- .../Loader/YamlFileLoader.php | 18 ++- .../ResolveNamedArgumentsPassTest.php | 98 +++++++++++++++ .../Tests/Fixtures/NamedArgumentsDummy.php | 17 +++ .../Fixtures/xml/services_named_args.xml | 11 ++ .../Fixtures/yaml/services_named_args.yml | 9 ++ .../Tests/Loader/XmlFileLoaderTest.php | 15 +++ .../Tests/Loader/YamlFileLoaderTest.php | 17 +++ 10 files changed, 308 insertions(+), 7 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/NamedArgumentsDummy.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_named_args.xml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_named_args.yml diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php index c21a58f6b7..ba6117cb17 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php @@ -53,6 +53,7 @@ class PassConfig new ResolveFactoryClassPass(), new FactoryReturnTypePass($resolveClassPass), new CheckDefinitionValidityPass(), + new ResolveNamedArgumentsPass(), new AutowirePass(), new ResolveReferencesToAliasesPass(), new ResolveInvalidReferencesPass(), diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php new file mode 100644 index 0000000000..16e7138b70 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php @@ -0,0 +1,113 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; + +/** + * Resolves named arguments to their corresponding numeric index. + * + * @author Kévin Dunglas + */ +class ResolveNamedArgumentsPass extends AbstractRecursivePass +{ + /** + * {@inheritdoc} + */ + protected function processValue($value, $isRoot = false) + { + if (!$value instanceof Definition) { + 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; + + foreach ($arguments as $key => $argument) { + if (is_int($key) || '' === $key || '$' !== $key[0]) { + continue; + } + + $parameters = null !== $parameters ? $parameters : $this->getParameters($class, $method); + + foreach ($parameters as $j => $p) { + if ($key === '$'.$p->name) { + unset($arguments[$key]); + $arguments[$j] = $argument; + + continue 2; + } + } + + 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)); + } + + if ($arguments !== $call[1]) { + ksort($arguments); + $calls[$i][1] = $arguments; + } + } + + list(, $arguments) = array_pop($calls); + + if ($arguments !== $value->getArguments()) { + $value->setArguments($arguments); + } + if ($calls !== $value->getMethodCalls()) { + $value->setMethodCalls($calls); + } + + 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/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index 4feddafc99..7cd781442c 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -190,8 +190,8 @@ class Definition /** * Sets a specific argument. * - * @param int $index - * @param mixed $argument + * @param int|string $index + * @param mixed $argument * * @return $this * @@ -203,10 +203,14 @@ class Definition throw new OutOfBoundsException('Cannot replace arguments if none have been configured yet.'); } - if ($index < 0 || $index > count($this->arguments) - 1) { + if (is_int($index) && ($index < 0 || $index > count($this->arguments) - 1)) { throw new OutOfBoundsException(sprintf('The index "%d" is not in the range [0, %d].', $index, count($this->arguments) - 1)); } + if (!array_key_exists($index, $this->arguments)) { + throw new OutOfBoundsException(sprintf('The argument "%s" doesn\'t exist.', $index)); + } + $this->arguments[$index] = $argument; return $this; @@ -225,7 +229,7 @@ class Definition /** * Gets an argument to pass to the service constructor/factory method. * - * @param int $index + * @param int|string $index * * @return mixed The argument value * @@ -233,8 +237,8 @@ class Definition */ public function getArgument($index) { - if ($index < 0 || $index > count($this->arguments) - 1) { - throw new OutOfBoundsException(sprintf('The index "%d" is not in the range [0, %d].', $index, count($this->arguments) - 1)); + if (!array_key_exists($index, $this->arguments)) { + throw new OutOfBoundsException(sprintf('The argument "%s" doesn\'t exist.', $index)); } return $this->arguments[$index]; diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index caa7435078..e9017a8e9c 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -254,6 +254,22 @@ class YamlFileLoader extends FileLoader return $defaults; } + /** + * @param array $service + * + * @return bool + */ + private function isUsingShortSyntax(array $service) + { + foreach ($service as $key => $value) { + if (is_string($key) && ('' === $key || '$' !== $key[0])) { + return false; + } + } + + return true; + } + /** * Parses a definition. * @@ -273,7 +289,7 @@ class YamlFileLoader extends FileLoader return; } - if (is_array($service) && array_values($service) === $service) { + if (is_array($service) && $this->isUsingShortSyntax($service)) { $service = array('arguments' => $service); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php new file mode 100644 index 0000000000..a933b950df --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php @@ -0,0 +1,98 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Tests\Compiler; + +use Symfony\Component\DependencyInjection\Compiler\ResolveNamedArgumentsPass; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy; + +/** + * @author Kévin Dunglas + */ +class ResolveNamedArgumentsPassTest extends \PHPUnit_Framework_TestCase +{ + public function testProcess() + { + $container = new ContainerBuilder(); + + $definition = $container->register(NamedArgumentsDummy::class, NamedArgumentsDummy::class); + $definition->setArguments(array(0 => new Reference('foo'), '$apiKey' => '123')); + $definition->addMethodCall('setApiKey', array('$apiKey' => '123')); + + $pass = new ResolveNamedArgumentsPass(); + $pass->process($container); + + $this->assertEquals(array(0 => new Reference('foo'), 1 => '123'), $definition->getArguments()); + $this->assertEquals(array(array('setApiKey', array('123'))), $definition->getMethodCalls()); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + */ + public function testClassNull() + { + $container = new ContainerBuilder(); + + $definition = $container->register(NamedArgumentsDummy::class); + $definition->setArguments(array('$apiKey' => '123')); + + $pass = new ResolveNamedArgumentsPass(); + $pass->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + */ + public function testClassNotExist() + { + $container = new ContainerBuilder(); + + $definition = $container->register(NotExist::class, NotExist::class); + $definition->setArguments(array('$apiKey' => '123')); + + $pass = new ResolveNamedArgumentsPass(); + $pass->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + */ + public function testClassNoConstructor() + { + $container = new ContainerBuilder(); + + $definition = $container->register(NoConstructor::class, NoConstructor::class); + $definition->setArguments(array('$apiKey' => '123')); + + $pass = new ResolveNamedArgumentsPass(); + $pass->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + */ + public function testArgumentNotFound() + { + $container = new ContainerBuilder(); + + $definition = $container->register(NamedArgumentsDummy::class, NamedArgumentsDummy::class); + $definition->setArguments(array('$notFound' => '123')); + + $pass = new ResolveNamedArgumentsPass(); + $pass->process($container); + } +} + +class NoConstructor +{ +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/NamedArgumentsDummy.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/NamedArgumentsDummy.php new file mode 100644 index 0000000000..05cfc1e945 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/NamedArgumentsDummy.php @@ -0,0 +1,17 @@ + + */ +class NamedArgumentsDummy +{ + public function __construct(CaseSensitiveClass $c, $apiKey) + { + } + + public function setApiKey($apiKey) + { + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_named_args.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_named_args.xml new file mode 100644 index 0000000000..051dbf1233 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_named_args.xml @@ -0,0 +1,11 @@ + + + + + ABCD + + 123 + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_named_args.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_named_args.yml new file mode 100644 index 0000000000..1ce7477d5c --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_named_args.yml @@ -0,0 +1,9 @@ +services: + Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy: { $apiKey: ABCD } + + another_one: + class: Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy + arguments: + $apiKey: ABCD + calls: + - ['setApiKey', { $apiKey: '123' }] diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index 623768ba4d..9cb883a61a 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -24,6 +24,7 @@ use Symfony\Component\Config\Resource\DirectoryResource; use Symfony\Component\Config\Resource\FileResource; use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass; use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype; +use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy; use Symfony\Component\ExpressionLanguage\Expression; class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase @@ -693,4 +694,18 @@ class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase $this->assertSame(array('setFoo'), $container->getDefinition('no_defaults_child')->getAutowiredCalls()); $this->assertSame(array(), $container->getDefinition('with_defaults_child')->getAutowiredCalls()); } + + public function testNamedArguments() + { + $container = new ContainerBuilder(); + $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml')); + $loader->load('services_named_args.xml'); + + $this->assertEquals(array('$apiKey' => 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments()); + + $container->compile(); + + $this->assertEquals(array(1 => 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments()); + $this->assertEquals(array(array('setApiKey', array('123'))), $container->getDefinition(NamedArgumentsDummy::class)->getMethodCalls()); + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index de417b59cc..66ce2d8fe5 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -24,6 +24,7 @@ use Symfony\Component\Config\Resource\DirectoryResource; use Symfony\Component\Config\Resource\FileResource; use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass; use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype; +use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy; use Symfony\Component\ExpressionLanguage\Expression; class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase @@ -440,6 +441,22 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array('getbar' => array('bar' => new Reference('bar'))), $container->getDefinition('foo')->getOverriddenGetters()); } + public function testNamedArguments() + { + $container = new ContainerBuilder(); + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('services_named_args.yml'); + + $this->assertEquals(array('$apiKey' => 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments()); + $this->assertEquals(array('$apiKey' => 'ABCD'), $container->getDefinition('another_one')->getArguments()); + + $container->compile(); + + $this->assertEquals(array(1 => 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments()); + $this->assertEquals(array(1 => 'ABCD'), $container->getDefinition('another_one')->getArguments()); + $this->assertEquals(array(array('setApiKey', array('123'))), $container->getDefinition('another_one')->getMethodCalls()); + } + /** * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException * @expectedExceptionMessage The value of the "decorates" option for the "bar" service must be the id of the service without the "@" prefix (replace "@foo" with "foo").