Drop type comments entirely

This feature is only useful for reverse-engineering purposes since we
have platform-aware comparison. Reverse-engineering is not enough to
justify having that feature, and ORM metadata reverse-engineered from
the database can still be adjusted afterwards.

UPGRADE.md

@@ -8,6 +8,11 @@ awareness about deprecated code.
 
 # Upgrade to 4.0
 
+## BC BREAK: Types can no longer be reverse-engineered reliably
+
+Introspecting a table no longer guarantees getting the same column types that
+were used when creating that table.
+
 ## BC BREAK: Removed `AbstractPlatform::prefersIdentityColumns()`
 
 The `AbstractPlatform::prefersIdentityColumns()` method has been removed.

docs/en/sidebar.rst

@@ -20,5 +20,4 @@
     reference/upgrading
     reference/testing
 
-    explanation/dc2type-comments.rst
     explanation/implicit-indexes

src/Platforms/AbstractPlatform.php

@@ -375,26 +375,12 @@ public function hasDoctrineTypeMappingFor(string $dbType): bool
         return isset($this->doctrineTypeMapping[$dbType]);
     }
 
-    /**
-     * Gets the comment to append to a column comment that helps parsing this type in reverse engineering.
-     */
-    public function getDoctrineTypeComment(Type $doctrineType): string
-    {
-        return '(DC2Type:' . $doctrineType->getName() . ')';
-    }
-
     /**
      * Gets the comment of a passed column modified by potential doctrine type comment hints.
      */
     protected function getColumnComment(Column $column): string
     {
-        $comment = $column->getComment();
-
-        if ($column->getType()->requiresSQLCommentHint($this)) {
-            $comment .= $this->getDoctrineTypeComment($column->getType());
-        }
-
-        return $comment;
+        return $column->getComment();
     }
 
     /**
@@ -2510,7 +2496,7 @@ private function columnToArray(Column $column): array
         return array_merge($column->toArray(), [
             'name' => $column->getQuotedName($this),
             'version' => $column->hasPlatformOption('version') ? $column->getPlatformOption('version') : false,
-            'comment' => $this->getColumnComment($column),
+            'comment' => $column->getComment(),
         ]);
     }
 

src/Schema/AbstractSchemaManager.php

@@ -20,7 +20,6 @@
 use function array_values;
 use function assert;
 use function count;
-use function preg_match;
 use function strtolower;
 
 /**
@@ -845,23 +844,6 @@ public function createSchemaConfig(): SchemaConfig
         return $schemaConfig;
     }
 
-    /**
-     * Given a table comment this method tries to extract a type hint for Doctrine Type. If the type hint is found,
-     * it's removed from the comment.
-     *
-     * @return string|null The extracted Doctrine type or NULL of the type hint was not found.
-     */
-    final protected function extractDoctrineTypeFromComment(?string &$comment): ?string
-    {
-        if ($comment === null || preg_match('/(.*)\(DC2Type:(((?!\)).)+)\)(.*)/', $comment, $match) === 0) {
-            return null;
-        }
-
-        $comment = $match[1] . $match[4];
-
-        return $match[2];
-    }
-
     /**
      * @throws DatabaseRequired
      */

src/Schema/DB2SchemaManager.php

@@ -61,8 +61,7 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
             }
         }
 
