Skip to content

Commit 3d20b08

Browse files
matthiasnobackmatthiasnoback
committed
Add validation for expression arguments
Arguments that are expressions were previously skipped by the argument validator. Now they will be evaluated to verify that they contain no syntax errors and will cause no problems at runtime. This fixes #6.
1 parent 0412c2c commit 3d20b08

8 files changed

+186
-10
lines changed

‎ArgumentValidator.php

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@
22

33
namespace Matthias\SymfonyServiceDefinitionValidator;
44

5+
use Matthias\SymfonyServiceDefinitionValidator\Exception\InvalidExpressionException;
6+
use Matthias\SymfonyServiceDefinitionValidator\Exception\InvalidExpressionSyntaxException;
57
use Matthias\SymfonyServiceDefinitionValidator\Exception\TypeHintMismatchException;
68
use Symfony\Component\DependencyInjection\ContainerBuilder;
79
use Symfony\Component\DependencyInjection\Definition;
10+
use Symfony\Component\DependencyInjection\ExpressionLanguage;
811
use Symfony\Component\DependencyInjection\Reference;
912
use Symfony\Component\ExpressionLanguage\Expression;
13+
use Symfony\Component\ExpressionLanguage\SyntaxError;
1014

1115
class ArgumentValidator implements ArgumentValidatorInterface
1216
{
@@ -48,11 +52,9 @@ private function validateObjectArgument($className, $argument, $allowsNull)
4852
$this->validateReferenceArgument($className, $argument);
4953
} elseif ($argument instanceof Definition) {
5054
$this->validateDefinitionArgument($className, $argument);
51-
} elseif ($argument === null && $allowsNull) {
52-
return;
5355
} elseif (class_exists('Symfony\Component\ExpressionLanguage\Expression') && $argument instanceof Expression) {
54-
// We currently have no way to validate an expression
55-
// See also https://github.com/matthiasnoback/symfony-service-definition-validator/issues/6
56+
$this->validateExpressionArgument($className, $argument, $allowsNull);
57+
} elseif ($argument === null && $allowsNull) {
5658
return;
5759
} else {
5860
throw new TypeHintMismatchException(sprintf(
@@ -79,16 +81,56 @@ private function validateDefinitionArgument($className, Definition $definition)
7981
return;
8082
}
8183

82-
if ($className === $resultingClass) {
84+
$this->validateClass($className, $resultingClass);
85+
}
86+
87+
private function validateExpressionArgument($className, Expression $expression, $allowsNull)
88+
{
89+
$expressionLanguage = new ExpressionLanguage();
90+
91+
try {
92+
$result = $expressionLanguage->evaluate($expression, array('container' => $this->containerBuilder));
93+
} catch (SyntaxError $exception) {
94+
throw new InvalidExpressionSyntaxException($expression, $exception);
95+
} catch (\Exception $exception) {
96+
throw new InvalidExpressionException($expression, $exception);
97+
}
98+
99+
if ($result === null) {
100+
if ($allowsNull) {
101+
return;
102+
}
103+
104+
throw new TypeHintMismatchException(sprintf(
105+
'Argument for type-hint "%s" is an expression that evaluates to null, which is not allowed',
106+
$className
107+
));
108+
}
109+
110+
if (!is_object($result)) {
111+
throw new TypeHintMismatchException(sprintf(
112+
'Argument for type-hint "%s" is an expression that evaluates to a non-object',
113+
$className
114+
));
115+
}
116+
117+
$resultingClass = get_class($result);
118+
119+
$this->validateClass($className, $resultingClass);
120+
}
121+
122+
private function validateClass($expectedClassName, $actualClassName)
123+
{
124+
if ($expectedClassName === $actualClassName) {
83125
return;
84126
}
85127

86-
$reflectionClass = new \ReflectionClass($resultingClass);
87-
if (!$reflectionClass->isSubclassOf($className)) {
128+
$reflectionClass = new \ReflectionClass($actualClassName);
129+
if (!$reflectionClass->isSubclassOf($expectedClassName)) {
88130
throw new TypeHintMismatchException(sprintf(
89-
'Argument for type-hint "%s" should point to a service of class "%s"',
90-
$className,
91-
$resultingClass
131+
'Argument for type-hint "%s" points to a service of class "%s"',
132+
$expectedClassName,
133+
$actualClassName
92134
));
93135
}
94136
}

‎Exception/InvalidExpressionException.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
namespace Matthias\SymfonyServiceDefinitionValidator\Exception;
4+
5+
class InvalidExpressionException extends \RuntimeException implements DefinitionValidationExceptionInterface
6+
{
7+
public function __construct($expression, \Exception $exception)
8+
{
9+
parent::__construct(
10+
sprintf(
11+
'Expression "%s" could not be evaluated: %s',
12+
$expression,
13+
$exception->getMessage()
14+
),
15+
null,
16+
$exception
17+
);
18+
}
19+
}

‎Exception/InvalidExpressionSyntaxException.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
namespace Matthias\SymfonyServiceDefinitionValidator\Exception;
4+
5+
use Symfony\Component\ExpressionLanguage\SyntaxError;
6+
7+
class InvalidExpressionSyntaxException extends \RuntimeException implements DefinitionValidationExceptionInterface
8+
{
9+
public function __construct($expression, SyntaxError $exception)
10+
{
11+
parent::__construct(
12+
sprintf(
13+
'The syntax of expression "%s" is invalid: %s',
14+
$expression,
15+
$exception->getMessage()
16+
),
17+
null,
18+
$exception
19+
);
20+
}
21+
}

‎README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ problems can be recognized:
2626
- Missing required arguments for method calls
2727
- Type-hint mismatches for constructor arguments (array or class/interface)
2828
- Type-hint mismatches for method call arguments (array or class/interface)
29+
- Syntax errors in arguments that are
30+
[expressions](http://symfony.com/doc/current/book/service_container.html#using-the-expression-language)
31+
- Expressions that cause errors when being evaluated
2932

3033
This will prevent lots of run-time problems, and will warn you about inconsistencies in your
3134
service definitions early on.

‎TODO

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
# TODO
22

33
- Rewrite tests to verify positive results instead of just catching negative results.
4+
- Make Expression evaluation optional

‎Tests/ArgumentValidatorTest.php

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
namespace Matthias\SymfonyServiceDefinitionValidator\Tests;
44

55
use Matthias\SymfonyServiceDefinitionValidator\ArgumentValidator;
6+
use Matthias\SymfonyServiceDefinitionValidator\Exception\TypeHintMismatchException;
67
use Symfony\Component\DependencyInjection\ContainerBuilder;
78
use Symfony\Component\DependencyInjection\Definition;
89
use Symfony\Component\DependencyInjection\Reference;
10+
use Symfony\Component\ExpressionLanguage\Expression;
911

1012
class ArgumentValidatorTest extends \PHPUnit_Framework_TestCase
1113
{
@@ -82,6 +84,81 @@ public function testFailsWhenParameterHasArrayTypeHintButArgumentIsNotArray()
8284
$validator->validate($parameter, $argument);
8385
}
8486

87+
public function testFailsWhenResultOfExpressionIsNotAnObjectOfTheExpectedClass()
88+
{
89+
$class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithTypeHintedConstructorArgument';
90+
91+
$parameter = new \ReflectionParameter(array($class, '__construct'), 'expected');
92+
$argument = new Expression('service("service_wrong_class")');
93+
94+
$containerBuilder = new ContainerBuilder();
95+
$containerBuilder->setDefinition('service_wrong_class', new Definition('stdClass'));
96+
97+
$validator = new ArgumentValidator($containerBuilder, $this->createMockResultingClassResolver());
98+
99+
$this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\TypeHintMismatchException', 'ExpectedClass');
100+
101+
$validator->validate($parameter, $argument);
102+
}
103+
104+
public function testFailsWhenResultOfExpressionIsNotAnObject()
105+
{
106+
$class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithTypeHintedConstructorArgument';
107+
108+
$parameter = new \ReflectionParameter(array($class, '__construct'), 'expected');
109+
$argument = new Expression('"a string"');
110+
111+
$validator = new ArgumentValidator(new ContainerBuilder(), $this->createMockResultingClassResolver());
112+
113+
$this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\TypeHintMismatchException', 'ExpectedClass');
114+
115+
$validator->validate($parameter, $argument);
116+
}
117+
118+
public function testFailsWhenResultOfExpressionIsNullButNullIsNotAllowed()
119+
{
120+
$class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithTypeHintedOptionalConstructorArgument';
121+
122+
$parameter = new \ReflectionParameter(array($class, '__construct'), 'expected');
123+
$argument = new Expression('null');
124+
125+
$validator = new ArgumentValidator(new ContainerBuilder(), $this->createMockResultingClassResolver());
126+
127+
try {
128+
$validator->validate($parameter, $argument);
129+
} catch (TypeHintMismatchException $exception) {
130+
$this->fail('null argument should be allowed');
131+
}
132+
}
133+
134+
public function testFailsIfSyntaxOfExpressionIsInvalid()
135+
{
136+
$class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithTypeHintedConstructorArgument';
137+
138+
$parameter = new \ReflectionParameter(array($class, '__construct'), 'expected');
139+
$argument = new Expression('*invalid expression');
140+
141+
$validator = new ArgumentValidator(new ContainerBuilder(), $this->createMockResultingClassResolver());
142+
143+
$this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\InvalidExpressionSyntaxException');
144+
145+
$validator->validate($parameter, $argument);
146+
}
147+
148+
public function testFailsIfExpressionCouldNotBeEvaluated()
149+
{
150+
$class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithTypeHintedConstructorArgument';
151+
152+
$parameter = new \ReflectionParameter(array($class, '__construct'), 'expected');
153+
$argument = new Expression('service("invalid service")');
154+
155+
$validator = new ArgumentValidator(new ContainerBuilder(), $this->createMockResultingClassResolver());
156+
157+
$this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\InvalidExpressionException');
158+
159+
$validator->validate($parameter, $argument);
160+
}
161+
85162
private function createMockResultingClassResolver()
86163
{
87164
return $this->getMock('Matthias\SymfonyServiceDefinitionValidator\ResultingClassResolverInterface');

‎Tests/Fixtures/ClassWithTypeHintedOptionalConstructorArgument.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures;
4+
5+
class ClassWithTypeHintedOptionalConstructorArgument
6+
{
7+
public function __construct(ExpectedClass $expected = null)
8+
{
9+
}
10+
}

‎Tests/Functional/Fixtures/service_definition_with_expression.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
44
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
55
<services>
6+
<service id="transport"
7+
class="Matthias\SymfonyServiceDefinitionValidator\Tests\Functional\Fixtures\Transport">
8+
</service>
69
<service id="service_with_expression_argument"
710
class="Matthias\SymfonyServiceDefinitionValidator\Tests\Functional\Fixtures\Mailer">
811
<argument type="expression">service('transport')</argument>

0 commit comments

Comments
 (0)