Skip to content

Commit

Permalink
Evaluate the impact of ignoring DC2Type comments
Browse files Browse the repository at this point in the history
This PR drops all calls to SchemManager::extractDoctrineTypeFromComment
found in src
I have commented out the tests that fail in order to spot tests that
fail when running more complete test suites that are available in the
CI.
  • Loading branch information
greg0ire committed Jan 17, 2022
1 parent 51b7dee commit de2b2ef
Show file tree
Hide file tree
Showing 13 changed files with 18 additions and 84 deletions.
10 changes: 2 additions & 8 deletions src/Platforms/AbstractPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -513,13 +513,7 @@ public function getDoctrineTypeComment(Type $doctrineType)
*/
protected function getColumnComment(Column $column)
{
$comment = $column->getComment();

if ($column->getType()->requiresSQLCommentHint($this)) {
$comment .= $this->getDoctrineTypeComment($column->getType());
}

return $comment;
return $column->getComment();
}

/**
Expand Down Expand Up @@ -3978,7 +3972,7 @@ private function columnToArray(Column $column): array
$columnData = array_merge($column->toArray(), [
'name' => $name,
'version' => $column->hasPlatformOption('version') ? $column->getPlatformOption('version') : false,
'comment' => $this->getColumnComment($column),
'comment' => $column->getComment(),
]);

if ($columnData['type'] instanceof Types\StringType && $columnData['length'] === null) {
Expand Down
4 changes: 2 additions & 2 deletions src/Schema/DB2SchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ protected function _getPortableTableColumnDefinition($tableColumn)
$type = $this->_platform->getDoctrineTypeMapping($tableColumn['typename']);

if (isset($tableColumn['comment'])) {
$type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type);
$tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type);
$typeFromComment = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type);
$tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $typeFromComment);
}

switch (strtolower($tableColumn['typename'])) {
Expand Down
4 changes: 2 additions & 2 deletions src/Schema/MySQLSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ protected function _getPortableTableColumnDefinition($tableColumn)

// In cases where not connected to a database DESCRIBE $table does not return 'Comment'
if (isset($tableColumn['comment'])) {
$type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type);
$tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type);
$typeFromComment = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type);
$tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $typeFromComment);
}

switch ($dbType) {
Expand Down
4 changes: 2 additions & 2 deletions src/Schema/OracleSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ protected function _getPortableTableColumnDefinition($tableColumn)
}

$type = $this->_platform->getDoctrineTypeMapping($dbType);
$type = $this->extractDoctrineTypeFromComment($tableColumn['comments'], $type);
$tableColumn['comments'] = $this->removeDoctrineTypeFromComment($tableColumn['comments'], $type);
$typeFromComment = $this->extractDoctrineTypeFromComment($tableColumn['comments'], $type);
$tableColumn['comments'] = $this->removeDoctrineTypeFromComment($tableColumn['comments'], $typeFromComment);

switch ($dbType) {
case 'number':
Expand Down
4 changes: 2 additions & 2 deletions src/Schema/PostgreSQLSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,8 @@ protected function _getPortableTableColumnDefinition($tableColumn)
}

$type = $this->_platform->getDoctrineTypeMapping($dbType);
$type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type);
$tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type);
$typeFromComment = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type);
$tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $typeFromComment);

switch ($dbType) {
case 'smallint':
Expand Down
4 changes: 2 additions & 2 deletions src/Schema/SQLServerSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ protected function _getPortableTableColumnDefinition($tableColumn)
}

$type = $this->_platform->getDoctrineTypeMapping($dbType);
$type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type);
$tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type);
$typeFromComment = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type);
$tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $typeFromComment);

$options = [
'unsigned' => false,
Expand Down
8 changes: 3 additions & 5 deletions src/Schema/SqliteSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -311,12 +311,10 @@ protected function _getPortableTableColumnList($table, $database, $tableColumns)
continue;
}

$type = $this->extractDoctrineTypeFromComment($comment, '');
$typeFromComment = $this->extractDoctrineTypeFromComment($comment, '');

if ($type !== '') {
$column->setType(Type::getType($type));

$comment = $this->removeDoctrineTypeFromComment($comment, $type);
if ($typeFromComment !== '') {
$comment = $this->removeDoctrineTypeFromComment($comment, $typeFromComment);
}

$column->setComment($comment);
Expand Down
4 changes: 0 additions & 4 deletions tests/Functional/Schema/Db2SchemaManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\DB2Platform;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Types\BooleanType;

class Db2SchemaManagerTest extends SchemaManagerFunctionalTestCase
{
Expand All @@ -24,9 +23,6 @@ public function testGetBooleanColumn(): void

$columns = $this->schemaManager->listTableColumns('boolean_column_test');

self::assertInstanceOf(BooleanType::class, $columns['bool']->getType());
self::assertInstanceOf(BooleanType::class, $columns['bool_commented']->getType());

self::assertNull($columns['bool']->getComment());
self::assertSame("That's a comment", $columns['bool_commented']->getComment());
}
Expand Down
12 changes: 0 additions & 12 deletions tests/Functional/Schema/MySQLSchemaManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use Doctrine\DBAL\Tests\Functional\Schema\MySQL\PointType;
use Doctrine\DBAL\Types\BlobType;
use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Types\Types;

class MySQLSchemaManagerTest extends SchemaManagerFunctionalTestCase
{
Expand Down Expand Up @@ -406,17 +405,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();
Expand Down
9 changes: 3 additions & 6 deletions tests/Functional/Schema/SchemaManagerFunctionalTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,11 @@
use Doctrine\DBAL\Schema\UniqueConstraint;
use Doctrine\DBAL\Schema\View;
use Doctrine\DBAL\Tests\FunctionalTestCase;
use Doctrine\DBAL\Types\ArrayType;
use Doctrine\DBAL\Types\BinaryType;
use Doctrine\DBAL\Types\BlobType;
use Doctrine\DBAL\Types\DateIntervalType;
use Doctrine\DBAL\Types\DateTimeType;
use Doctrine\DBAL\Types\DecimalType;
use Doctrine\DBAL\Types\IntegerType;
use Doctrine\DBAL\Types\ObjectType;
use Doctrine\DBAL\Types\StringType;
use Doctrine\DBAL\Types\TextType;
use Doctrine\DBAL\Types\Type;
Expand Down Expand Up @@ -927,9 +924,9 @@ public function testAutomaticallyAppendCommentOnMarkedColumns(): void
self::assertEquals(3, count($columns));
self::assertEquals('This is a comment', $columns['id']->getComment());
self::assertEquals('This is a comment', $columns['obj']->getComment());
self::assertInstanceOf(ObjectType::class, $columns['obj']->getType());
/* self::assertInstanceOf(ObjectType::class, $columns['obj']->getType()); */
self::assertEquals('This is a comment', $columns['arr']->getComment());
self::assertInstanceOf(ArrayType::class, $columns['arr']->getType());
/* self::assertInstanceOf(ArrayType::class, $columns['arr']->getType()); */
}

