diff --git a/.github/patch-types.php b/.github/patch-types.php index 0d5576ea7f..2cc11432ed 100644 --- a/.github/patch-types.php +++ b/.github/patch-types.php @@ -21,6 +21,7 @@ foreach ($loader->getClassMap() as $class => $file) { case false !== strpos($file, '/src/Symfony/Component/Config/Tests/Fixtures/ParseError.php'): case false !== strpos($file, '/src/Symfony/Component/Debug/Tests/Fixtures/'): case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Compiler/OptionalServiceClass.php'): + case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/UnionConstructor.php'): case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php'): case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/uniontype_classes.php'): case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/ParentNotExists.php'): diff --git a/src/Symfony/Component/Config/Definition/Dumper/YamlReferenceDumper.php b/src/Symfony/Component/Config/Definition/Dumper/YamlReferenceDumper.php index 70f740e973..1c0253ba42 100644 --- a/src/Symfony/Component/Config/Definition/Dumper/YamlReferenceDumper.php +++ b/src/Symfony/Component/Config/Definition/Dumper/YamlReferenceDumper.php @@ -17,6 +17,7 @@ use Symfony\Component\Config\Definition\EnumNode; use Symfony\Component\Config\Definition\NodeInterface; use Symfony\Component\Config\Definition\PrototypedArrayNode; use Symfony\Component\Config\Definition\ScalarNode; +use Symfony\Component\Config\Definition\VariableNode; use Symfony\Component\Yaml\Inline; /** @@ -95,6 +96,9 @@ class YamlReferenceDumper } elseif ($node instanceof EnumNode) { $comments[] = 'One of '.implode('; ', array_map('json_encode', $node->getValues())); $default = $node->hasDefaultValue() ? Inline::dump($node->getDefaultValue()) : '~'; + } elseif (VariableNode::class === \get_class($node) && \is_array($example)) { + // If there is an array example, we are sure we dont need to print a default value + $default = ''; } else { $default = '~'; diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php index f4974cce10..752633ce17 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php @@ -153,26 +153,27 @@ final class CheckTypeDeclarationsPass extends AbstractRecursivePass /** * @throws InvalidParameterTypeException When a parameter is not compatible with the declared type */ - private function checkType(Definition $checkedDefinition, $value, \ReflectionParameter $parameter, ?string $envPlaceholderUniquePrefix, string $type = null): void + private function checkType(Definition $checkedDefinition, $value, \ReflectionParameter $parameter, ?string $envPlaceholderUniquePrefix, \ReflectionType $reflectionType = null): void { - if (null === $type) { - $type = $parameter->getType(); + $reflectionType = $reflectionType ?? $parameter->getType(); - if ($type instanceof \ReflectionUnionType) { - foreach ($type->getTypes() as $type) { - try { - $this->checkType($checkedDefinition, $value, $parameter, $envPlaceholderUniquePrefix, $type); + if ($reflectionType instanceof \ReflectionUnionType) { + foreach ($reflectionType->getTypes() as $t) { + try { + $this->checkType($checkedDefinition, $value, $parameter, $envPlaceholderUniquePrefix, $t); - return; - } catch (InvalidParameterTypeException $e) { - } + return; + } catch (InvalidParameterTypeException $e) { } - - throw new InvalidParameterTypeException($this->currentId, $e->getCode(), $parameter); } - $type = $type->getName(); + throw new InvalidParameterTypeException($this->currentId, $e->getCode(), $parameter); } + if (!$reflectionType instanceof \ReflectionNamedType) { + return; + } + + $type = $reflectionType->getName(); if ($value instanceof Reference) { if (!$this->container->has($value = (string) $value)) { @@ -285,8 +286,8 @@ final class CheckTypeDeclarationsPass extends AbstractRecursivePass $checkFunction = sprintf('is_%s', $type); - if (!$parameter->getType()->isBuiltin() || !$checkFunction($value)) { - throw new InvalidParameterTypeException($this->currentId, \is_object($value) ? $class : get_debug_type($value), $parameter); + if (!$reflectionType->isBuiltin() || !$checkFunction($value)) { + 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 0aef6e89e3..f3bf304196 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php @@ -26,6 +26,7 @@ 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\UnionConstructor; use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Waldo; use Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Wobble; use Symfony\Component\ExpressionLanguage\Expression; @@ -803,4 +804,72 @@ class CheckTypeDeclarationsPassTest extends TestCase putenv('ARRAY='); } + + /** + * @requires PHP 8 + */ + public function testUnionTypePassesWithReference() + { + $container = new ContainerBuilder(); + + $container->register('foo', Foo::class); + $container->register('union', UnionConstructor::class) + ->setArguments([new Reference('foo')]); + + (new CheckTypeDeclarationsPass(true))->process($container); + + $this->addToAssertionCount(1); + } + + /** + * @requires PHP 8 + */ + public function testUnionTypePassesWithBuiltin() + { + $container = new ContainerBuilder(); + + $container->register('union', UnionConstructor::class) + ->setArguments([42]); + + (new CheckTypeDeclarationsPass(true))->process($container); + + $this->addToAssertionCount(1); + } + + /** + * @requires PHP 8 + */ + public function testUnionTypeFailsWithReference() + { + $container = new ContainerBuilder(); + + $container->register('waldo', Waldo::class); + $container->register('union', UnionConstructor::class) + ->setArguments([new Reference('waldo')]); + + $this->expectException(\Symfony\Component\DependencyInjection\Exception\InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid definition for service "union": argument 1 of "Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CheckTypeDeclarationsPass\\UnionConstructor::__construct" accepts "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Foo|int", "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Waldo" passed.'); + + (new CheckTypeDeclarationsPass(true))->process($container); + + $this->addToAssertionCount(1); + } + + /** + * @requires PHP 8 + */ + public function testUnionTypeFailsWithBuiltin() + { + $container = new ContainerBuilder(); + + $container->register('union', UnionConstructor::class) + ->setArguments([[1, 2, 3]]); + + $this->expectException(\Symfony\Component\DependencyInjection\Exception\InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid definition for service "union": argument 1 of "Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CheckTypeDeclarationsPass\\UnionConstructor::__construct" accepts "Symfony\Component\DependencyInjection\Tests\Fixtures\CheckTypeDeclarationsPass\Foo|int", "array" passed.'); + + (new CheckTypeDeclarationsPass(true))->process($container); + + $this->addToAssertionCount(1); + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/UnionConstructor.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/UnionConstructor.php new file mode 100644 index 0000000000..5ba5972863 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/UnionConstructor.php @@ -0,0 +1,10 @@ +createMock(\Doctrine\DBAL\Connection::class); + $driverConnection = $this->createMock(DBALConnection::class); $platform = $this->createMock(AbstractPlatform::class); $platform->method('getWriteLockSQL')->willReturn('FOR UPDATE'); $configuration = $this->createMock(\Doctrine\DBAL\Configuration::class); @@ -349,6 +354,55 @@ class ConnectionTest extends TestCase $this->assertEquals(['type' => DummyMessage::class], $doctrineEnvelopes[1]['headers']); } + /** + * @dataProvider providePlatformSql + */ + public function testGeneratedSql(AbstractPlatform $platform, string $expectedSql) + { + $driverConnection = $this->createMock(DBALConnection::class); + $driverConnection->method('getDatabasePlatform')->willReturn($platform); + $driverConnection->method('createQueryBuilder')->willReturnCallback(function () use ($driverConnection) { + return new QueryBuilder($driverConnection); + }); + + if (interface_exists(DriverResult::class)) { + $result = $this->createMock(DriverResult::class); + $result->method('fetchAssociative')->willReturn(false); + + if (class_exists(Result::class)) { + $result = new Result($result, $driverConnection); + } + } else { + $result = $this->createMock(ResultStatement::class); + $result->method('fetch')->willReturn(false); + } + + $driverConnection->expects($this->once())->method('beginTransaction'); + $driverConnection + ->expects($this->once()) + ->method('executeQuery') + ->with($expectedSql) + ->willReturn($result) + ; + $driverConnection->expects($this->once())->method('commit'); + + $connection = new Connection([], $driverConnection); + $connection->get(); + } + + public function providePlatformSql(): iterable + { + yield 'MySQL' => [ + new MySQL57Platform(), + 'SELECT m.* FROM messenger_messages m WHERE (m.delivered_at is null OR m.delivered_at < ?) AND (m.available_at <= ?) AND (m.queue_name = ?) ORDER BY available_at ASC LIMIT 1 FOR UPDATE', + ]; + + yield 'SQL Server' => [ + new SQLServer2012Platform(), + 'SELECT m.* FROM messenger_messages m WITH (UPDLOCK, ROWLOCK) WHERE (m.delivered_at is null OR m.delivered_at < ?) AND (m.available_at <= ?) AND (m.queue_name = ?) ORDER BY available_at ASC OFFSET 0 ROWS FETCH NEXT 1 ROWS ONLY ', + ]; + } + public function testConfigureSchema() { $driverConnection = $this->getDBALConnectionMock();