Prevent column flagging as diff just because type differs (when one t…

…ype is parent to other)

This allows us, for example, to define an enum type that extends the string built-in and not be forced to modify the schema with Type::requiresSQLCommentHint()

lib/Doctrine/DBAL/Schema/Comparator.php

@@ -13,6 +13,7 @@
 use function assert;
 use function count;
 use function get_class;
+use function is_a;
 use function strtolower;
 
 /**
@@ -426,7 +427,11 @@ public function diffColumn(Column $column1, Column $column2)
 
  $changedProperties = [];
 
- if (get_class($properties1['type']) !== get_class($properties2['type'])) {
+ // Exception here helps with specific case of base string-type and a custom enum-type (inheriting from string).
+ // When doing migration diffs, there is no need to flag the column as different (unless options differ too)
+ $typeClass1 = get_class($properties1['type']);
+ $typeClass2 = get_class($properties2['type']);
+ if ($typeClass1 !== $typeClass2 && ! $this->isStringParentOf($typeClass1, $typeClass2)) {
  $changedProperties[] = 'type';
  }
 
@@ -508,6 +513,15 @@ public function diffColumn(Column $column1, Column $column2)
  return array_unique($changedProperties);
  }
 
+ private function isStringParentOf(string $type1, string $type2) : bool
+ {
+ if ($type1 !== Types\StringType::class) {
+ return false;
+ }
+
+ return is_a($type2, $type1, true);
+ }
+
  /**
  * TODO: kill with fire on v3.0
  *

tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php

@@ -13,8 +13,12 @@
 use Doctrine\DBAL\Schema\Sequence;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Schema\TableDiff;
+use Doctrine\DBAL\Types\StringType;
 use Doctrine\DBAL\Types\Type;
+use Doctrine\DBAL\Types\TypeRegistry;
+use Doctrine\DBAL\Types\Types;
 use PHPUnit\Framework\TestCase;
+use ReflectionClass;
 use function array_keys;
 use function get_class;
 
@@ -224,6 +228,61 @@ public function testCompareColumnsOverriddenType() : void
  self::assertEquals([], $c->diffColumn($column1, $column2));
  }
 
+ private function makeEnumType() : StringType
+ {
+ return new class extends StringType {
+ /**
+ * @inheritDoc
+ */
+ public function getName()
+ {
+ return 'custom_enum';
+ }
+ };
+ }
+
+ private function removeRegistryType(TypeRegistry $registry, string $type) : void
+ {
+ $reflection = new ReflectionClass($registry);
+ $typesListProperty = $reflection->getProperty('instances');
+ $typesListProperty->setAccessible(true);
+ $typesList = $typesListProperty->getValue($registry);
+ unset($typesList[$type]);
+ $typesListProperty->setValue($registry, $typesList);
+ }
+
+ public function testCompareColumnsStringParentTypeShouldNotBeDiff() : void
+ {
+ $typeRegistry = Type::getTypeRegistry();
+ $typeRegistry->register('custom_enum', $this->makeEnumType());
+ $fromType = Type::getType(Types::STRING);
+ $toType = Type::getType('custom_enum');
+
+ $sameOptions = ['length' => 50, 'default' => 'default_string'];
+ $column1 = new Column('column1', $fromType, $sameOptions);
+ $column2 = new Column('column2', $toType, $sameOptions);
+
+ $c = new Comparator();
+ self::assertEquals([], $c->diffColumn($column1, $column2));
+ $this->removeRegistryType($typeRegistry, 'custom_enum');
+ }
+
+ public function testCompareColumnsWithStringParentTypeShouldStillBeDiffWhenOptionsDiffer() : void
+ {
+ $typeRegistry = Type::getTypeRegistry();
+ $typeRegistry->register('custom_enum', $this->makeEnumType());
+ $fromType = Type::getType(Types::STRING);
+ $toType = Type::getType('custom_enum');
+ $options1 = ['length' => 50, 'default' => 'default_string'];
+ $options2 = ['length' => 10, 'default' => 'default_string'];
+ $column1 = new Column('column1', $fromType, $options1);
+ $column2 = new Column('column2', $toType, $options2);
+
+ $c = new Comparator();
+ self::assertEquals(['length'], $c->diffColumn($column1, $column2));
+ $this->removeRegistryType($typeRegistry, 'custom_enum');
+ }
+
  public function testCompareChangedColumnsChangeCustomSchemaOption() : void
  {
  $column1 = new Column('charfield1', Type::getType('string'));