Merge pull request #51 from matthiasnoback/issue_49

Fix reported issue
matthiasnoback · Feb 2, 2022 · c021934 · c021934
2 parents 163d732 + c080da7
commit c021934
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 23 deletions.
diff --git a/ArgumentValidator.php b/ArgumentValidator.php
@@ -30,21 +30,33 @@ public function __construct(
 
     public function validate(\ReflectionParameter $parameter, $argument)
     {
-        if ($parameter->isArray()) {
-            if ($parameter->allowsNull() && is_null($argument)) {
-                return;
-            }
-            $this->validateArrayArgument($argument);
-        } elseif ($parameter->getClass()) {
-            $this->validateObjectArgument($parameter->getClass()->getName(), $argument, $parameter->allowsNull());
+        $parameterType = $parameter->getType();
+        if ($parameterType === null) {
+            return;
+        }
+
+        if ($parameterType->getName() === 'array') {
+            $this->validateArrayArgument($parameter, $argument);
+        } elseif (class_exists($parameterType->getName())) {
+            $this->validateObjectArgument($parameterType->getName(), $argument, $parameter->allowsNull());
         }
 
         // other arguments don't need to be or can't be validated
     }
 
-    private function validateArrayArgument($argument)
+    private function validateArrayArgument(\ReflectionParameter $parameter, $argument)
     {
-        if (!is_array($argument)) {
+        if ($parameter->allowsNull() && is_null($argument)) {
+            return;
+        }
+
+        if (class_exists('Symfony\Component\ExpressionLanguage\Expression') && $argument instanceof Expression) {
+            $this->validateExpressionArgument('array', $argument, $parameter->allowsNull());
+        } else {
+            if (is_array($argument)) {
+                return;
+            }
+
             throw new TypeHintMismatchException(sprintf(
                 'Argument of type "%s" should have been an array',
                 gettype($argument)
@@ -95,14 +107,14 @@ private function validateDefinitionArgument($className, Definition $definition)
         $this->validateClass($className, $resultingClass);
     }
 
-    private function validateExpressionArgument($className, Expression $expression, $allowsNull)
+    private function validateExpressionArgument($type, Expression $expression, $allowsNull)
     {
         $expressionLanguage = new ExpressionLanguage();
 
         $this->validateExpressionSyntax($expression, $expressionLanguage);
 
         if ($this->evaluateExpressions) {
-            $this->validateExpressionResult($className, $expression, $allowsNull, $expressionLanguage);
+            $this->validateExpressionResult($type, $expression, $allowsNull, $expressionLanguage);
         }
     }
 
@@ -115,7 +127,7 @@ private function validateExpressionSyntax(Expression $expression, ExpressionLang
         }
     }
 
-    private function validateExpressionResult($className, Expression $expression, $allowsNull, ExpressionLanguage $expressionLanguage)
+    private function validateExpressionResult($expectedType, Expression $expression, $allowsNull, ExpressionLanguage $expressionLanguage)
     {
         try {
             $result = $expressionLanguage->evaluate($expression, array('container' => $this->containerBuilder));
@@ -130,20 +142,29 @@ private function validateExpressionResult($className, Expression $expression, $a
 
             throw new TypeHintMismatchException(sprintf(
                 'Argument for type-hint "%s" is an expression that evaluates to null, which is not allowed',
-                $className
+                $expectedType
             ));
         }
 
-        if (!is_object($result)) {
+        if ($expectedType === 'array' && !is_array($result)) {
             throw new TypeHintMismatchException(sprintf(
-                'Argument for type-hint "%s" is an expression that evaluates to a non-object',
-                $className
+                'Argument for type-hint "%s" is an expression that evaluates to a non-array',
+                $expectedType
             ));
         }
 
-        $resultingClass = get_class($result);
+        if (class_exists($expectedType)) {
+            if (!is_object($result)) {
+                throw new TypeHintMismatchException(sprintf(
+                    'Argument for type-hint "%s" is an expression that evaluates to a non-object',
+                    $expectedType
+                ));
+            }
 
-        $this->validateClass($className, $resultingClass);
+            $resultingClass = get_class($result);
+
+            $this->validateClass($expectedType, $resultingClass);
+        }
     }
 
     private function validateClass($expectedClassName, $actualClassName)

diff --git a/ArgumentsValidator.php b/ArgumentsValidator.php
@@ -41,7 +41,7 @@ private function shouldParameterHaveAnArgument(\ReflectionParameter $parameter)
             return false;
         }
 
-        if ($parameter->getClass() && $parameter->allowsNull()) {
+        if ($parameter->getType() instanceof \ReflectionType && $parameter->getType()->allowsNull()) {
             // e.g. LoggerInterface $logger = null
             return false;
         }

diff --git a/Error/ValidationErrorList.php b/Error/ValidationErrorList.php
@@ -11,12 +11,12 @@ public function add(ValidationErrorInterface $error)
         $this->errors[] = $error;
     }
 
-    public function count()
+    public function count(): int
     {
         return count($this->errors);
     }
 
-    public function getIterator()
+    public function getIterator(): \ArrayIterator
     {
         return new \ArrayIterator($this->errors);
     }

diff --git a/Exception/InvalidExpressionException.php b/Exception/InvalidExpressionException.php
@@ -12,7 +12,7 @@ public function __construct($expression, \Exception $exception)
                 $expression,
                 $exception->getMessage()
             ),
-            null,
+            0,
             $exception
         );
     }

diff --git a/Exception/InvalidExpressionSyntaxException.php b/Exception/InvalidExpressionSyntaxException.php
@@ -14,7 +14,7 @@ public function __construct($expression, SyntaxError $exception)
                 $expression,
                 $exception->getMessage()
             ),
-            null,
+            0,
             $exception
         );
     }

diff --git a/Tests/Functional/Fixtures/Issue49.php b/Tests/Functional/Fixtures/Issue49.php
@@ -0,0 +1,11 @@
+<?php
+declare(strict_types=1);
+
+namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Functional\Fixtures;
+
+final class Issue49
+{
+    public function __construct(array $values)
+    {
+    }
+}
diff --git a/Tests/Functional/Fixtures/issue_49.yml b/Tests/Functional/Fixtures/issue_49.yml
@@ -0,0 +1,12 @@
+parameters:
+  app_config:
+    abc:
+      - a
+      - b
+      - c
+
+services:
+  app.services.service_id:
+    class: Matthias\SymfonyServiceDefinitionValidator\Tests\Functional\Fixtures\Issue49
+    arguments:
+      - "@=parameter('app_config')['abc']"
diff --git a/Tests/Functional/FunctionalTest.php b/Tests/Functional/FunctionalTest.php
@@ -52,6 +52,16 @@ public function testIfTheServiceDefinitionsAreCorrectTheContainerWillBeCompiled(
         $this->addToAssertionCount(1);
     }
 
+    public function testIssue49(): void
+    {
+        $loader = new YamlFileLoader($this->container, new FileLocator(__DIR__ . '/Fixtures'));
+        $loader->load('issue_49.yml');
+
+        $this->container->compile();
+
+        $this->addToAssertionCount(1);
+    }
+
     public function testIfAServiceDefinitionWithAnExpressionArgumentIsCorrectTheContainerWillBeCompiled()
     {
         if (!class_exists('Symfony\Component\DependencyInjection\ExpressionLanguage')) {