-        $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'])
-            ?? $this->_platform->getDoctrineTypeMapping($tableColumn['typename']);
+        $type = $this->_platform->getDoctrineTypeMapping($tableColumn['typename']);
 
         switch (strtolower($tableColumn['typename'])) {
             case 'varchar':

src/Schema/MySQLSchemaManager.php

@@ -127,8 +127,7 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
         $scale     = 0;
         $precision = null;
 
-        $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'])
-            ?? $this->_platform->getDoctrineTypeMapping($dbType);
+        $type = $this->_platform->getDoctrineTypeMapping($dbType);
 
         switch ($dbType) {
             case 'char':

src/Schema/OracleSchemaManager.php

@@ -139,8 +139,7 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
             $scale = (int) $tableColumn['data_scale'];
         }
 
-        $type = $this->extractDoctrineTypeFromComment($tableColumn['comments'])
-            ?? $this->_platform->getDoctrineTypeMapping($dbType);
+        $type = $this->_platform->getDoctrineTypeMapping($dbType);
 
         switch ($dbType) {
             case 'number':

src/Schema/PostgreSQLSchemaManager.php

@@ -321,8 +321,7 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
             $tableColumn['complete_type'] = $tableColumn['domain_complete_type'];
         }
 
-        $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'])
-            ?? $this->_platform->getDoctrineTypeMapping($dbType);
+        $type = $this->_platform->getDoctrineTypeMapping($dbType);
 
         switch ($dbType) {
             case 'smallint':

src/Schema/SQLServerSchemaManager.php

@@ -108,8 +108,7 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
             $fixed = true;
         }
 
-        $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'])
-            ?? $this->_platform->getDoctrineTypeMapping($dbType);
+        $type = $this->_platform->getDoctrineTypeMapping($dbType);
 
         $options = [
             'fixed'         => $fixed,

src/Schema/SqliteSchemaManager.php

@@ -260,12 +260,6 @@ protected function _getPortableTableColumnList(string $table, string $database,
 
             $comment = $this->parseColumnCommentFromSQL($columnName, $createSql);
 
-            $type = $this->extractDoctrineTypeFromComment($comment);
-
-            if ($type !== null) {
-                $column->setType(Type::getType($type));
-            }
-
             $column->setComment($comment);
         }
 

tests/Functional/Schema/MySQLSchemaManagerTest.php

@@ -17,7 +17,6 @@
 use Doctrine\DBAL\Tests\TestUtil;
 use Doctrine\DBAL\Types\BlobType;
 use Doctrine\DBAL\Types\Type;
-use Doctrine\DBAL\Types\Types;
 
 class MySQLSchemaManagerTest extends SchemaManagerFunctionalTestCase
 {
@@ -391,17 +390,6 @@ public function testListFloatTypeColumns(): void
         self::assertTrue($columns['col_unsigned']->getUnsigned());
     }
 
-    public function testJsonColumnType(): void
-    {
-        $table = new Table('test_mysql_json');
-        $table->addColumn('col_json', 'json');
-        $this->dropAndCreateTable($table);
-
-        $columns = $this->schemaManager->listTableColumns('test_mysql_json');
-
-        self::assertSame(Types::JSON, $columns['col_json']->getType()->getName());
-    }
-
     public function testColumnDefaultCurrentTimestamp(): void
     {
         $platform = $this->schemaManager->getDatabasePlatform();

tests/Functional/Schema/SchemaManagerFunctionalTestCase.php

@@ -33,7 +33,6 @@
 use Doctrine\DBAL\Types\TextType;
 use Doctrine\DBAL\Types\Type;
 use Doctrine\DBAL\Types\Types;
-use ReflectionMethod;
 
 use function array_filter;
 use function array_keys;
@@ -1128,32 +1127,6 @@ public function testComparatorShouldNotAddCommentToJsonTypeSinceItIsTheDefaultNo
         self::assertNull($tableDiff);
     }
 
-    /**
-     * @dataProvider commentsProvider
-     */
-    public function testExtractDoctrineTypeFromComment(string $comment, ?string $expectedType): void
-    {
-        $re = new ReflectionMethod($this->schemaManager, 'extractDoctrineTypeFromComment');
-        $re->setAccessible(true);
-
-        self::assertSame($expectedType, $re->invokeArgs($this->schemaManager, [&$comment]));
-    }
-
-    /**
-     * @return mixed[][]
-     */
-    public static function commentsProvider(): iterable
-    {
-        return [
-            'invalid custom type comments'      => ['should.return.null', null],
-            'valid doctrine type'               => ['(DC2Type:guid)', 'guid'],
-            'valid with dots'                   => ['(DC2Type:type.should.return)', 'type.should.return'],
-            'valid with namespace'              => ['(DC2Type:Namespace\Class)', 'Namespace\Class'],
-            'valid with extra closing bracket'  => ['(DC2Type:should.stop)).before)', 'should.stop'],
-            'valid with extra opening brackets' => ['(DC2Type:should((.stop)).before)', 'should((.stop'],
-        ];
-    }
-
     public function testCreateAndListSequences(): void
     {
         if (! $this->schemaManager->getDatabasePlatform()->supportsSequences()) {

tests/Platforms/AbstractMySQLPlatformTestCase.php

@@ -223,10 +223,7 @@ public function getAlterTableColumnCommentsSQL(): array
      */
     public function getCreateTableColumnTypeCommentsSQL(): array
     {
-        return [
-            "CREATE TABLE test (id INT NOT NULL, data LONGTEXT NOT NULL COMMENT '(DC2Type:array)', "
-                . 'PRIMARY KEY(id))',
-        ];
+        return ['CREATE TABLE test (id INT NOT NULL, data LONGTEXT NOT NULL, PRIMARY KEY(id))'];
     }
 
     public function testChangeIndexWithForeignKeys(): void

tests/Platforms/AbstractPlatformTestCase.php

@@ -448,16 +448,6 @@ public function testAlterTableColumnComments(): void
         self::assertEquals($this->getAlterTableColumnCommentsSQL(), $this->platform->getAlterTableSQL($tableDiff));
     }
 
-    public function testCreateTableColumnTypeComments(): void
-    {
-        $table = new Table('test');
-        $table->addColumn('id', 'integer');
-        $table->addColumn('data', 'array');
-        $table->setPrimaryKey(['id']);
-
-        self::assertEquals($this->getCreateTableColumnTypeCommentsSQL(), $this->platform->getCreateTableSQL($table));
-    }
-
     /**
      * @return string[]
      */

tests/Platforms/DB2PlatformTest.php

@@ -164,10 +164,7 @@ public function getAlterTableColumnCommentsSQL(): array
      */
     public function getCreateTableColumnTypeCommentsSQL(): array
     {
-        return [
-            'CREATE TABLE test (id INTEGER NOT NULL, "data" CLOB(1M) NOT NULL, PRIMARY KEY(id))',
-            'COMMENT ON COLUMN test."data" IS \'(DC2Type:array)\'',
-        ];
+        return ['CREATE TABLE test (id INTEGER NOT NULL, "data" CLOB(1M) NOT NULL, PRIMARY KEY(id))'];
     }
 
     public function testGeneratesCreateTableSQLWithCommonIndexes(): void

tests/Platforms/OraclePlatformTest.php

@@ -288,10 +288,7 @@ public function getCreateTableColumnCommentsSQL(): array
      */
     public function getCreateTableColumnTypeCommentsSQL(): array
     {
-        return [
-            'CREATE TABLE test (id NUMBER(10) NOT NULL, data CLOB NOT NULL, PRIMARY KEY(id))',
-            "COMMENT ON COLUMN test.data IS '(DC2Type:array)'",
-        ];
+        return ['CREATE TABLE test (id NUMBER(10) NOT NULL, data CLOB NOT NULL, PRIMARY KEY(id))'];
     }
 
     /**

tests/Platforms/PostgreSQLPlatformTest.php

@@ -370,10 +370,7 @@ public function getAlterTableColumnCommentsSQL(): array
      */
     public function getCreateTableColumnTypeCommentsSQL(): array
     {
-        return [
-            'CREATE TABLE test (id INT NOT NULL, data TEXT NOT NULL, PRIMARY KEY(id))',
-            "COMMENT ON COLUMN test.data IS '(DC2Type:array)'",
-        ];
+        return ['CREATE TABLE test (id INT NOT NULL, data TEXT NOT NULL, PRIMARY KEY(id))'];
     }
 
     /**
@@ -646,8 +643,7 @@ public function testDoesNotPropagateUnnecessaryTableAlterationOnBinaryType(): vo
         // BINARY    -> VARBINARY
         // BLOB      -> VARBINARY
         $diff = $comparator->diffTable($table1, $table2);
-        self::assertNotNull($diff);
-        self::assertEmpty($this->platform->getAlterTableSQL($diff));
+        self::assertNull($diff);
 
         $table2 = new Table('mytable');
         $table2->addColumn('column_varbinary', 'binary', ['length' => 42]);
@@ -658,8 +654,7 @@ public function testDoesNotPropagateUnnecessaryTableAlterationOnBinaryType(): vo
         // BINARY    -> BLOB
         // BLOB      -> BINARY
         $diff = $comparator->diffTable($table1, $table2);
-        self::assertNotNull($diff);
-        self::assertEmpty($this->platform->getAlterTableSQL($diff));
+        self::assertNull($diff);
 
         $table2 = new Table('mytable');
         $table2->addColumn('column_varbinary', 'blob');
@@ -670,8 +665,7 @@ public function testDoesNotPropagateUnnecessaryTableAlterationOnBinaryType(): vo
         // BINARY    -> BINARY with changed length
         // BLOB      -> BLOB
         $diff = $comparator->diffTable($table1, $table2);
-        self::assertNotNull($diff);
-        self::assertEmpty($this->platform->getAlterTableSQL($diff));
+        self::assertNull($diff);
     }
 
     /**
@@ -830,25 +824,6 @@ public function testAltersTableColumnCommentWithExplicitlyQuotedIdentifiers(): v
         );
     }
 
-    public function testAltersTableColumnCommentIfRequiredByType(): void
-    {
-        $table1 = new Table('"foo"', [new Column('"bar"', Type::getType('datetime'))]);
-        $table2 = new Table('"foo"', [new Column('"bar"', Type::getType('datetime_immutable'))]);
-
-        $tableDiff = $this->createComparator()
-            ->diffTable($table1, $table2);
-
-        self::assertNotNull($tableDiff);
-        self::assertSame(
-            [
-                'ALTER TABLE "foo" ALTER "bar" TYPE TIMESTAMP(0) WITHOUT TIME ZONE',
-                'ALTER TABLE "foo" ALTER "bar" DROP DEFAULT',
-                'COMMENT ON COLUMN "foo"."bar" IS \'(DC2Type:datetime_immutable)\'',
-            ],
-            $this->platform->getAlterTableSQL($tableDiff)
-        );
-    }
-
     protected function getQuotesReservedKeywordInUniqueConstraintDeclarationSQL(): string
     {
         return 'CONSTRAINT "select" UNIQUE (foo)';