Fixed some false negatives, renamed some classes

Author: matthiasnoback

ArgumentValidator.php

@@ -4,6 +4,7 @@
 
 use Matthias\SymfonyServiceDefinitionValidator\Exception\TypeHintMismatchException;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
 
 class ArgumentValidator implements ArgumentValidatorInterface
@@ -14,19 +15,24 @@ class ArgumentValidator implements ArgumentValidatorInterface
     public function __construct(
         ContainerBuilder $containerBuilder,
         ResultingClassResolverInterface $resultingClassResolver
-    )
-    {
+    ) {
         $this->containerBuilder = $containerBuilder;
         $this->resultingClassResolver = $resultingClassResolver;
     }
 
     public function validate(\ReflectionParameter $parameter, $argument)
     {
+        if ($argument === null && $parameter->allowsNull()) {
+            return;
+        }
+
         if ($parameter->isArray()) {
             $this->validateArrayArgument($argument);
         } elseif ($parameter->getClass()) {
             $this->validateObjectArgument($parameter->getClass()->getName(), $argument);
         }
+
+        // other argument don't need to be or can't be validated
     }
 
     private function validateArrayArgument($argument)
@@ -41,18 +47,30 @@ private function validateArrayArgument($argument)
 
     private function validateObjectArgument($className, $argument)
     {
-        if (!($argument instanceof Reference)) {
+        if ($argument instanceof Reference) {
+            $this->validateReferenceArgument($className, $argument);
+        } elseif ($argument instanceof Definition) {
+            $this->validateDefinitionArgument($className, $argument);
+        } else {
             throw new TypeHintMismatchException(sprintf(
-                'Type-hint "%s" requires this argument to be an instance of Symfony\Component\DependencyInjection\Reference',
+                'Type-hint "%s" requires this argument to be a reference to a service or an inline service definition',
                 $className
             ));
         }
+    }
 
+    private function validateReferenceArgument($className, Reference $reference)
+    {
         // the __toString method of a Reference is the referenced service id
-        $referencedServiceId = (string)$argument;
+        $referencedServiceId = (string)$reference;
         $definition = $this->containerBuilder->findDefinition($referencedServiceId);
         // we don't have to check if the definition exists, since the ContainerBuilder itself does that already
 
+        $this->validateDefinitionArgument($className, $definition);
+    }
+
+    private function validateDefinitionArgument($className, Definition $definition)
+    {
         $resultingClass = $this->resultingClassResolver->resolve($definition);
         if ($resultingClass === null) {
             return;
@@ -65,7 +83,7 @@ private function validateObjectArgument($className, $argument)
         $reflectionClass = new \ReflectionClass($resultingClass);
         if (!$reflectionClass->isSubclassOf($className)) {
             throw new TypeHintMismatchException(sprintf(
-                'Argument with type-hint "%s" is a reference to a service of class "%s"',
+                'Argument for type-hint "%s" should point to a service of class "%s"',
                 $className,
                 $resultingClass
             ));

ArgumentsValidator.php

@@ -17,6 +17,7 @@ public function validate(\ReflectionMethod $method, array $arguments)
     {
         foreach ($method->getParameters() as $parameterNumber => $parameter) {
             $argument = $this->resolveArgument($parameterNumber, $parameter, $arguments);
+
             $this->argumentValidator->validate($parameter, $argument);
         }
     }
@@ -31,7 +32,8 @@ private function resolveArgument($parameterIndex, \ReflectionParameter $paramete
         }
 
         if (!$parameter->isOptional()) {
-            throw new MissingRequiredArgumentException($parameter->getDeclaringClass()->getName(), $parameter->getName());
+            throw new MissingRequiredArgumentException($parameter->getDeclaringClass()->getName(), $parameter->getName(
+            ));
         }
 
         return null;

BatchServiceDefinitionValidator.php

@@ -14,8 +14,7 @@ class BatchServiceDefinitionValidator implements BatchServiceDefinitionValidator
     public function __construct(
         ServiceDefinitionValidatorInterface $serviceDefinitionValidator,
         ValidationErrorFactoryInterface $validationErrorFactory
-    )
-    {
+    ) {
         $this->serviceDefinitionValidator = $serviceDefinitionValidator;
         $this->validationErrorFactory = $validationErrorFactory;
     }

ConstructorResolver.php

@@ -15,8 +15,7 @@ class ConstructorResolver implements ConstructorResolverInterface
     public function __construct(
         ContainerBuilder $containerBuilder,
         ResultingClassResolverInterface $resultingClassResolver
-    )
-    {
+    ) {
         $this->containerBuilder = $containerBuilder;
         $this->resultingClassResolver = $resultingClassResolver;
     }

MethodCallsValidator.php

@@ -13,8 +13,7 @@ class MethodCallsValidator implements MethodCallsValidatorInterface
     public function __construct(
         ResultingClassResolverInterface $resultingClassResolver,
         ArgumentsValidatorInterface $argumentsValidator
-    )
-    {
+    ) {
         $this->resultingClassResolver = $resultingClassResolver;
         $this->argumentsValidator = $argumentsValidator;
     }

ServiceDefinitionValidator.php

@@ -20,8 +20,7 @@ public function __construct(
         ContainerBuilder $containerBuilder,
         DefinitionArgumentsValidatorInterface $definitionArgumentsValidator,
         MethodCallsValidatorInterface $methodCallsValidator
-    )
-    {
+    ) {
         $this->containerBuilder = $containerBuilder;
         $this->definitionArgumentsValidator = $definitionArgumentsValidator;
         $this->methodCallsValidator = $methodCallsValidator;

ServiceDefinitionValidatorFactory.php

@@ -23,5 +23,4 @@ public function create(ContainerBuilder $containerBuilder)
 
         return $validator;
     }
-
 }

TODO

@@ -0,0 +1 @@
+- Rewrite tests to verify positive results instead of just catching negative results.

Tests/ArgumentValidatorTest.php

@@ -9,25 +9,50 @@
 
 class ArgumentValidatorTest extends \PHPUnit_Framework_TestCase
 {
-    public function testFailsWhenParameterHasTypeHintButNoReferenceWasProvidedAsArgument()
+    private $containerBuilder;
+
+    protected function setUp()
+    {
+        $this->containerBuilder = new ContainerBuilder();
+    }
+
+    public function testFailsWhenParameterHasTypeHintButNoReferenceOrDefinitionWasProvidedAsArgument()
+    {
+        $class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithTypeHintedConstructorArgument';
+
+        $validator = new ArgumentValidator($this->containerBuilder, $this->createMockResultingClassResolver());
+
+        $this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\TypeHintMismatchException', 'reference');
+        $validator->validate(new \ReflectionParameter(array($class, '__construct'), 'expected'), new \stdClass());
+    }
+
+    public function testFailsWhenParameterHasTypeHintForObjectButArgumentIsDefinitionForServiceOfWrongType()
     {
-        $class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithTypeHintedDateTimeConstructorArgument';
-        $containerBuilder = new ContainerBuilder();
+        $class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithTypeHintedConstructorArgument';
+        $this->containerBuilder = new ContainerBuilder();
 
-        $validator = new ArgumentValidator($containerBuilder, $this->createMockResultingClassResolver());
+        $inlineDefinition = new Definition();
+
+        $resultingClassResolver = $this->createMockResultingClassResolver();
+        $resultingClassResolver
+            ->expects($this->once())
+            ->method('resolve')
+            ->with($inlineDefinition)
+            ->will($this->returnValue('Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\WrongClass'));
+        $validator = new ArgumentValidator($this->containerBuilder, $resultingClassResolver);
 
-        $this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\TypeHintMismatchException', 'Reference');
-        $validator->validate(new \ReflectionParameter(array($class, '__construct'), 'date'), new \stdClass());
+        $this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\TypeHintMismatchException', 'ExpectedClass');
+        $validator->validate(new \ReflectionParameter(array($class, '__construct'), 'expected'), $inlineDefinition);
     }
 
-    public function testFailsWhenParameterHasTypeHintButArgumentIsReferenceToServiceOfWrongTypeWasProvided()
+    public function testFailsWhenParameterHasTypeHintForObjectButArgumentIsReferenceToServiceOfWrongType()
     {
-        $class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithTypeHintedDateTimeConstructorArgument';
-        $containerBuilder = new ContainerBuilder();
+        $class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithTypeHintedConstructorArgument';
+        $this->containerBuilder = new ContainerBuilder();
         $definition = new Definition();
-        $containerBuilder->setDefinition('referenced_service', $definition);
+        $this->containerBuilder->setDefinition('referenced_service', $definition);
 
-        $parameter = new \ReflectionParameter(array($class, '__construct'), 'date');
+        $parameter = new \ReflectionParameter(array($class, '__construct'), 'expected');
         $argument = new Reference('referenced_service');
 
         $resultingClassResolver = $this->createMockResultingClassResolver();
@@ -36,16 +61,16 @@ public function testFailsWhenParameterHasTypeHintButArgumentIsReferenceToService
             ->method('resolve')
             ->with($definition)
             ->will($this->returnValue('stdClass'));
-        $validator = new ArgumentValidator($containerBuilder, $resultingClassResolver);
+        $validator = new ArgumentValidator($this->containerBuilder, $resultingClassResolver);
 
-        $this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\TypeHintMismatchException', 'DateTime');
+        $this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\TypeHintMismatchException', 'ExpectedClass');
 
         $validator->validate($parameter, $argument);
     }
 
     public function testFailsWhenParameterHasArrayTypeHintButArgumentIsNotArray()
     {
-        $class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithArrayConstructorArgument';
+        $class = 'Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\ClassWithRequiredArrayConstructorArgument';
 
         $parameter = new \ReflectionParameter(array($class, '__construct'), 'options');
         $argument = 'string';

Tests/Fixtures/ClassWithArrayConstructorArgument.php renamed to Tests/Fixtures/ClassWithRequiredArrayConstructorArgument.php

@@ -2,7 +2,7 @@
 
 namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures;
 
-class ClassWithArrayConstructorArgument
+class ClassWithRequiredArrayConstructorArgument
 {
     public function __construct(array $options)
     {

Tests/Fixtures/ClassWithTypeHintedConstructorArgument.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures;
+
+class ClassWithTypeHintedConstructorArgument
+{
+    public function __construct(ExpectedClass $expected)
+    {
+    }
+}

Tests/Fixtures/ClassWithTypeHintedDateTimeConstructorArgument.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures;
+
+class ExpectedClass
+{
+}

Tests/Fixtures/ExpectedClass.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures;
+
+class WrongClass
+{
+}

Tests/Fixtures/WrongClass.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Functional\Fixtures;
+
+class Catalogue
+{
+}

Tests/Functional/Fixtures/Catalogue.php

@@ -6,6 +6,5 @@ class Factory
 {
     public function create($argument1)
     {
-
     }
 }

Tests/Functional/Fixtures/Factory.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Functional\Fixtures;
+
+class MailManager
+{
+    public function __construct(Mailer $mailer = null)
+    {
+    }
+}

Tests/Functional/Fixtures/MailManager.php

@@ -2,10 +2,9 @@
 
 namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Functional\Fixtures;
 
-class Product
+class Mailer
 {
-    public function __construct(Price $price)
+    public function __construct(Transport $transport)
     {
-
     }
 }

Tests/Functional/Fixtures/Product.php renamed to Tests/Functional/Fixtures/Mailer.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Functional\Fixtures;
+
+class Translator
+{
+    public function __construct(Catalogue $catalogue)
+    {
+    }
+}

Tests/Functional/Fixtures/Translator.php

@@ -2,15 +2,13 @@
 
 namespace Matthias\SymfonyServiceDefinitionValidator\Tests\Functional\Fixtures;
 
-class Price
+class Transport
 {
-    public function __construct($currency = 'euro')
+    public function __construct($username = 'root')
     {
-
     }
 
-    public function setCurrency($currency)
+    public function setUsername($username)
     {
-
     }
 }