Merge pull request doctrine#5775 from morozov/remove-comparator-diff-…

…table Remove Comparator::diffTable()
derrabus · Oct 18, 2022 · 88cfa33 · 88cfa33
2 parents 26b6351 + d9825af
commit 88cfa33
Show file tree

Hide file tree

Showing 28 changed files with 169 additions and 213 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -645,7 +645,7 @@ The method `Comparator::compareSchemas()` cannot be called statically anymore.
 
 ## Removed `Comparator` methods
 
-The `Comparator::compare()` and `::diffColumn()` methods have been removed.
+The `Comparator::compare()`, `::diffTable()` and `::diffColumn()` methods have been removed.
 
 ## Removed `ColumnDiff` methods
 
@@ -736,7 +736,6 @@ Table columns are no longer indexed by column name. Use the `name` attribute of
 ## BC BREAK: Changes in the `Doctrine\DBAL\Schema` API
 
 - Method `Doctrine\DBAL\Schema\AbstractSchemaManager::_getPortableViewDefinition()` no longer optionally returns false. It will always return a `Doctrine\DBAL\Schema\View` instance.
-- Method `Doctrine\DBAL\Schema\Comparator::diffTable()` now optionally returns null instead of false.
 - Property `Doctrine\DBAL\Schema\Table::$_primaryKeyName` is now optionally null instead of false.
 - Method `Doctrine\DBAL\Schema\AbstractSchemaManager::tablesExist()` no longer accepts a string. Use `Doctrine\DBAL\Schema\AbstractSchemaManager::tableExists()` instead.
 - Method `Doctrine\DBAL\Schema\OracleSchemaManager::createDatabase()` no longer accepts `null` for `$database` argument.

diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -39,10 +39,6 @@
                     See https://github.com/doctrine/dbal/pull/4317
                 -->
                 <file name="tests/Functional/LegacyAPITest.php"/>
-                <!--
-                    TODO: remove in 4.0.0
-                -->
-                <referencedMethod name="Doctrine\DBAL\Schema\Comparator::diffTable"/>
             </errorLevel>
         </DeprecatedMethod>
         <DocblockTypeContradiction>

diff --git a/src/Platforms/MySQL/Comparator.php b/src/Platforms/MySQL/Comparator.php
@@ -30,9 +30,9 @@ public function __construct(
         parent::__construct($platform);
     }
 
