Static analysis

doctrine · Jan 26, 2024 · cde8651 · cde8651
1 parent 0b5e343
commit cde8651
Show file tree

Hide file tree

Showing 14 changed files with 58 additions and 96 deletions.
diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php
@@ -334,7 +334,6 @@ private function buildTableOptions(array $options): string
      */
     public function getAlterTableSQL(TableDiff $diff): array
     {
-        $columnSql  = [];
         $queryParts = [];
 
         foreach ($diff->getAddedColumns() as $column) {
@@ -406,21 +405,18 @@ public function getAlterTableSQL(TableDiff $diff): array
             );
         }
 
-        $sql      = [];
         $tableSql = [];
 
         if (count($queryParts) > 0) {
-            $sql[] = 'ALTER TABLE ' . $diff->getOldTable()->getQuotedName($this) . ' '
+            $tableSql[] = 'ALTER TABLE ' . $diff->getOldTable()->getQuotedName($this) . ' '
                 . implode(', ', $queryParts);
         }
 
-        $sql = array_merge(
+        return array_merge(
             $this->getPreAlterTableIndexForeignKeySQL($diff),
-            $sql,
+            $tableSql,
             $this->getPostAlterTableIndexForeignKeySQL($diff),
         );
-
-        return array_merge($sql, $tableSql, $columnSql);
     }
 
     /**

diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php
@@ -815,8 +815,7 @@ private function buildCreateTableSQL(Table $table, bool $createForeignKeys): arr
             }
         }
 
-        $columnSql = [];
-        $columns   = [];
+        $columns = [];
 
         foreach ($table->getColumns() as $column) {
             $columnData = $this->columnToArray($column);
@@ -846,7 +845,7 @@ private function buildCreateTableSQL(Table $table, bool $createForeignKeys): arr
             }
         }
 
-        return array_merge($sql, $columnSql);
+        return $sql;
     }
 
     /**
@@ -944,7 +943,7 @@ public function getInlineColumnCommentSQL(string $comment): string
      * @param mixed[][] $columns
      * @param mixed[]   $options
      *
-     * @return array<int, string>
+     * @return list<string>
      */
     protected function _getCreateTableSQL(string $name, array $columns, array $options = []): array
     {
@@ -961,7 +960,7 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio
         }
 
         if (isset($options['indexes']) && ! empty($options['indexes'])) {
-            foreach ($options['indexes'] as $index => $definition) {
+            foreach ($options['indexes'] as $definition) {
                 $columnListSql .= ', ' . $this->getIndexDeclarationSQL($definition);
             }
         }
@@ -1297,7 +1296,7 @@ protected function getRenameIndexSQL(string $oldIndexName, Index $index, string
      * @param string $oldColumnName The name of the column we want to rename.
      * @param string $newColumnName The name we should rename it to.
      *
-     * @return string[] The sequence of SQL statements for renaming the given column.
+     * @return list<string> The sequence of SQL statements for renaming the given column.
      */
     protected function getRenameColumnSQL(string $tableName, string $oldColumnName, string $newColumnName): array
     {

diff --git a/src/Platforms/DB2Platform.php b/src/Platforms/DB2Platform.php
@@ -263,7 +263,6 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio
     public function getAlterTableSQL(TableDiff $diff): array
     {
         $sql         = [];
-        $columnSql   = [];
         $commentsSQL = [];
 
         $tableNameSQL = $diff->getOldTable()->getQuotedName($this);
@@ -331,14 +330,12 @@ public function getAlterTableSQL(TableDiff $diff): array
             $sql[] = "CALL SYSPROC.ADMIN_CMD ('REORG TABLE " . $tableNameSQL . "')";
         }
 
-        $sql = array_merge(
+        return array_merge(
             $this->getPreAlterTableIndexForeignKeySQL($diff),
             $sql,
             $commentsSQL,
             $this->getPostAlterTableIndexForeignKeySQL($diff),
         );
-
-        return array_merge($sql, $columnSql);
     }
 
     public function getRenameTableSQL(string $oldName, string $newName): string

diff --git a/src/Platforms/OraclePlatform.php b/src/Platforms/OraclePlatform.php
@@ -524,10 +524,8 @@ public function getDropDatabaseSQL(string $name): string
      */
     public function getAlterTableSQL(TableDiff $diff): array
     {
-        $sql         = [];
-        $commentsSQL = [];
-        $columnSql   = [];
-
+        $sql          = [];
+        $commentsSQL  = [];
         $addColumnSQL = [];
 
         $tableNameSQL = $diff->getOldTable()->getQuotedName($this);
@@ -629,7 +627,6 @@ public function getAlterTableSQL(TableDiff $diff): array
             $sql,
             $commentsSQL,
             $this->getPostAlterTableIndexForeignKeySQL($diff),
-            $columnSql,
         );
     }
 

diff --git a/src/Platforms/PostgreSQLPlatform.php b/src/Platforms/PostgreSQLPlatform.php
@@ -202,7 +202,6 @@ public function getAlterTableSQL(TableDiff $diff): array
     {
         $sql         = [];
         $commentsSQL = [];
-        $columnSql   = [];
 
         $table = $diff->getOldTable();
 
@@ -289,29 +288,22 @@ public function getAlterTableSQL(TableDiff $diff): array
                 $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ALTER ' . $newColumnName . ' ' . $query;
             }
 
-            if ($columnDiff->hasCommentChanged()) {
-                $commentsSQL[] = $this->getCommentOnColumnSQL(
-                    $tableNameSQL,
-                    $newColumn->getQuotedName($this),
-                    $newColumn->getComment(),
-                );
-            }
-
-            if (! $columnDiff->hasLengthChanged()) {
+            if (! $columnDiff->hasCommentChanged()) {
                 continue;
             }
 
-            $query = 'ALTER ' . $oldColumnName . ' TYPE '
-                . $newColumn->getType()->getSQLDeclaration($newColumn->toArray(), $this);
-            $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ' . $query;
+            $commentsSQL[] = $this->getCommentOnColumnSQL(
+                $tableNameSQL,
+                $newColumn->getQuotedName($this),
+                $newColumn->getComment(),
+            );
         }
 
         return array_merge(
             $this->getPreAlterTableIndexForeignKeySQL($diff),
             $sql,
             $commentsSQL,
             $this->getPostAlterTableIndexForeignKeySQL($diff),
-            $columnSql,
         );
     }
 

diff --git a/src/Platforms/SQLServerPlatform.php b/src/Platforms/SQLServerPlatform.php
@@ -358,7 +358,6 @@ public function getAlterTableSQL(TableDiff $diff): array
     {
         $queryParts  = [];
         $sql         = [];
-        $columnSql   = [];
         $commentsSql = [];
 
         $table = $diff->getOldTable();
@@ -481,7 +480,6 @@ public function getAlterTableSQL(TableDiff $diff): array
             $sql,
             $commentsSql,
             $this->getPostAlterTableIndexForeignKeySQL($diff),
-            $columnSql,
         );
     }
 
