Skip to content

Commit

Permalink
feature #21383 [DependencyInjection] Add support for named arguments …
Browse files Browse the repository at this point in the history
…(dunglas, nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DependencyInjection] Add support for named arguments

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | todo

This PR introduces named arguments for services definitions. It's especially useful to inject parameters in an autowired service. It is (at least partially) an alternative to #21376 and #20738.

Usage:

```yml
services:
    _defaults: { autowire: true }
    Acme\NewsletterManager: { $apiKey: "%mandrill_api_key%" }

# Alternative (traditional) syntax
services:
    newsletter_manager:
        class: Acme\NewsletterManager
        arguments:
            $apiKey: "%mandrill_api_key%"
        autowire: true
```
```php
use Doctrine\ORM\EntityManager;
use Psr\Log\LoggerInterface;

namespace Acme;

class NewsletterManager
{
    private $logger;
    private $em;
    private $apiKey;

    public function __construct(LoggerInterface $logger, EntityManager $em, $apiKey)
    {
        $this->logger = $logger;
        $this->em = $em;
        $this->apiKey = $apiKey;
    }
}
```

Commits
-------

8a126c8537 [DI] Deprecate string keys in arguments
2ce36a6074 [DependencyInjection] Add a new pass to check arguments validity
6e501296f9 [DependencyInjection] Add support for named arguments
  • Loading branch information
fabpot committed Feb 13, 2017
2 parents 3b8afe7 + 6f58dae commit 3b9c55b
Show file tree
Hide file tree
Showing 14 changed files with 447 additions and 12 deletions.
8 changes: 5 additions & 3 deletions ChildDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,14 @@ public function getArgument($index)
*/
public function replaceArgument($index, $value)
{
if (!is_int($index)) {
if (is_int($index)) {
$this->arguments['index_'.$index] = $value;
} elseif (0 === strpos($index, '$')) {
$this->arguments[$index] = $value;
} else {
throw new InvalidArgumentException('$index must be an integer.');
}

$this->arguments['index_'.$index] = $value;

return $this;
}
}
Expand Down
58 changes: 58 additions & 0 deletions Compiler/CheckArgumentsValidityPass.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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\RuntimeException;

/**
* Checks if arguments of methods are properly configured.
*
* @author Kévin Dunglas <dunglas@gmail.com>
* @author Nicolas Grekas <p@tchwork.com>
*/
class CheckArgumentsValidityPass extends AbstractRecursivePass
{
/**
* {@inheritdoc}
*/
protected function processValue($value, $isRoot = false)
{
if (!$value instanceof Definition) {
return parent::processValue($value, $isRoot);
}

$i = 0;
foreach ($value->getArguments() as $k => $v) {
if ($k !== $i++) {
if (!is_int($k)) {
throw new RuntimeException(sprintf('Invalid constructor argument for service "%s": integer expected but found string "%s". Check your service definition.', $this->currentId, $k));
}

throw new RuntimeException(sprintf('Invalid constructor argument %d for service "%s": argument %d must be defined before. Check your service definition.', 1 + $k, $this->currentId, $i));
}
}

foreach ($value->getMethodCalls() as $methodCall) {
$i = 0;
foreach ($methodCall[1] as $k => $v) {
if ($k !== $i++) {
if (!is_int($k)) {
throw new RuntimeException(sprintf('Invalid argument for method call "%s" of service "%s": integer expected but found string "%s". Check your service definition.', $methodCall[0], $this->currentId, $k));
}

throw new RuntimeException(sprintf('Invalid argument %d for method call "%s" of service "%s": argument %d must be defined before. Check your service definition.', 1 + $k, $methodCall[0], $this->currentId, $i));
}
}
}
}
}
2 changes: 2 additions & 0 deletions Compiler/PassConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,14 @@ public function __construct()
new ResolveFactoryClassPass(),
new FactoryReturnTypePass($resolveClassPass),
new CheckDefinitionValidityPass(),
new ResolveNamedArgumentsPass(),
new AutowirePass(),
new ResolveReferencesToAliasesPass(),
new ResolveInvalidReferencesPass(),
new AnalyzeServiceReferencesPass(true),
new CheckCircularReferencesPass(),
new CheckReferenceValidityPass(),
new CheckArgumentsValidityPass(),
));

