From 0d4c0a6492c7ebcc58646e39e41281309c3c70e4 Mon Sep 17 00:00:00 2001 From: Patrick Luca Fazzi Date: Wed, 22 Jan 2020 09:27:23 +0100 Subject: [PATCH] [DI] CheckTypeDeclarationsPass now checks if value is type of parameter type --- .../Compiler/CheckTypeDeclarationsPass.php | 83 ++++++++++++------- .../CheckTypeDeclarationsPassTest.php | 17 ++++ .../CheckTypeDeclarationsPass/Waldo.php | 7 ++ .../WaldoInterface.php | 7 ++ .../CheckTypeDeclarationsPass/Wobble.php | 13 +++ 5 files changed, 98 insertions(+), 29 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/Waldo.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/WaldoInterface.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/Wobble.php diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php index 7c414258b7..4e145f294f 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php @@ -12,7 +12,9 @@ namespace Symfony\Component\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\Argument\IteratorArgument; +use Symfony\Component\DependencyInjection\Argument\RewindableGenerator; use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument; +use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Exception\EnvNotFoundException; @@ -40,7 +42,23 @@ use Symfony\Component\ExpressionLanguage\Expression; */ final class CheckTypeDeclarationsPass extends AbstractRecursivePass { - private const SCALAR_TYPES = ['int', 'float', 'bool', 'string']; + private const SCALAR_TYPES = [ + 'int' => true, + 'float' => true, + 'bool' => true, + 'string' => true, + ]; + + private const BUILTIN_TYPES = [ + 'array' => true, + 'bool' => true, + 'callable' => true, + 'float' => true, + 'int' => true, + 'iterable' => true, + 'object' => true, + 'string' => true, + ]; private $autoload; private $skippedIds; @@ -160,33 +178,17 @@ final class CheckTypeDeclarationsPass extends AbstractRecursivePass $type = $checkedDefinition->getClass(); } + $class = null; + if ($value instanceof Definition) { $class = $value->getClass(); - if (!$class || (!$this->autoload && !class_exists($class, false) && !interface_exists($class, false))) { + if (isset(self::BUILTIN_TYPES[strtolower($class)])) { + $class = strtolower($class); + } elseif (!$class || (!$this->autoload && !class_exists($class, false) && !interface_exists($class, false))) { return; } - - if ('callable' === $type && (\Closure::class === $class || method_exists($class, '__invoke'))) { - return; - } - - if ('iterable' === $type && is_subclass_of($class, 'Traversable')) { - return; - } - - if ('object' === $type) { - return; - } - - if (is_a($class, $type, true)) { - return; - } - - throw new InvalidParameterTypeException($this->currentId, $class, $parameter); - } - - if ($value instanceof Parameter) { + } elseif ($value instanceof Parameter) { $value = $this->container->getParameter($value); } elseif ($value instanceof Expression) { $value = $this->getExpressionLanguage()->evaluate($value, ['container' => $this->container]); @@ -212,30 +214,53 @@ final class CheckTypeDeclarationsPass extends AbstractRecursivePass return; } - if (\in_array($type, self::SCALAR_TYPES, true) && is_scalar($value)) { + if (null === $class) { + if ($value instanceof IteratorArgument) { + $class = RewindableGenerator::class; + } elseif ($value instanceof ServiceClosureArgument) { + $class = \Closure::class; + } elseif ($value instanceof ServiceLocatorArgument) { + $class = ServiceLocator::class; + } elseif (\is_object($value)) { + $class = \get_class($value); + } else { + $class = \gettype($value); + $class = ['integer' => 'int', 'double' => 'float', 'boolean' => 'bool'][$class] ?? $class; + } + } + + if (isset(self::SCALAR_TYPES[$type]) && isset(self::SCALAR_TYPES[$class])) { return; } - if ('callable' === $type && \is_array($value) && isset($value[0]) && ($value[0] instanceof Reference || $value[0] instanceof Definition)) { + if ('string' === $type && \is_callable([$class, '__toString'])) { return; } - if (\in_array($type, ['callable', 'Closure'], true) && $value instanceof ServiceClosureArgument) { + if ('callable' === $type && (\Closure::class === $class || \is_callable([$class, '__invoke']))) { return; } - if ('iterable' === $type && (\is_array($value) || $value instanceof \Traversable || $value instanceof IteratorArgument)) { + if ('callable' === $type && \is_array($value) && isset($value[0]) && ($value[0] instanceof Reference || $value[0] instanceof Definition || \is_string($value[0]))) { return; } - if ('Traversable' === $type && ($value instanceof \Traversable || $value instanceof IteratorArgument)) { + if ('iterable' === $type && (\is_array($value) || is_subclass_of($class, \Traversable::class))) { + return; + } + + if ('object' === $type && !isset(self::BUILTIN_TYPES[$class])) { + return; + } + + if (is_a($class, $type, true)) { return; } $checkFunction = sprintf('is_%s', $parameter->getType()->getName()); if (!$parameter->getType()->isBuiltin() || !$checkFunction($value)) { - throw new InvalidParameterTypeException($this->currentId, \is_object($value) ? \get_class($value) : \gettype($value), $parameter); + throw new InvalidParameterTypeException($this->currentId, \is_object($value) ? $class : \gettype($value), $parameter); } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php index 350f85296a..79cbacecfc 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php @@ -26,6 +26,8 @@ use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPa use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\BarOptionalArgumentNotNull; use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Foo; use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\FooObject; +use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Waldo; +use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Wobble; use Symfony\Component\ExpressionLanguage\Expression; /** @@ -602,6 +604,21 @@ class CheckTypeDeclarationsPassTest extends TestCase $this->addToAssertionCount(1); } + public function testProcessSuccessWhenExpressionReturnsObject() + { + $container = new ContainerBuilder(); + + $container->register('waldo', Waldo::class); + + $container + ->register('wobble', Wobble::class) + ->setArguments([new Expression("service('waldo')")]); + + (new CheckTypeDeclarationsPass(true))->process($container); + + $this->addToAssertionCount(1); + } + public function testProcessHandleMixedEnvPlaceholder() { $this->expectException(\Symfony\Component\DependencyInjection\Exception\InvalidArgumentException::class); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/Waldo.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/Waldo.php new file mode 100644 index 0000000000..321cc81858 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/Waldo.php @@ -0,0 +1,7 @@ +waldo = $waldo; + } +}