@@ -647,7 +645,7 @@ protected function getRenameIndexSQL(string $oldIndexName, Index $index, string
      * @param string $oldColumnName The name of the column we want to rename.
      * @param string $newColumnName The name we should rename it to.
      *
-     * @return string[] The sequence of SQL statements for renaming the given column.
+     * @return list<string> The sequence of SQL statements for renaming the given column.
      */
     protected function getRenameColumnSQL(string $tableName, string $oldColumnName, string $newColumnName): array
     {

diff --git a/src/Platforms/SQLitePlatform.php b/src/Platforms/SQLitePlatform.php
@@ -614,7 +614,6 @@ public function getAlterTableSQL(TableDiff $diff): array
         $columns        = [];
         $oldColumnNames = [];
         $newColumnNames = [];
-        $columnSql      = [];
 
         foreach ($table->getColumns() as $column) {
             $columnName                  = strtolower($column->getName());
@@ -690,7 +689,7 @@ public function getAlterTableSQL(TableDiff $diff): array
         );
         $sql[] = $this->getDropTableSQL($dataTable->getQuotedName($this));
 
-        return array_merge($sql, $this->getPostAlterTableIndexForeignKeySQL($diff), $columnSql);
+        return array_merge($sql, $this->getPostAlterTableIndexForeignKeySQL($diff));
     }
 
     /**
@@ -742,8 +741,7 @@ private function getSimpleAlterTableSQL(TableDiff $diff): array|false
 
         $table = $diff->getOldTable();
 
-        $sql       = [];
-        $columnSql = [];
+        $sql = [];
 
         foreach ($diff->getAddedColumns() as $column) {
             $definition = array_merge([
@@ -768,7 +766,7 @@ private function getSimpleAlterTableSQL(TableDiff $diff): array|false
                 . $this->getColumnDeclarationSQL($definition['name'], $definition);
         }
 
-        return array_merge($sql, $columnSql);
+        return $sql;
     }
 
     /** @return string[] */

diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php
@@ -194,7 +194,7 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff
                 continue;
             }
 
