Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[DoctrineBridge] Extend type guessing on enum fields #46676

Merged
merged 1 commit into from Jun 19, 2022
Merged

[DoctrineBridge] Extend type guessing on enum fields #46676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 18 additions & 6 deletions src/Symfony/Bridge/Doctrine/PropertyInfo/DoctrineExtractor.php
View file
Open in desktop
Expand Up @@ -152,17 +152,18 @@ public function getTypes($class, $property, array $context = [])
}

if ($metadata->hasField($property)) {
$nullable = $metadata instanceof ClassMetadataInfo && $metadata->isNullable($property);
if (null !== $enumClass = $metadata->getFieldMapping($property)['enumType'] ?? null) {
return [new Type(Type::BUILTIN_TYPE_OBJECT, $nullable, $enumClass)];
}

$typeOfField = $metadata->getTypeOfField($property);

if (!$builtinType = $this->getPhpType($typeOfField)) {
return null;
}

$nullable = $metadata instanceof ClassMetadataInfo && $metadata->isNullable($property);
$enumType = null;
if (null !== $enumClass = $metadata->getFieldMapping($property)['enumType'] ?? null) {
$enumType = new Type(Type::BUILTIN_TYPE_OBJECT, $nullable, $enumClass);
}

switch ($builtinType) {
case Type::BUILTIN_TYPE_OBJECT:
switch ($typeOfField) {
Expand Down Expand Up @@ -192,11 +193,22 @@ public function getTypes($class, $property, array $context = [])
case self::$useDeprecatedConstants ? DBALType::TARRAY : Types::ARRAY:
// no break
case 'json_array':
// return null if $enumType is set, because we can't determine if collectionKeyType is string or int
if ($enumType) {
return null;
}

return [new Type(Type::BUILTIN_TYPE_ARRAY, $nullable, null, true)];

case self::$useDeprecatedConstants ? DBALType::SIMPLE_ARRAY : Types::SIMPLE_ARRAY:
return [new Type(Type::BUILTIN_TYPE_ARRAY, $nullable, null, true, new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_STRING))];
return [new Type(Type::BUILTIN_TYPE_ARRAY, $nullable, null, true, new Type(Type::BUILTIN_TYPE_INT), $enumType ?? new Type(Type::BUILTIN_TYPE_STRING))];
}
case Type::BUILTIN_TYPE_INT:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have merged to fast here.
@Sajito is the missing break before this line on purpose? Can you please send a PR that adds either the "break" or the "no break" comment, with a test case ideally?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have sent a new PR here: #46713.
I didn't know a "no break" comment would be required here, as I took these lines as a reference:

symfony/src/Symfony/Bridge/Doctrine/PropertyInfo/DoctrineExtractor.php

Lines 169 to 175 in 7fc7acb

switch ($typeOfField) {
case self::$useDeprecatedConstants ? DBALType::DATE : Types::DATE_MUTABLE:
case self::$useDeprecatedConstants ? DBALType::DATETIME : Types::DATETIME_MUTABLE:
case self::$useDeprecatedConstants ? DBALType::DATETIMETZ : Types::DATETIMETZ_MUTABLE:
case 'vardatetime':
case self::$useDeprecatedConstants ? DBALType::TIME : Types::TIME_MUTABLE:
return [new Type(Type::BUILTIN_TYPE_OBJECT, $nullable, 'DateTime')];

Hope it's fine with the new PR.

case Type::BUILTIN_TYPE_STRING:
if ($enumType !== null) {
return [$enumType];
}
// no break
}

return [new Type($builtinType, $nullable)];
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Bridge/Doctrine/Tests/PropertyInfo/DoctrineExtractorTest.php
View file
Open in desktop
Expand Up @@ -185,6 +185,9 @@ public function testExtractEnum()
}
$this->assertEquals([new Type(Type::BUILTIN_TYPE_OBJECT, false, EnumString::class)], $this->createExtractor()->getTypes(DoctrineEnum::class, 'enumString', []));
$this->assertEquals([new Type(Type::BUILTIN_TYPE_OBJECT, false, EnumInt::class)], $this->createExtractor()->getTypes(DoctrineEnum::class, 'enumInt', []));
$this->assertEquals(null, $this->createExtractor()->getTypes(DoctrineEnum::class, 'enumStringArray', []));
$this->assertEquals([new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_OBJECT, false, EnumInt::class))], $this->createExtractor()->getTypes(DoctrineEnum::class, 'enumIntArray', []));
$this->assertEquals(null, $this->createExtractor()->getTypes(DoctrineEnum::class, 'enumCustom', []));
}

public function typesProvider()
Expand Down
15 changes: 15 additions & 0 deletions src/Symfony/Bridge/Doctrine/Tests/PropertyInfo/Fixtures/DoctrineEnum.php
View file
Open in desktop
Expand Up @@ -35,4 +35,19 @@ class DoctrineEnum
* @Column(type="integer", enumType="Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\EnumInt")
*/
protected $enumInt;

/**
* @Column(type="array", enumType="Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\EnumString")
*/
protected $enumStringArray;

/**
* @Column(type="simple_array", enumType="Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\EnumInt")
*/
protected $enumIntArray;

/**
* @Column(type="custom_foo", enumType="Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\EnumInt")
*/
protected $enumCustom;
}