Merge pull request #5770 from morozov/deprecate-comparator-diff-table

Non-nullabe result of comparing tables
doctrine · Oct 18, 2022 · 45a2bb4 · 45a2bb4
2 parents 63b94e1 + acc2c8a
commit 45a2bb4
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 23 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -121,6 +121,36 @@ The "unique" and "check" column properties have been deprecated. Use unique cons
 Relying on the default precision and scale of decimal columns provided by the DBAL is deprecated.
 When declaring decimal columns, specify the precision and scale explicitly.
 
+## Deprecated `Comparator::diffTable()` method.
+
+The `Comparator::diffTable()` method has been deprecated in favor of `Comparator::compareTables()`
+and `TableDiff::isEmpty()`.
+
+Instead of having to check whether the diff is equal to the boolean `false`, you can optionally check
+if the returned table diff is empty.
+
+### Before
+
+```php
+$diff = $comparator->diffTable($oldTable, $newTable);
+
+// mandatory check
+if ($diff !== false) {
+    // we have a diff
+}
+```
+
+### After
+
+```php
+$diff = $comparator->compareTables($oldTable, $newTable);
+
+// optional check
+if (! $diff->isEmpty()) {
+    // we have a diff
+}
+```
+
 ## Deprecated not passing `$fromColumn` to the `TableDiff` constructor.
 
 Not passing `$fromColumn` to the `TableDiff` constructor has been deprecated.

diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -446,6 +446,10 @@
                     TODO: remove in 4.0.0
                 -->
                 <referencedMethod name="Doctrine\DBAL\Schema\SchemaDiff::toSql"/>
+                <!--
+                    TODO: remove in 4.0.0
+                -->
+                <referencedMethod name="Doctrine\DBAL\Schema\Comparator::diffTable"/>
             </errorLevel>
         </DeprecatedMethod>
         <DeprecatedProperty>

diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php
@@ -280,14 +280,37 @@ public function diffSequence(Sequence $sequence1, Sequence $sequence2)
      *
      * If there are no differences this method returns the boolean false.
      *
+     * @deprecated Use {@see compareTables()} and, optionally, {@see TableDiff::isEmpty()} instead.
+     *
      * @return TableDiff|false
      *
      * @throws Exception
      */
     public function diffTable(Table $fromTable, Table $toTable)
     {
-        $hasChanges = false;
+        Deprecation::triggerIfCalledFromOutside(
+            'doctrine/dbal',
+            'https://github.com/doctrine/dbal/pull/5770',
+            '%s is deprecated. Use compareTables() instead.',
+            __METHOD__,
+        );
+
+        $diff = $this->compareTables($fromTable, $toTable);
 
+        if ($diff->isEmpty()) {
+            return false;
+        }
+
+        return $diff;
+    }
+
+    /**
+     * Compares the tables and returns the difference between them.
+     *
+     * @throws Exception
+     */
+    public function compareTables(Table $fromTable, Table $toTable): TableDiff
+    {
         $addedColumns        = [];
         $modifiedColumns     = [];
         $droppedColumns      = [];
@@ -308,8 +331,6 @@ public function diffTable(Table $fromTable, Table $toTable)
             }
 
             $addedColumns[$columnName] = $column;
-
-            $hasChanges = true;
         }
 
         /* See if there are any removed columns in "to" table */
@@ -318,7 +339,6 @@ public function diffTable(Table $fromTable, Table $toTable)
             if (! $toTable->hasColumn($columnName)) {
                 $droppedColumns[$columnName] = $column;
 
-                $hasChanges = true;
                 continue;
             }
 
@@ -341,8 +361,6 @@ public function diffTable(Table $fromTable, Table $toTable)
                 $changedProperties,
                 $column,
             );
-
-            $hasChanges = true;
         }
 
         $renamedColumns = $this->detectRenamedColumns($addedColumns, $droppedColumns);
@@ -357,8 +375,6 @@ public function diffTable(Table $fromTable, Table $toTable)
             }
 
             $addedIndexes[$indexName] = $index;
-
-            $hasChanges = true;
         }
 
         /* See if there are any removed indexes in "to" table */
@@ -370,7 +386,6 @@ public function diffTable(Table $fromTable, Table $toTable)
             ) {
                 $droppedIndexes[$indexName] = $index;
 
-                $hasChanges = true;
                 continue;
             }
 
@@ -383,8 +398,6 @@ public function diffTable(Table $fromTable, Table $toTable)
             }
 
             $modifiedIndexes[$indexName] = $toTableIndex;
-
-            $hasChanges = true;
         }
 
         $renamedIndexes = $this->detectRenamedIndexes($addedIndexes, $droppedIndexes);