$this->removingPasses = array(array(
Expand Down
5 changes: 3 additions & 2 deletions Compiler/ResolveDefinitionTemplatesPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,12 @@ private function doResolveDefinition(ChildDefinition $definition)
continue;
}

if (0 !== strpos($k, 'index_')) {
if (0 === strpos($k, 'index_')) {
$index = (int) substr($k, strlen('index_'));
} elseif (0 !== strpos($k, '$')) {
throw new RuntimeException(sprintf('Invalid argument key "%s" found.', $k));
}

$index = (int) substr($k, strlen('index_'));
$def->replaceArgument($index, $v);
}

Expand Down
117 changes: 117 additions & 0 deletions Compiler/ResolveNamedArgumentsPass.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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 <dunglas@gmail.com>
*/
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;
$resolvedArguments = array();

foreach ($arguments as $key => $argument) {
if (is_int($key) || '' === $key || '$' !== $key[0]) {
if (!is_int($key)) {
@trigger_error(sprintf('Using key "%s" for defining arguments of method "%s" for service "%s" is deprecated since Symfony 3.3 and will throw an exception in 4.0. Use no keys or $named arguments instead.', $key, $method, $this->currentId), E_USER_DEPRECATED);
}
$resolvedArguments[] = $argument;
continue;
}

$parameters = null !== $parameters ? $parameters : $this->getParameters($class, $method);

foreach ($parameters as $j => $p) {
if ($key === '$'.$p->name) {
$resolvedArguments[$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 ($resolvedArguments !== $call[1]) {
ksort($resolvedArguments);
$calls[$i][1] = $resolvedArguments;
}
}

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();
}
}
16 changes: 10 additions & 6 deletions Definition.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ public function addArgument($argument)
/**
* Sets a specific argument.
*
* @param int $index
* @param mixed $argument
* @param int|string $index
* @param mixed $argument
*
* @return $this
*
Expand All @@ -203,10 +203,14 @@ public function replaceArgument($index, $argument)
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;
Expand All @@ -225,16 +229,16 @@ public function getArguments()
/**
* Gets an argument to pass to the service constructor/factory method.
*
* @param int $index
* @param int|string $index
*
* @return mixed The argument value
*
* @throws OutOfBoundsException When the argument does not exist
*/
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];
Expand Down
18 changes: 17 additions & 1 deletion Loader/YamlFileLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,22 @@ private function parseDefaults(array &$content, $file)
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.
*
Expand All @@ -273,7 +289,7 @@ private function parseDefinition($id, $service, $file, array $defaults)
return;
}

if (is_array($service) && array_values($service) === $service) {
if (is_array($service) && $this->isUsingShortSyntax($service)) {
$service = array('arguments' => $service);
}

Expand Down
66 changes: 66 additions & 0 deletions Tests/Compiler/CheckArgumentsValidityPassTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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\CheckArgumentsValidityPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class CheckArgumentsValidityPassTest extends \PHPUnit_Framework_TestCase
{
public function testProcess()
{
$container = new ContainerBuilder();
$definition = $container->register('foo');
$definition->setArguments(array(null, 1, 'a'));
$definition->setMethodCalls(array(
array('bar', array('a', 'b')),
array('baz', array('c', 'd')),
));

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

$this->assertEquals(array(null, 1, 'a'), $container->getDefinition('foo')->getArguments());
$this->assertEquals(array(
array('bar', array('a', 'b')),
array('baz', array('c', 'd')),
), $container->getDefinition('foo')->getMethodCalls());
}

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @dataProvider definitionProvider
*/
public function testException(array $arguments, array $methodCalls)
{
$container = new ContainerBuilder();
$definition = $container->register('foo');
$definition->setArguments($arguments);
$definition->setMethodCalls($methodCalls);

$pass = new CheckArgumentsValidityPass();
$pass->process($container);
}

public function definitionProvider()
{
return array(
array(array(null, 'a' => 'a'), array()),
array(array(1 => 1), array()),
array(array(), array(array('baz', array(null, 'a' => 'a')))),
array(array(), array(array('baz', array(1 => 1)))),
);
}
}

0 comments on commit 3b9c55b

Please sign in to comment.