-    public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
+    public function compareTables(Table $oldTable, Table $newTable): TableDiff
     {
-        return parent::diffTable(
+        return parent::compareTables(
             $this->normalizeTable($oldTable),
             $this->normalizeTable($newTable),
         );

diff --git a/src/Platforms/SQLServer/Comparator.php b/src/Platforms/SQLServer/Comparator.php
@@ -22,15 +22,15 @@ public function __construct(SQLServerPlatform $platform, private readonly string
         parent::__construct($platform);
     }
 
-    public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
+    public function compareTables(Table $oldTable, Table $newTable): TableDiff
     {
         $oldTable = clone $oldTable;
         $newTable = clone $newTable;
 
         $this->normalizeColumns($oldTable);
         $this->normalizeColumns($newTable);
 
-        return parent::diffTable($oldTable, $newTable);
+        return parent::compareTables($oldTable, $newTable);
     }
 
     private function normalizeColumns(Table $table): void

diff --git a/src/Platforms/SQLite/Comparator.php b/src/Platforms/SQLite/Comparator.php
@@ -24,15 +24,15 @@ public function __construct(SQLitePlatform $platform)
         parent::__construct($platform);
     }
 
-    public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
+    public function compareTables(Table $oldTable, Table $newTable): TableDiff
     {
         $oldTable = clone $oldTable;
         $newTable = clone $newTable;
 
         $this->normalizeColumns($oldTable);
         $this->normalizeColumns($newTable);
 
-        return parent::diffTable($oldTable, $newTable);
+        return parent::compareTables($oldTable, $newTable);
     }
 
     private function normalizeColumns(Table $table): void

diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php
@@ -6,7 +6,6 @@
 
 use Doctrine\DBAL\Exception;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
-use Doctrine\Deprecations\Deprecation;
 
 use function array_map;
 use function assert;
@@ -25,6 +24,8 @@ public function __construct(private readonly AbstractPlatform $platform)
 
     /**
      * Returns the differences between the schemas.
+     *
+     * @throws Exception
      */
     public function compareSchemas(Schema $oldSchema, Schema $newSchema): SchemaDiff
     {
@@ -58,13 +59,13 @@ public function compareSchemas(Schema $oldSchema, Schema $newSchema): SchemaDiff
             if (! $oldSchema->hasTable($newTableName)) {
                 $createdTables[] = $newSchema->getTable($newTableName);
             } else {
-                $tableDifferences = $this->diffTable(
+                $tableDiff = $this->compareTables(
                     $oldSchema->getTable($newTableName),
                     $newSchema->getTable($newTableName),
                 );
 
-                if ($tableDifferences !== null) {
-                    $alteredTables[] = $tableDifferences;
+                if (! $tableDiff->isEmpty()) {
+                    $alteredTables[] = $tableDiff;
                 }
             }
         }
@@ -140,33 +141,6 @@ public function diffSequence(Sequence $sequence1, Sequence $sequence2): bool
         return $sequence1->getInitialValue() !== $sequence2->getInitialValue();
     }
 
-    /**
-     * Returns the difference between the tables.
-     *
-     * If there are no differences this method returns null.
-     *
-     * @deprecated Use {@see compareTables()} and, optionally, {@see TableDiff::isEmpty()} instead.
-     *
-     * @throws Exception
-     */
-    public function diffTable(Table $oldTable, Table $newTable): ?TableDiff
-    {
-        Deprecation::triggerIfCalledFromOutside(
-            'doctrine/dbal',
-            'https://github.com/doctrine/dbal/pull/5770',
-            '%s is deprecated. Use compareTables() instead.',
-            __METHOD__,
-        );
-
-        $diff = $this->compareTables($oldTable, $newTable);
-
-        if ($diff->isEmpty()) {
-            return null;
-        }
-
-        return $diff;
-    }
-
     /**
      * Compares the tables and returns the difference between them.
      *

diff --git a/tests/Functional/Platform/AddColumnWithDefaultTest.php b/tests/Functional/Platform/AddColumnWithDefaultTest.php
@@ -26,11 +26,11 @@ public function testAddColumnWithDefault(): void
             'default' => 'DEFAULT',
         ]);
 
-        $diff = $schemaManager->createComparator()->diffTable(
+        $diff = $schemaManager->createComparator()->compareTables(
             $schemaManager->introspectTable('add_default_test'),
             $table,
         );
-        self::assertNotNull($diff);
+
         $schemaManager->alterTable($diff);
 
         $query  = 'SELECT original_field, new_field FROM add_default_test';

diff --git a/tests/Functional/Platform/AlterColumnTest.php b/tests/Functional/Platform/AlterColumnTest.php
@@ -25,9 +25,8 @@ public function testColumnPositionRetainedAfterAltering(): void
 
         $sm   = $this->connection->createSchemaManager();
         $diff = $sm->createComparator()
-            ->diffTable($sm->introspectTable('test_alter'), $table);
+            ->compareTables($sm->introspectTable('test_alter'), $table);
 
-        self::assertNotNull($diff);
         $sm->alterTable($diff);
 
         $table   = $sm->introspectTable('test_alter');

diff --git a/tests/Functional/Platform/AlterDecimalColumnTest.php b/tests/Functional/Platform/AlterDecimalColumnTest.php
@@ -24,8 +24,8 @@ public function testAlterPrecisionAndScale(int $newPrecision, int $newScale): vo
         $schemaManager = $this->connection->createSchemaManager();
 
         $diff = $schemaManager->createComparator()
-            ->diffTable($schemaManager->introspectTable('decimal_table'), $table);
-        self::assertNotNull($diff);
+            ->compareTables($schemaManager->introspectTable('decimal_table'), $table);
+
         $schemaManager->alterTable($diff);
 
         $table  = $schemaManager->introspectTable('decimal_table');

diff --git a/tests/Functional/Platform/RenameColumnTest.php b/tests/Functional/Platform/RenameColumnTest.php
@@ -23,9 +23,8 @@ public function testColumnPositionRetainedAfterRenaming(string $columnName, stri
 
         $sm   =  $this->connection->createSchemaManager();
         $diff = $sm->createComparator()
-            ->diffTable($sm->introspectTable('test_rename'), $table);
+            ->compareTables($sm->introspectTable('test_rename'), $table);
 
-        self::assertNotNull($diff);
         $sm->alterTable($diff);
 
         $table   = $sm->introspectTable('test_rename');

diff --git a/tests/Functional/Schema/ColumnCommentTest.php b/tests/Functional/Schema/ColumnCommentTest.php
@@ -93,8 +93,7 @@ public function testAlterColumnComment(string $comment1, string $comment2): void
         $schemaManager = $this->connection->createSchemaManager();
 
         $diff = $schemaManager->createComparator()
-            ->diffTable($table1, $table2);
-        self::assertNotNull($diff);
+            ->compareTables($table1, $table2);
 
         $schemaManager->alterTable($diff);
 

diff --git a/tests/Functional/Schema/ComparatorTest.php b/tests/Functional/Schema/ComparatorTest.php
@@ -39,9 +39,10 @@ public function testDefaultValueComparison(string $type, mixed $value): void
 
         $onlineTable = $this->schemaManager->introspectTable('default_value');
 
-        self::assertNull(
+        self::assertTrue(
             $this->schemaManager->createComparator()
-                ->diffTable($table, $onlineTable),
+                ->compareTables($table, $onlineTable)
+                ->isEmpty(),
         );
     }
 

diff --git a/tests/Functional/Schema/ComparatorTestUtils.php b/tests/Functional/Schema/ComparatorTestUtils.php
@@ -19,8 +19,8 @@ public static function diffFromActualToDesiredTable(
         AbstractSchemaManager $schemaManager,
         Comparator $comparator,
         Table $desiredTable,
-    ): ?TableDiff {
-        return $comparator->diffTable(
+    ): TableDiff {
+        return $comparator->compareTables(
             $schemaManager->introspectTable($desiredTable->getName()),
             $desiredTable,
         );
@@ -31,8 +31,8 @@ public static function diffFromDesiredToActualTable(
         AbstractSchemaManager $schemaManager,
         Comparator $comparator,
         Table $desiredTable,
-    ): ?TableDiff {
-        return $comparator->diffTable(
+    ): TableDiff {
+        return $comparator->compareTables(
             $desiredTable,
             $schemaManager->introspectTable($desiredTable->getName()),
         );
@@ -44,11 +44,17 @@ public static function assertDiffNotEmpty(Connection $connection, Comparator $co
 
         $diff = self::diffFromActualToDesiredTable($schemaManager, $comparator, $table);
 
-        TestCase::assertNotNull($diff);
+        TestCase::assertFalse($diff->isEmpty());
 
         $schemaManager->alterTable($diff);
 
-        TestCase::assertNull(self::diffFromActualToDesiredTable($schemaManager, $comparator, $table));
-        TestCase::assertNull(self::diffFromDesiredToActualTable($schemaManager, $comparator, $table));
+        TestCase::assertTrue(
+            self::diffFromActualToDesiredTable($schemaManager, $comparator, $table)
+                ->isEmpty(),
+        );
+        TestCase::assertTrue(
+            self::diffFromDesiredToActualTable($schemaManager, $comparator, $table)
+                ->isEmpty(),
+        );
     }
 }
diff --git a/tests/Functional/Schema/MySQL/ComparatorTest.php b/tests/Functional/Schema/MySQL/ComparatorTest.php
@@ -41,17 +41,17 @@ public function testLobLengthIncrementWithinLimit(string $type, int $length): vo
         $table = $this->createLobTable($type, $length - 1);
         $this->setBlobLength($table, $length);
 
-        self::assertNull(ComparatorTestUtils::diffFromActualToDesiredTable(
+        self::assertTrue(ComparatorTestUtils::diffFromActualToDesiredTable(
             $this->schemaManager,
             $this->comparator,
             $table,
-        ));
+        )->isEmpty());
 
-        self::assertNull(ComparatorTestUtils::diffFromDesiredToActualTable(
+        self::assertTrue(ComparatorTestUtils::diffFromDesiredToActualTable(
             $this->schemaManager,
             $this->comparator,
             $table,
-        ));
+        )->isEmpty());
     }
 
     /** @dataProvider lobColumnProvider */
@@ -96,17 +96,17 @@ public function testExplicitDefaultCollation(): void
         [$table, $column] = $this->createCollationTable();
         $column->setPlatformOption('collation', 'utf8mb4_general_ci');
 
-        self::assertNull(ComparatorTestUtils::diffFromActualToDesiredTable(
+        self::assertTrue(ComparatorTestUtils::diffFromActualToDesiredTable(
             $this->schemaManager,
             $this->comparator,
             $table,
-        ));
+        )->isEmpty());
 
-        self::assertNull(ComparatorTestUtils::diffFromDesiredToActualTable(
+        self::assertTrue(ComparatorTestUtils::diffFromDesiredToActualTable(
             $this->schemaManager,
             $this->comparator,
             $table,
-        ));
+        )->isEmpty());
     }
 
     public function testChangeColumnCharsetAndCollation(): void
@@ -142,17 +142,17 @@ public function testTableAndColumnOptions(array $tableOptions, array $columnOpti
 
         $this->dropAndCreateTable($table);
 
-        self::assertNull(ComparatorTestUtils::diffFromActualToDesiredTable(
+        self::assertTrue(ComparatorTestUtils::diffFromActualToDesiredTable(
             $this->schemaManager,
             $this->comparator,
             $table,
-        ));
+        )->isEmpty());
 
-        self::assertNull(ComparatorTestUtils::diffFromDesiredToActualTable(
+        self::assertTrue(ComparatorTestUtils::diffFromDesiredToActualTable(
             $this->schemaManager,
             $this->comparator,
             $table,
-        ));
+        )->isEmpty());
     }
 
     /** @return iterable<string,array{array<string,string>,array<string,string>}> */