public function testCommentHintOnDateIntervalTypeColumn(): void
Expand All @@ -955,7 +952,7 @@ public function testCommentHintOnDateIntervalTypeColumn(): void
self::assertEquals(2, count($columns));
self::assertEquals('This is a comment', $columns['id']->getComment());
self::assertEquals('This is a comment', $columns['date_interval']->getComment());
self::assertInstanceOf(DateIntervalType::class, $columns['date_interval']->getType());
/* self::assertInstanceOf(DateIntervalType::class, $columns['date_interval']->getType()); */
}

public function testChangeColumnsTypeWithDefaultValue(): void
Expand Down
10 changes: 0 additions & 10 deletions tests/Platforms/AbstractPlatformTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -508,16 +508,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[]
*/
Expand Down
20 changes: 0 additions & 20 deletions tests/Platforms/PostgreSQLPlatformTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -878,26 +878,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'))]);

$comparator = new Comparator();

$tableDiff = $comparator->diffTable($table1, $table2);

self::assertNotFalse($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)';
Expand Down
9 changes: 0 additions & 9 deletions tests/Schema/ComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Types\Types;
use PHPUnit\Framework\TestCase;

use function array_keys;
Expand Down Expand Up @@ -1228,14 +1227,6 @@ public static function getCompareColumnComments(): iterable
];
}

public function testCompareCommentedTypes(): void
{
$column1 = new Column('foo', Type::getType(Types::ARRAY));
$column2 = new Column('foo', Type::getType(Types::OBJECT));

self::assertFalse($this->comparator->columnsEqual($column1, $column2));
}

public function testForeignKeyRemovalWithRenamedLocalColumn(): void
{
$fromSchema = new Schema([
Expand Down

0 comments on commit de2b2ef

Please sign in to comment.