@@ -400,7 +413,6 @@ public function diffTable(Table $fromTable, Table $toTable)
                     if (strtolower($fromConstraint->getName()) === strtolower($toConstraint->getName())) {
                         $modifiedForeignKeys[] = $toConstraint;
 
-                        $hasChanges = true;
                         unset($fromForeignKeys[$fromKey], $toForeignKeys[$toKey]);
                     }
                 }
@@ -409,18 +421,10 @@ public function diffTable(Table $fromTable, Table $toTable)
 
         foreach ($fromForeignKeys as $fromConstraint) {
             $droppedForeignKeys[] = $fromConstraint;
-
-            $hasChanges = true;
         }
 
         foreach ($toForeignKeys as $toConstraint) {
             $addedForeignKeys[] = $toConstraint;
-
-            $hasChanges = true;
-        }
-
-        if (! $hasChanges) {
-            return false;
         }
 
         return new TableDiff(
@@ -598,7 +602,7 @@ public function columnsEqual(Column $column1, Column $column2): bool
      * If there are differences this method returns the changed properties as a
      * string array, otherwise an empty array gets returned.
      *
-     * @deprecated Use {@see diffTable()} instead.
+     * @deprecated Use {@see columnsEqual()} instead.
      *
      * @return string[]
      */

diff --git a/src/Schema/SchemaDiff.php b/src/Schema/SchemaDiff.php
@@ -5,7 +5,9 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\Deprecations\Deprecation;
 
+use function array_filter;
 use function array_merge;
+use function count;
 
 /**
  * Differences between two schemas.
@@ -120,8 +122,12 @@ public function __construct(
         $alteredSequences = [],
         $droppedSequences = []
     ) {
-        $this->newTables         = $newTables;
-        $this->changedTables     = $changedTables;
+        $this->newTables = $newTables;
+
+        $this->changedTables = array_filter($changedTables, static function (TableDiff $diff): bool {
+            return ! $diff->isEmpty();
+        });
+
         $this->removedTables     = $removedTables;
         $this->fromSchema        = $fromSchema;
         $this->newNamespaces     = $createdSchemas;
@@ -179,6 +185,21 @@ public function getDroppedSequences(): array
         return $this->removedSequences;
     }
 
+    /**
+     * Returns whether the diff is empty (contains no changes).
+     */
+    public function isEmpty(): bool
+    {
+        return count($this->newNamespaces) === 0
+            && count($this->removedNamespaces) === 0
+            && count($this->newTables) === 0
+            && count($this->changedTables) === 0
+            && count($this->removedTables) === 0
+            && count($this->newSequences) === 0
+            && count($this->changedSequences) === 0
+            && count($this->removedSequences) === 0;
+    }
+
     /**
      * The to save sql mode ensures that the following things don't happen:
      *

diff --git a/src/Schema/TableDiff.php b/src/Schema/TableDiff.php
@@ -7,6 +7,7 @@
 
 use function array_filter;
 use function array_values;
+use function count;
 
 /**
  * Table Diff.
@@ -339,4 +340,22 @@ static function ($removedForeignKey) use ($foreignKey): bool {
             },
         );
     }
+
+    /**
+     * Returns whether the diff is empty (contains no changes).
+     */
+    public function isEmpty(): bool
+    {
+        return count($this->addedColumns) === 0
+            && count($this->changedColumns) === 0
+            && count($this->removedColumns) === 0
+            && count($this->renamedColumns) === 0
+            && count($this->addedIndexes) === 0
+            && count($this->changedIndexes) === 0
+            && count($this->removedIndexes) === 0
+            && count($this->renamedIndexes) === 0
+            && count($this->addedForeignKeys) === 0
+            && count($this->changedForeignKeys) === 0
+            && count($this->removedForeignKeys) === 0;
+    }
 }
diff --git a/tests/Platforms/AbstractPlatformTestCase.php b/tests/Platforms/AbstractPlatformTestCase.php
@@ -14,6 +14,7 @@
 use Doctrine\DBAL\Schema\Comparator;
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
 use Doctrine\DBAL\Schema\Index;
+use Doctrine\DBAL\Schema\SchemaDiff;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Schema\TableDiff;
 use Doctrine\DBAL\Schema\UniqueConstraint;
@@ -1382,6 +1383,22 @@ public function requiresSQLCommentHint(AbstractPlatform $platform): bool
         self::assertTrue($this->platform->isCommentedDoctrineType($type));
     }
 
+    public function testEmptyTableDiff(): void
+    {
+        $diff = new TableDiff('test');
+
+        self::assertTrue($diff->isEmpty());
+        self::assertSame([], $this->platform->getAlterTableSQL($diff));
+    }
+
+    public function testEmptySchemaDiff(): void
+    {
+        $diff = new SchemaDiff();
+
+        self::assertTrue($diff->isEmpty());
+        self::assertSame([], $this->platform->getAlterSchemaSQL($diff));
+    }
+
     public function tearDown(): void
     {
         if (! isset($this->backedUpType)) {