-            $removedColumnName = $renamedColumnNames[$addedColumn->getName()];
+            $removedColumnName = strtolower($renamedColumnNames[$addedColumn->getName()]);
             // Explicitly renamed columns need to be diffed, because their types can also have changed
             $modifiedColumns[$removedColumnName] = new ColumnDiff(
                 $droppedColumns[$removedColumnName],

diff --git a/tests/Functional/Platform/RenameColumnTest.php b/tests/Functional/Platform/RenameColumnTest.php
@@ -8,8 +8,9 @@
 use Doctrine\DBAL\Tests\FunctionalTestCase;
 use Doctrine\DBAL\Types\Type;
 use Doctrine\DBAL\Types\Types;
+use Throwable;
 
-use function array_values;
+use function var_dump;
 
 class RenameColumnTest extends FunctionalTestCase
 {
@@ -43,28 +44,34 @@ public function testColumnPositionRetainedAfterImplicitRenaming(string $columnNa
     /** @dataProvider columnNameProvider */
     public function testColumnPositionRetainedAfterExplicitRenaming(string $columnName, string $newColumnName): void
     {
-        $table = new Table('test_rename');
-        $table->addColumn($columnName, Types::INTEGER, ['length' => 16]);
-        $table->addColumn('c2', Types::INTEGER);
+            $table = new Table('test_rename');
+            $table->addColumn($columnName, Types::INTEGER, ['length' => 16]);
+            $table->addColumn('c2', Types::INTEGER);
 
-        $this->dropAndCreateTable($table);
+            $this->dropAndCreateTable($table);
 
-        // Force a different type to make sure it's not being caught implicitly
-        $table->renameColumn($columnName, $newColumnName)->setType(Type::getType(Types::BIGINT))->setLength(32);
+            // Force a different type to make sure it's not being caught implicitly
+            $table->renameColumn($columnName, $newColumnName)->setType(Type::getType(Types::BIGINT))->setLength(32);
 
-        $sm   =  $this->connection->createSchemaManager();
-        $diff = $sm->createComparator()
-            ->compareTables($sm->introspectTable('test_rename'), $table);
+            $sm   = $this->connection->createSchemaManager();
+            $diff = $sm->createComparator()
+                ->compareTables($sm->introspectTable('test_rename'), $table);
 
-        $sm->alterTable($diff);
+        try {
+            $sm->alterTable($diff);
 
-        $table   = $sm->introspectTable('test_rename');
-        $columns = array_values($table->getColumns());
+            $table   = $sm->introspectTable('test_rename');
+            $columns = $table->getColumns();
 
-        self::assertCount(1, $diff->getChangedColumns());
-        self::assertCount(2, $columns);
-        self::assertEqualsIgnoringCase($newColumnName, $columns[0]->getName());
-        self::assertEqualsIgnoringCase('c2', $columns[1]->getName());
+            self::assertCount(1, $diff->getChangedColumns());
+            self::assertCount(2, $columns);
+            self::assertEqualsIgnoringCase($newColumnName, $columns[0]->getName());
+            self::assertEqualsIgnoringCase('c2', $columns[1]->getName());
+        } catch (Throwable $e) {
+            var_dump($this->connection->getDatabasePlatform()->getAlterTableSQL($diff));
+
+            throw $e;
+        }
     }
 
     /** @return iterable<array{string,string}> */

diff --git a/tests/Platforms/AbstractPlatformTestCase.php b/tests/Platforms/AbstractPlatformTestCase.php
@@ -477,7 +477,7 @@ public function testAlterTableChangeQuotedColumn(): void
         $table->addColumn('select', Types::INTEGER);
 
         $tableDiff = new TableDiff($table, changedColumns: [
-            new ColumnDiff(
+            'select' => new ColumnDiff(
                 $table->getColumn('select'),
                 new Column(
                     'select',
@@ -859,7 +859,7 @@ public function testAlterStringToFixedString(): void
         $table->addColumn('name', Types::STRING, ['length' => 2]);
 
         $tableDiff = new TableDiff($table, changedColumns: [
-            new ColumnDiff(
+            'name' => new ColumnDiff(
                 $table->getColumn('name'),
                 new Column(
                     'name',
@@ -985,28 +985,3 @@ public static function asciiStringSqlDeclarationDataProvider(): array
         ];
     }
 }
-
-interface GetCreateTableSqlDispatchEventListener
-{
-    public function onSchemaCreateTable(): void;
-
-    public function onSchemaCreateTableColumn(): void;
-}
-
-interface GetAlterTableSqlDispatchEventListener
-{
-    public function onSchemaAlterTable(SchemaAlterTableEventArgs $args): void;
-
-    public function onSchemaAlterTableAddColumn(SchemaAlterTableAddColumnEventArgs $args): void;
-
-    public function onSchemaAlterTableRemoveColumn(SchemaAlterTableRemoveColumnEventArgs $args): void;
-
-    public function onSchemaAlterTableChangeColumn(SchemaAlterTableChangeColumnEventArgs $args): void;
-
-    public function onSchemaAlterTableRenameColumn(SchemaAlterTableRenameColumnEventArgs $args): void;
-}
-
-interface GetDropTableSqlDispatchEventListener
-{
-    public function onSchemaDropTable(): void;
-}
diff --git a/tests/Platforms/DB2PlatformTest.php b/tests/Platforms/DB2PlatformTest.php
@@ -448,7 +448,7 @@ public function testGeneratesAlterColumnSQL(
         bool $shouldReorg = true,
     ): void {
         $tableDiff = new TableDiff(new Table('foo'), changedColumns: [
-            new ColumnDiff($oldColumn, $newColumn),
+            $oldColumn->getName() => new ColumnDiff($oldColumn, $newColumn),
         ]);
 
         $expectedSQL = [];

diff --git a/tests/Platforms/SQLServerPlatformTest.php b/tests/Platforms/SQLServerPlatformTest.php
@@ -668,7 +668,7 @@ public function testAlterTableWithSchemaDropColumnComments(): void
         $table = new Table('testschema.mytable');
 
         $tableDiff = new TableDiff($table, changedColumns: [
-            new ColumnDiff(
+            'quota' => new ColumnDiff(
                 new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'A comment']),
                 new Column('quota', Type::getType(Types::INTEGER), []),
             ),
@@ -687,7 +687,7 @@ public function testAlterTableWithSchemaUpdateColumnComments(): void
         $table = new Table('testschema.mytable');
 
         $tableDiff = new TableDiff($table, changedColumns: [
-            new ColumnDiff(
+            'quota' => new ColumnDiff(
                 new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'A comment']),
                 new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'B comment']),
             ),
@@ -1062,7 +1062,7 @@ public function testAlterTableWithSchemaSameColumnComments(): void
         $table = new Table('testschema.mytable');
 
         $tableDiff = new TableDiff($table, changedColumns: [
-            new ColumnDiff(
+            'quota' => new ColumnDiff(
                 new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'A comment', 'notnull' => false]),
                 new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'A comment', 'notnull' => true]),
             ),

diff --git a/tests/Platforms/SQLitePlatformTest.php b/tests/Platforms/SQLitePlatformTest.php
@@ -303,11 +303,11 @@ public function testAlterTable(): void
         $diff = new TableDiff(
             $table,
             changedColumns: [
-                new ColumnDiff(
+                'id' => new ColumnDiff(
                     $table->getColumn('id'),
                     new Column('key', Type::getType(Types::INTEGER)),
                 ),
-                new ColumnDiff(
+                'post' => new ColumnDiff(
                     $table->getColumn('post'),
                     new Column('comment', Type::getType(Types::INTEGER)),
                 ),