From 4b5920107b156167dc38f2b2927366a75701533f Mon Sep 17 00:00:00 2001 From: Julien Maulny Date: Fri, 29 Jun 2018 16:18:59 +0200 Subject: [PATCH 1/7] [#27744] Add compiler pass to check arguments type hint --- .../DependencyInjection/CHANGELOG.md | 1 + .../Compiler/CheckTypeHintsPass.php | 165 +++++ .../InvalidParameterTypeHintException.php | 28 + .../Tests/Compiler/CheckTypeHintsPassTest.php | 563 ++++++++++++++++++ .../Tests/Fixtures/CheckTypeHintsPass/Bar.php | 13 + .../CheckTypeHintsPass/BarMethodCall.php | 31 + .../BarOptionalArgument.php | 13 + .../BarOptionalArgumentNotNull.php | 13 + .../Tests/Fixtures/CheckTypeHintsPass/Foo.php | 16 + 9 files changed, 843 insertions(+) create mode 100644 src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php create mode 100644 src/Symfony/Component/DependencyInjection/Exception/InvalidParameterTypeHintException.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeHintsPassTest.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/Bar.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarMethodCall.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarOptionalArgument.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarOptionalArgumentNotNull.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/Foo.php diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index de2421a60a19b..95150ffc3ef07 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/src/Symfony/Component/DependencyInjection/CHANGELOG.md @@ -6,6 +6,7 @@ CHANGELOG * added `ServiceSubscriberTrait` * added `ServiceLocatorArgument` for creating optimized service-locators + * added `CheckTypeHintsPass` to check injected parameters type during compilation 4.1.0 ----- diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php new file mode 100644 index 0000000000000..5cac189068752 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php @@ -0,0 +1,165 @@ + + * + * 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\Reference; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; +use Symfony\Component\DependencyInjection\Exception\InvalidParameterTypeHintException; + +/** + * Checks whether injected parameters types are compatible with type hints. + * This pass should be added before removing (PassConfig::TYPE_BEFORE_REMOVING). + * + * @author Nicolas Grekas + * @author Julien Maulny + */ +class CheckTypeHintsPass extends AbstractRecursivePass +{ + /** + * If set to true, allows to autoload classes during compilation + * in order to check type hints on parameters that are not yet loaded. + * Defaults to false to prevent code loading during compilation. + * + * @param bool + */ + private $autoload; + + public function __construct(bool $autoload = false) + { + $this->autoload = $autoload; + } + + /** + * {@inheritdoc} + */ + protected function processValue($value, $isRoot = false) + { + if (!$value instanceof Definition) { + return parent::processValue($value, $isRoot); + } + + if (!$this->autoload && !class_exists($className = $this->getClassName($value), false) && !interface_exists($className, false)) { + return parent::processValue($value, $isRoot); + } + + if (null !== $constructor = $this->getConstructor($value, false)) { + $this->checkArgumentsTypeHints($constructor, $value->getArguments()); + } + + foreach ($value->getMethodCalls() as $methodCall) { + $reflectionMethod = $this->getReflectionMethod($value, $methodCall[0]); + + $this->checkArgumentsTypeHints($reflectionMethod, $methodCall[1]); + } + + return parent::processValue($value, $isRoot); + } + + /** + * Check type hints for every parameter of a method/constructor. + * + * @throws InvalidArgumentException on type hint incompatibility + */ + private function checkArgumentsTypeHints(\ReflectionFunctionAbstract $reflectionFunction, array $configurationArguments): void + { + $numberOfRequiredParameters = $reflectionFunction->getNumberOfRequiredParameters(); + + if (count($configurationArguments) < $numberOfRequiredParameters) { + throw new InvalidArgumentException(sprintf( + 'Invalid definition for service "%s": "%s::%s()" requires %d arguments, %d passed.', $this->currentId, $reflectionFunction->class, $reflectionFunction->name, $numberOfRequiredParameters, count($configurationArguments))); + } + + $reflectionParameters = $reflectionFunction->getParameters(); + $checksCount = min($reflectionFunction->getNumberOfParameters(), count($configurationArguments)); + + for ($i = 0; $i < $checksCount; ++$i) { + if (!$reflectionParameters[$i]->hasType() || $reflectionParameters[$i]->isVariadic()) { + continue; + } + + $this->checkTypeHint($configurationArguments[$i], $reflectionParameters[$i]); + } + + if ($reflectionFunction->isVariadic() && ($lastParameter = end($reflectionParameters))->hasType()) { + $variadicParameters = array_slice($configurationArguments, $lastParameter->getPosition()); + + foreach ($variadicParameters as $variadicParameter) { + $this->checkTypeHint($variadicParameter, $lastParameter); + } + } + } + + /** + * Check type hints compatibility between + * a definition argument and a reflection parameter. + * + * @throws InvalidArgumentException on type hint incompatibility + */ + private function checkTypeHint($configurationArgument, \ReflectionParameter $parameter): void + { + $referencedDefinition = $configurationArgument; + + if ($referencedDefinition instanceof Reference) { + $referencedDefinition = $this->container->findDefinition((string) $referencedDefinition); + } + + if ($referencedDefinition instanceof Definition) { + $class = $this->getClassName($referencedDefinition); + + if (!$this->autoload && !class_exists($class, false)) { + return; + } + + if (!is_a($class, $parameter->getType()->getName(), true)) { + throw new InvalidParameterTypeHintException($this->currentId, null === $class ? 'null' : $class, $parameter); + } + } else { + if (null === $configurationArgument && $parameter->allowsNull()) { + return; + } + + if ($parameter->getType()->isBuiltin() && is_scalar($configurationArgument)) { + return; + } + + $checkFunction = 'is_'.$parameter->getType()->getName(); + + if (!$parameter->getType()->isBuiltin() || !$checkFunction($configurationArgument)) { + throw new InvalidParameterTypeHintException($this->currentId, gettype($configurationArgument), $parameter); + } + } + } + + /** + * Get class name from value that can have a factory. + * + * @return string|null + */ + private function getClassName($value) + { + if (is_array($factory = $value->getFactory())) { + list($class, $method) = $factory; + if ($class instanceof Reference) { + $class = $this->container->findDefinition((string) $class)->getClass(); + } elseif (null === $class) { + $class = $value->getClass(); + } elseif ($class instanceof Definition) { + $class = $this->getClassName($class); + } + } else { + $class = $value->getClass(); + } + + return $class; + } +} diff --git a/src/Symfony/Component/DependencyInjection/Exception/InvalidParameterTypeHintException.php b/src/Symfony/Component/DependencyInjection/Exception/InvalidParameterTypeHintException.php new file mode 100644 index 0000000000000..a04d0719729bc --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Exception/InvalidParameterTypeHintException.php @@ -0,0 +1,28 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Exception; + +/** + * Thrown when trying to inject a parameter into a constructor/method + * with a type that does not match type hint. + * + * @author Nicolas Grekas + * @author Julien Maulny + */ +class InvalidParameterTypeHintException extends InvalidArgumentException +{ + public function __construct(string $serviceId, string $typeHint, \ReflectionParameter $parameter) + { + parent::__construct(sprintf( + 'Invalid definition for service "%s": argument %d of "%s::%s" requires a "%s", "%s" passed.', $serviceId, $parameter->getPosition(), $parameter->getDeclaringClass()->getName(), $parameter->getDeclaringFunction()->getName(), $parameter->getType()->getName(), $typeHint)); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeHintsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeHintsPassTest.php new file mode 100644 index 0000000000000..9b48ee333393a --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeHintsPassTest.php @@ -0,0 +1,563 @@ + + * + * 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 PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\DependencyInjection\Compiler\CheckTypeHintsPass; +use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Bar; +use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarOptionalArgument; +use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarOptionalArgumentNotNull; +use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarMethodCall; +use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo; + +/** + * @author Nicolas Grekas + * @author Julien Maulny + */ +class CheckTypeHintsPassTest extends TestCase +{ + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 0 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Bar::__construct" requires a "stdClass", "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo" passed + */ + public function testProcessThrowsExceptionOnInvalidTypeHintsConstructorArguments() + { + $container = new ContainerBuilder(); + + $container->register('foo', Foo::class); + $container->register('bar', Bar::class) + ->addArgument(new Reference('foo')); + + (new CheckTypeHintsPass(true))->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 0 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarMethodCall::setFoo" requires a "stdClass", "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo" passed + */ + public function testProcessThrowsExceptionOnInvalidTypeHintsMethodCallArguments() + { + $container = new ContainerBuilder(); + + $container->register('foo', Foo::class); + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setFoo', array(new Reference('foo'))); + + (new CheckTypeHintsPass(true))->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 0 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Bar::__construct" requires a "stdClass", "NULL" passed + */ + public function testProcessFailsWhenPassingNullToRequiredArgument() + { + $container = new ContainerBuilder(); + + $container->register('bar', Bar::class) + ->addArgument(null); + + (new CheckTypeHintsPass(true))->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Bar::__construct()" requires 1 arguments, 0 passed + */ + public function testProcessThrowsExceptionWhenMissingArgumentsInConstructor() + { + $container = new ContainerBuilder(); + + $container->register('bar', Bar::class); + + (new CheckTypeHintsPass(true))->process($container); + } + + public function testProcessSuccessWhenPassingTooManyArgumentInConstructor() + { + $container = new ContainerBuilder(); + + $container->register('foo', \stdClass::class); + $container->register('bar', Bar::class) + ->addArgument(new Reference('foo')) + ->addArgument(new Reference('foo')); + + (new CheckTypeHintsPass(true))->process($container); + + $this->addToAssertionCount(1); + } + + public function testProcessRegisterWithClassName() + { + $container = new ContainerBuilder(); + + $container->register(Foo::class, Foo::class); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(Foo::class, $container->get(Foo::class)); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarMethodCall::setFoo()" requires 1 arguments, 0 passed + */ + public function testProcessThrowsExceptionWhenMissingArgumentsInMethodCall() + { + $container = new ContainerBuilder(); + + $container->register('foo', \stdClass::class); + $container->register('bar', BarMethodCall::class) + ->addArgument(new Reference('foo')) + ->addMethodCall('setFoo', array()); + + (new CheckTypeHintsPass(true))->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 1 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarMethodCall::setFoosVariadic" requires a "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo", "stdClass" passed + */ + public function testProcessVariadicFails() + { + $container = new ContainerBuilder(); + + $container->register('stdClass', \stdClass::class); + $container->register('foo', Foo::class); + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setFoosVariadic', array( + new Reference('foo'), + new Reference('foo'), + new Reference('stdClass'), + )); + + (new CheckTypeHintsPass(true))->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 0 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarMethodCall::setFoosVariadic" requires a "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo", "stdClass" passed + */ + public function testProcessVariadicFailsOnPassingBadTypeOnAnotherArgument() + { + $container = new ContainerBuilder(); + + $container->register('stdClass', \stdClass::class); + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setFoosVariadic', array( + new Reference('stdClass'), + )); + + (new CheckTypeHintsPass(true))->process($container); + } + + public function testProcessVariadicSuccess() + { + $container = new ContainerBuilder(); + + $container->register('foo', Foo::class); + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setFoosVariadic', array( + new Reference('foo'), + new Reference('foo'), + new Reference('foo'), + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(Foo::class, $container->get('bar')->foo); + } + + public function testProcessSuccessWhenNotUsingOptionalArgument() + { + $container = new ContainerBuilder(); + + $container->register('foo', Foo::class); + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setFoosOptional', array( + new Reference('foo'), + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(Foo::class, $container->get('bar')->foo); + } + + public function testProcessSuccessWhenUsingOptionalArgumentWithGoodType() + { + $container = new ContainerBuilder(); + + $container->register('foo', Foo::class); + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setFoosOptional', array( + new Reference('foo'), + new Reference('foo'), + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(Foo::class, $container->get('bar')->foo); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 1 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarMethodCall::setFoosOptional" requires a "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo", "stdClass" passed + */ + public function testProcessFailsWhenUsingOptionalArgumentWithBadType() + { + $container = new ContainerBuilder(); + + $container->register('stdClass', \stdClass::class); + $container->register('foo', Foo::class); + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setFoosOptional', array( + new Reference('foo'), + new Reference('stdClass'), + )); + + (new CheckTypeHintsPass(true))->process($container); + } + + public function testProcessSuccessWhenPassingNullToOptional() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarOptionalArgument::class) + ->addArgument(null); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertNull($container->get('bar')->foo); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 0 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarOptionalArgumentNotNull::__construct" requires a "int", "NULL" passed + */ + public function testProcessSuccessWhenPassingNullToOptionalThatDoesNotAcceptNull() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarOptionalArgumentNotNull::class) + ->addArgument(null); + + (new CheckTypeHintsPass(true))->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 0 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarOptionalArgument::__construct" requires a "stdClass", "string" passed + */ + public function testProcessFailsWhenPassingBadTypeToOptional() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarOptionalArgument::class) + ->addArgument('string instead of stdClass'); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertNull($container->get('bar')->foo); + } + + public function testProcessSuccessScalarType() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setScalars', array( + 1, + 'string', + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(BarMethodCall::class, $container->get('bar')); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 0 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Bar::__construct" requires a "stdClass", "integer" passed + */ + public function testProcessFailsOnPassingScalarTypeToConstructorTypeHintedWithClass() + { + $container = new ContainerBuilder(); + + $container->register('bar', Bar::class) + ->addArgument(1); + + (new CheckTypeHintsPass(true))->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 0 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarMethodCall::setFoo" requires a "stdClass", "string" passed + */ + public function testProcessFailsOnPassingScalarTypeToMethodTypeHintedWithClass() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setFoo', array( + 'builtin type instead of class', + )); + + (new CheckTypeHintsPass(true))->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 0 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarMethodCall::setScalars" requires a "int", "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo" passed + */ + public function testProcessFailsOnPassingClassToScalarTypeHintedParameter() + { + $container = new ContainerBuilder(); + + $container->register('foo', Foo::class); + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setScalars', array( + new Reference('foo'), + new Reference('foo'), + )); + + (new CheckTypeHintsPass(true))->process($container); + } + + /** + * Strict mode not yet handled. + */ + public function testProcessSuccessOnPassingBadScalarType() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setScalars', array( + 1, + true, + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(BarMethodCall::class, $container->get('bar')); + } + + /** + * Strict mode not yet handled. + */ + public function testProcessSuccessPassingBadScalarTypeOptionalArgument() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setScalars', array( + 1, + 'string', + 'string instead of optional boolean', + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(BarMethodCall::class, $container->get('bar')); + } + + public function testProcessSuccessWhenPassingArray() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setArray', array( + array(), + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(BarMethodCall::class, $container->get('bar')); + } + + public function testProcessSuccessWhenPassingIntegerToArrayTypeHintedParameter() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setArray', array( + 1, + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->addToAssertionCount(1); + } + + public function testProcessFactory() + { + $container = new ContainerBuilder(); + + $container->register('foo', Foo::class); + $container->register('bar', Bar::class) + ->setFactory(array( + new Reference('foo'), + 'createBar', + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(Bar::class, $container->get('bar')); + } + + public function testProcessFactoryWhithClassName() + { + $container = new ContainerBuilder(); + + $container->register(Foo::class, Foo::class); + $container->register(Bar::class, Bar::class) + ->setFactory(array( + new Reference(Foo::class), + 'createBar', + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(Bar::class, $container->get(Bar::class)); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 0 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo::createBarArguments" requires a "stdClass", "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo" passed + */ + public function testProcessFactoryFailsOnInvalidParameterType() + { + $container = new ContainerBuilder(); + + $container->register('foo', Foo::class); + $container->register('bar', Bar::class) + ->addArgument(new Reference('foo')) + ->setFactory(array( + new Reference('foo'), + 'createBarArguments', + )); + + (new CheckTypeHintsPass(true))->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException + * @expectedExceptionMessage Invalid definition for service "bar": argument 1 of "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo::createBarArguments" requires a "stdClass", "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Foo" passed + */ + public function testProcessFactoryFailsOnInvalidParameterTypeOptional() + { + $container = new ContainerBuilder(); + + $container->register('stdClass', \stdClass::class); + $container->register('foo', Foo::class); + $container->register('bar', Bar::class) + ->addArgument(new Reference('stdClass')) + ->addArgument(new Reference('foo')) + ->setFactory(array( + new Reference('foo'), + 'createBarArguments', + )); + + (new CheckTypeHintsPass(true))->process($container); + } + + public function testProcessFactorySuccessOnValidTypes() + { + $container = new ContainerBuilder(); + + $container->register('stdClass', \stdClass::class); + $container->register('foo', Foo::class); + $container->register('bar', Bar::class) + ->addArgument(new Reference('stdClass')) + ->addArgument(new Reference('stdClass')) + ->setFactory(array( + new Reference('foo'), + 'createBarArguments', + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->addToAssertionCount(1); + } + + public function testProcessFactoryCallbackSuccessOnValidType() + { + $container = new ContainerBuilder(); + + $container->register('bar', \DateTime::class) + ->setFactory('date_create'); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(\DateTime::class, $container->get('bar')); + } + + public function testProcessDoesNotLoadCodeByDefault() + { + $container = new ContainerBuilder(); + + $container->register('foo', FooNotExisting::class); + $container->register('bar', BarNotExisting::class) + ->addArgument(new Reference('foo')) + ->addMethodCall('setFoo', array( + new Reference('foo'), + 'string', + 1, + )); + + (new CheckTypeHintsPass())->process($container); + + $this->addToAssertionCount(1); + } + + public function testProcessFactoryDoesNotLoadCodeByDefault() + { + $container = new ContainerBuilder(); + + $container->register('foo', FooNotExisting::class); + $container->register('bar', BarNotExisting::class) + ->setFactory(array( + new Reference('foo'), + 'notExistingMethod', + )); + + (new CheckTypeHintsPass())->process($container); + + $this->addToAssertionCount(1); + } + + public function testProcessPassingBuiltinTypeDoesNotLoadCodeByDefault() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarNotExisting::class) + ->addArgument(1); + + (new CheckTypeHintsPass())->process($container); + + $this->addToAssertionCount(1); + } + + public function testProcessDoesNotThrowsExceptionOnValidTypeHints() + { + $container = new ContainerBuilder(); + + $container->register('foo', \stdClass::class); + $container->register('bar', Bar::class) + ->addArgument(new Reference('foo')); + + (new CheckTypeHintsPass(true))->process($container); + + $this->assertInstanceOf(\stdClass::class, $container->get('bar')->foo); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/Bar.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/Bar.php new file mode 100644 index 0000000000000..85a1289815f85 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/Bar.php @@ -0,0 +1,13 @@ +foo = $foo; + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarMethodCall.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarMethodCall.php new file mode 100644 index 0000000000000..8a405713afbc1 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarMethodCall.php @@ -0,0 +1,31 @@ +foo = $foo; + } + + public function setFoosVariadic(Foo $foo, Foo ...$foos) + { + $this->foo = $foo; + } + + public function setFoosOptional(Foo $foo, Foo $fooOptional = null) + { + $this->foo = $foo; + } + + public function setScalars(int $int, string $string, bool $bool = false) + { + } + + public function setArray(array $array) + { + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarOptionalArgument.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarOptionalArgument.php new file mode 100644 index 0000000000000..3b6daa77f84dd --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarOptionalArgument.php @@ -0,0 +1,13 @@ +foo = $foo; + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarOptionalArgumentNotNull.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarOptionalArgumentNotNull.php new file mode 100644 index 0000000000000..5e54114aef90e --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarOptionalArgumentNotNull.php @@ -0,0 +1,13 @@ +foo = $foo; + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/Foo.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/Foo.php new file mode 100644 index 0000000000000..9132d6a246327 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/Foo.php @@ -0,0 +1,16 @@ + Date: Mon, 9 Jul 2018 21:15:20 +0200 Subject: [PATCH 2/7] update phpdoc --- .../DependencyInjection/Compiler/CheckTypeHintsPass.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php index 5cac189068752..9d1c544d65ee6 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php @@ -18,7 +18,12 @@ /** * Checks whether injected parameters types are compatible with type hints. - * This pass should be added before removing (PassConfig::TYPE_BEFORE_REMOVING). + * This pass should be run after all optimization passes. + * So it can be added either: + * * before removing (PassConfig::TYPE_BEFORE_REMOVING) so that it will check + * all services, even if they are not currently used, + * * after removing (PassConfig::TYPE_AFTER_REMOVING) so that it will check + * only services you are using. * * @author Nicolas Grekas * @author Julien Maulny From cc8c4dec4fb84ce4b51556192b49a0158ed3c73b Mon Sep 17 00:00:00 2001 From: Julien Maulny Date: Mon, 9 Jul 2018 22:18:25 +0200 Subject: [PATCH 3/7] add missing case with 'iterable' type hint --- .../Compiler/CheckTypeHintsPass.php | 5 +++++ .../Tests/Compiler/CheckTypeHintsPassTest.php | 15 +++++++++++++++ .../Fixtures/CheckTypeHintsPass/BarMethodCall.php | 4 ++++ 3 files changed, 24 insertions(+) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php index 9d1c544d65ee6..5fe06614ae774 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php @@ -15,6 +15,7 @@ use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Exception\InvalidParameterTypeHintException; +use Symfony\Component\DependencyInjection\Argument\IteratorArgument; /** * Checks whether injected parameters types are compatible with type hints. @@ -137,6 +138,10 @@ private function checkTypeHint($configurationArgument, \ReflectionParameter $par return; } + if ('iterable' === $parameter->getType()->getName() && $configurationArgument instanceof IteratorArgument) { + return; + } + $checkFunction = 'is_'.$parameter->getType()->getName(); if (!$parameter->getType()->isBuiltin() || !$checkFunction($configurationArgument)) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeHintsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeHintsPassTest.php index 9b48ee333393a..690c628cb64a9 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeHintsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeHintsPassTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\DependencyInjection\Argument\IteratorArgument; use Symfony\Component\DependencyInjection\Compiler\CheckTypeHintsPass; use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\Bar; use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeHintsPass\BarOptionalArgument; @@ -399,6 +400,20 @@ public function testProcessSuccessWhenPassingIntegerToArrayTypeHintedParameter() $this->addToAssertionCount(1); } + public function testProcessSuccessWhenPassingAnIteratorArgumentToIterable() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setIterable', array( + new IteratorArgument(array()), + )); + + (new CheckTypeHintsPass(true))->process($container); + + $this->addToAssertionCount(1); + } + public function testProcessFactory() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarMethodCall.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarMethodCall.php index 8a405713afbc1..076c219d3a4e0 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarMethodCall.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeHintsPass/BarMethodCall.php @@ -28,4 +28,8 @@ public function setScalars(int $int, string $string, bool $bool = false) public function setArray(array $array) { } + + public function setIterable(iterable $iterable) + { + } } From ddba54924bce849866e3bda383884e97b6bece7e Mon Sep 17 00:00:00 2001 From: Julien Maulny Date: Thu, 12 Jul 2018 00:02:35 +0200 Subject: [PATCH 4/7] lint:container command --- .../Command/ContainerLintCommand.php | 81 +++++++++++++++++++ .../Resources/config/console.xml | 4 + 2 files changed, 85 insertions(+) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php new file mode 100644 index 0000000000000..ec10cc0a137ca --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php @@ -0,0 +1,81 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\FrameworkBundle\Command; + +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Config\ConfigCache; +use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Compiler\CheckTypeHintsPass; +use Symfony\Component\DependencyInjection\Compiler\PassConfig; +use Symfony\Component\Config\FileLocator; + +class ContainerLintCommand extends Command +{ + /** + * @var ContainerBuilder + */ + private $containerBuilder; + + /** + * {@inheritdoc} + */ + protected function configure() + { + $this + ->setDescription('Lints container for services arguments type hints') + ; + } + + /** + * {@inheritdoc} + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $container = $this->getContainerBuilder(); + + $container->setParameter('container.build_id', 'lint_container'); + + $container->addCompilerPass(new CheckTypeHintsPass(), PassConfig::TYPE_AFTER_REMOVING); + + $container->compile(); + } + + /** + * Loads the ContainerBuilder from the cache. + * + * @return ContainerBuilder + * + * @throws \LogicException + */ + protected function getContainerBuilder() + { + if ($this->containerBuilder) { + return $this->containerBuilder; + } + + $kernel = $this->getApplication()->getKernel(); + + if (!$kernel->isDebug() || !(new ConfigCache($kernel->getContainer()->getParameter('debug.container.dump'), true))->isFresh()) { + $buildContainer = \Closure::bind(function () { return $this->buildContainer(); }, $kernel, get_class($kernel)); + $container = $buildContainer(); + $container->getCompilerPassConfig()->setRemovingPasses(array()); + $container->compile(); + } else { + (new XmlFileLoader($container = new ContainerBuilder(), new FileLocator()))->load($kernel->getContainer()->getParameter('debug.container.dump')); + } + + return $this->containerBuilder = $container; + } +} diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml index 16732e2af480d..b7174ca906959 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml @@ -60,6 +60,10 @@ + + + + From 9740eee73ff054196cade72c1444966773ce99dc Mon Sep 17 00:00:00 2001 From: Julien Maulny Date: Thu, 12 Jul 2018 13:45:02 +0200 Subject: [PATCH 5/7] handle case when passing array to Traversable, ignore definitions containing another Definition --- .../DependencyInjection/Compiler/CheckTypeHintsPass.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php index 5fe06614ae774..2ff5cae14a41a 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php @@ -13,6 +13,7 @@ use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Exception\InvalidParameterTypeHintException; use Symfony\Component\DependencyInjection\Argument\IteratorArgument; @@ -58,6 +59,10 @@ protected function processValue($value, $isRoot = false) return parent::processValue($value, $isRoot); } + if ($value->getClass() === ServiceLocator::class) { + return parent::processValue($value, $isRoot); + } + if (null !== $constructor = $this->getConstructor($value, false)) { $this->checkArgumentsTypeHints($constructor, $value->getArguments()); } @@ -142,6 +147,10 @@ private function checkTypeHint($configurationArgument, \ReflectionParameter $par return; } + if ('Traversable' === $parameter->getType()->getName() && $configurationArgument instanceof IteratorArgument) { + return; + } + $checkFunction = 'is_'.$parameter->getType()->getName(); if (!$parameter->getType()->isBuiltin() || !$checkFunction($configurationArgument)) { From 361e8a6c18e0f4b10959e15f090bab6c77c06077 Mon Sep 17 00:00:00 2001 From: Julien Maulny Date: Thu, 12 Jul 2018 14:14:58 +0200 Subject: [PATCH 6/7] command lint:container allows to check only used services --- .../FrameworkBundle/Command/ContainerLintCommand.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php index ec10cc0a137ca..72d77f9d7ec44 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php +++ b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php @@ -13,6 +13,8 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Input\InputDefinition; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Config\ConfigCache; use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; @@ -35,6 +37,8 @@ protected function configure() { $this ->setDescription('Lints container for services arguments type hints') + ->setHelp('This command will parse all your defined services and check that you are injecting service without type error based on type hints.') + ->addOption('only-used-services', 'o', InputOption::VALUE_NONE, 'Check only services that are used in your application') ; } @@ -47,7 +51,10 @@ protected function execute(InputInterface $input, OutputInterface $output) $container->setParameter('container.build_id', 'lint_container'); - $container->addCompilerPass(new CheckTypeHintsPass(), PassConfig::TYPE_AFTER_REMOVING); + $container->addCompilerPass( + new CheckTypeHintsPass(), + $input->getOption('only-used-services') ? PassConfig::TYPE_AFTER_REMOVING : PassConfig::TYPE_BEFORE_OPTIMIZATION + ); $container->compile(); } From b305a692e421e5aa6e8c8b6cbfc55bbf4c5cc8d0 Mon Sep 17 00:00:00 2001 From: Julien Maulny Date: Thu, 12 Jul 2018 14:17:01 +0200 Subject: [PATCH 7/7] lint --- .../Bundle/FrameworkBundle/Command/ContainerLintCommand.php | 1 - .../DependencyInjection/Compiler/CheckTypeHintsPass.php | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php index 72d77f9d7ec44..c7c6cb45071c8 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php +++ b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php @@ -14,7 +14,6 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; -use Symfony\Component\Console\Input\InputDefinition; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Config\ConfigCache; use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php index 2ff5cae14a41a..41085b8e68848 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeHintsPass.php @@ -59,7 +59,7 @@ protected function processValue($value, $isRoot = false) return parent::processValue($value, $isRoot); } - if ($value->getClass() === ServiceLocator::class) { + if (ServiceLocator::class === $value->getClass()) { return parent::processValue($value, $isRoot); }