Skip to content

Commit 2ce36a6

Browse files
dunglasdunglas
committed
[DependencyInjection] Add a new pass to check arguments validity
1 parent 6e50129 commit 2ce36a6

File tree

7 files changed

+134
-7
lines changed

7 files changed

+134
-7
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/CheckArgumentsValidityPass.php‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\Definition;
15+
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
16+
17+
/**
18+
* Checks if arguments of methods are properly configured.
19+
*
20+
* @author Kévin Dunglas <[email protected]>
21+
* @author Nicolas Grekas <[email protected]>
22+
*/
23+
class CheckArgumentsValidityPass extends AbstractRecursivePass
24+
{
25+
/**
26+
* {@inheritdoc}
27+
*/
28+
protected function processValue($value, $isRoot = false)
29+
{
30+
if (!$value instanceof Definition) {
31+
return parent::processValue($value, $isRoot);
32+
}
33+
34+
$i = 0;
35+
foreach ($value->getArguments() as $k => $v) {
36+
if ($k !== $i++) {
37+
if (!is_int($k)) {
38+
throw new RuntimeException(sprintf('Invalid constructor argument for service "%s": integer expected but found string "%s". Check your service definition.', $this->currentId, $k));
39+
}
40+
41+
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));
42+
}
43+
}
44+
45+
foreach ($value->getMethodCalls() as $methodCall) {
46+
$i = 0;
47+
foreach ($methodCall[1] as $k => $v) {
48+
if ($k !== $i++) {
49+
if (!is_int($k)) {
50+
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));
51+
}
52+
53+
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));
54+
}
55+
}
56+
}
57+
}
58+
}

‎src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public function __construct()
6060
new AnalyzeServiceReferencesPass(true),
6161
new CheckCircularReferencesPass(),
6262
new CheckReferenceValidityPass(),
63+
new CheckArgumentsValidityPass(),
6364
));
6465

6566
$this->removingPasses = array(array(

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckArgumentsValidityPassTest.php‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Tests\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\Compiler\CheckArgumentsValidityPass;
15+
use Symfony\Component\DependencyInjection\ContainerBuilder;
16+
17+
/**
18+
* @author Kévin Dunglas <[email protected]>
19+
*/
20+
class CheckArgumentsValidityPassTest extends \PHPUnit_Framework_TestCase
21+
{
22+
public function testProcess()
23+
{
24+
$container = new ContainerBuilder();
25+
$definition = $container->register('foo');
26+
$definition->setArguments(array(null, 1, 'a'));
27+
$definition->setMethodCalls(array(
28+
array('bar', array('a', 'b')),
29+
array('baz', array('c', 'd')),
30+
));
31+
32+
$pass = new CheckArgumentsValidityPass();
33+
$pass->process($container);
34+
35+
$this->assertEquals(array(null, 1, 'a'), $container->getDefinition('foo')->getArguments());
36+
$this->assertEquals(array(
37+
array('bar', array('a', 'b')),
38+
array('baz', array('c', 'd')),
39+
), $container->getDefinition('foo')->getMethodCalls());
40+
}
41+
42+
/**
43+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
44+
* @dataProvider definitionProvider
45+
*/
46+
public function testException(array $arguments, array $methodCalls)
47+
{
48+
$container = new ContainerBuilder();
49+
$definition = $container->register('foo');
50+
$definition->setArguments($arguments);
51+
$definition->setMethodCalls($methodCalls);
52+
53+
$pass = new CheckArgumentsValidityPass();
54+
$pass->process($container);
55+
}
56+
57+
public function definitionProvider()
58+
{
59+
return array(
60+
array(array(null, 'a' => 'a'), array()),
61+
array(array(1 => 1), array()),
62+
array(array(), array(array('baz', array(null, 'a' => 'a')))),
63+
array(array(), array(array('baz', array(1 => 1)))),
64+
);
65+
}
66+
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_named_args.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
33
<services>
44
<service id="Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy">
5+
<argument />
56
<argument key="$apiKey">ABCD</argument>
67
<call method="setApiKey">
78
<argument key="$apiKey">123</argument>

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_named_args.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
services:
2-
Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy: { $apiKey: ABCD }
2+
Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy: { 0: ~, $apiKey: ABCD }
33

44
another_one:
55
class: Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy
66
arguments:
7+
0: ~
78
$apiKey: ABCD
89
calls:
910
- ['setApiKey', { $apiKey: '123' }]

‎src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,11 +701,11 @@ public function testNamedArguments()
701701
$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
702702
$loader->load('services_named_args.xml');
703703

704-
$this->assertEquals(array('$apiKey' => 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments());
704+
$this->assertEquals(array(null, '$apiKey' => 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments());
705705

706706
$container->compile();
707707

708-
$this->assertEquals(array(1 => 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments());
708+
$this->assertEquals(array(null, 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments());
709709
$this->assertEquals(array(array('setApiKey', array('123'))), $container->getDefinition(NamedArgumentsDummy::class)->getMethodCalls());
710710
}
711711
}

‎src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -447,13 +447,13 @@ public function testNamedArguments()
447447
$loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
448448
$loader->load('services_named_args.yml');
449449

450-
$this->assertEquals(array('$apiKey' => 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments());
451-
$this->assertEquals(array('$apiKey' => 'ABCD'), $container->getDefinition('another_one')->getArguments());
450+
$this->assertEquals(array(null, '$apiKey' => 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments());
451+
$this->assertEquals(array(null, '$apiKey' => 'ABCD'), $container->getDefinition('another_one')->getArguments());
452452

453453
$container->compile();
454454

455-
$this->assertEquals(array(1 => 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments());
456-
$this->assertEquals(array(1 => 'ABCD'), $container->getDefinition('another_one')->getArguments());
455+
$this->assertEquals(array(null, 'ABCD'), $container->getDefinition(NamedArgumentsDummy::class)->getArguments());
456+
$this->assertEquals(array(null, 'ABCD'), $container->getDefinition('another_one')->getArguments());
457457
$this->assertEquals(array(array('setApiKey', array('123'))), $container->getDefinition('another_one')->getMethodCalls());
458458
}
459459

0 commit comments

Comments
 (0)