Incorporated code review suggestions

UPGRADE.md

 # Upgrade to 3.0
 
+## BC BREAK: Doctrine\DBAL\Schema\Table constructor new parameter
+
+Deprecated parameter `$idGeneratorType` removed and added a new parameter `$uniqueConstraints`.
+Constructor changed from:
+
+`__construct($tableName, array $columns = [], array $indexes = [], array $fkConstraints = [], $idGeneratorType = 0, array $options = [])`
+
+To the new constructor:
+
+`__construct($tableName, array $columns = [], array $indexes = [], array $uniqueConstraints = [], array $fkConstraints = [], array $options = [])`
+
 ## BC BREAK: Dropped support for `FetchMode::CUSTOM_OBJECT` and `::STANDARD_OBJECT`
 
 Instead of fetching an object, fetch an array and map it to an object of the desired class.

src/Platforms/AbstractPlatform.php

@@ -1565,7 +1565,6 @@ abstract class AbstractPlatform
             }
 
             foreach ($table->getUniqueConstraints() as $uniqueConstraint) {
-                /** @var UniqueConstraint $uniqueConstraint */
                 $options['uniqueConstraints'][$uniqueConstraint->getQuotedName($this)] = $uniqueConstraint;
             }
         }
@@ -1621,6 +1620,7 @@ abstract class AbstractPlatform
         }
 
         $sql = $this->_getCreateTableSQL($tableName, $columns, $options);
+
         if ($this->supportsCommentOnStatement()) {
             if ($table->hasOption('comment')) {
                 $sql[] = $this->getCommentOnTableSQL($tableName, $table->getOption('comment'));
@@ -1698,13 +1698,13 @@ abstract class AbstractPlatform
      *
      * @return string[]
      */
-    protected function _getCreateTableSQL($tableName, array $columns, array $options = [])
+    protected function _getCreateTableSQL($name, array $columns, array $options = [])
     {
         $columnListSql = $this->getColumnDeclarationListSQL($columns);
 
         if (isset($options['uniqueConstraints']) && ! empty($options['uniqueConstraints'])) {
-            foreach ($options['uniqueConstraints'] as $name => $definition) {
-                $columnListSql .= ', ' . $this->getUniqueConstraintDeclarationSQL($name, $definition);
+            foreach ($options['uniqueConstraints'] as $constraintName => $definition) {
+                $columnListSql .= ', ' . $this->getUniqueConstraintDeclarationSQL($constraintName, $definition);
             }
         }
 
@@ -1718,19 +1718,20 @@ abstract class AbstractPlatform
             }
         }
 
-        $query = 'CREATE TABLE ' . $tableName . ' (' . $columnListSql;
-
+        $query = 'CREATE TABLE ' . $name . ' (' . $columnListSql;
         $check = $this->getCheckDeclarationSQL($columns);
+
         if (! empty($check)) {
             $query .= ', ' . $check;
         }
+
         $query .= ')';
 
         $sql = [$query];
 
         if (isset($options['foreignKeys'])) {
             foreach ((array) $options['foreignKeys'] as $definition) {
-                $sql[] = $this->getCreateForeignKeySQL($definition, $tableName);
+                $sql[] = $this->getCreateForeignKeySQL($definition, $name);
             }
         }
 
@@ -2393,17 +2394,11 @@ abstract class AbstractPlatform
             throw new InvalidArgumentException("Incomplete definition. 'columns' required.");
         }
 
-        $flags = ['UNIQUE'];
-
-        if ($constraint->hasFlag('clustered')) {
-            $flags[] = 'CLUSTERED';
-        }
-
+        $constraintFlags = array_merge(['UNIQUE'], array_map('strtoupper', $constraint->getFlags()));
         $constraintName  = $name->getQuotedName($this);
-        $constraintName  = ! empty($constraintName) ? $constraintName . ' ' : '';
         $columnListNames = $this->getIndexFieldDeclarationListSQL($columns);
 
-        return sprintf('CONSTRAINT %s%s (%s)', $constraintName, implode(' ', $flags), $columnListNames);
+        return sprintf('CONSTRAINT %s %s (%s)', $constraintName, implode(' ', $constraintFlags), $columnListNames);
     }
 
     /**
@@ -3099,13 +3094,8 @@ abstract class AbstractPlatform
 
     /**
      * Gets the sequence name prefix based on table information.
-     *
-     * @param string      $tableName
-     * @param string|null $schemaName
-     *
-     * @return string
      */
-    public function getSequencePrefix($tableName, $schemaName = null)
+    public function getSequencePrefix(string $tableName, ?string $schemaName = null) : string
     {
         if ($schemaName === null) {
             return $tableName;

src/Platforms/SqlitePlatform.php

@@ -319,9 +319,9 @@ class SqlitePlatform extends AbstractPlatform
     /**
      * {@inheritDoc}
      */
-    protected function _getCreateTableSQL($tableName, array $columns, array $options = [])
+    protected function _getCreateTableSQL($name, array $columns, array $options = [])
     {
-        $tableName   = str_replace('.', '__', $tableName);
+        $name        = str_replace('.', '__', $name);
         $queryFields = $this->getColumnDeclarationListSQL($columns);
 
         if (isset($options['uniqueConstraints']) && ! empty($options['uniqueConstraints'])) {
@@ -345,7 +345,7 @@ class SqlitePlatform extends AbstractPlatform
             $tableComment = $this->getInlineTableCommentSQL($comment);
         }
 
-        $query = ['CREATE TABLE ' . $tableName . ' ' . $tableComment . '(' . $queryFields . ')'];
+        $query = ['CREATE TABLE ' . $name . ' ' . $tableComment . '(' . $queryFields . ')'];
 
         if (isset($options['alter']) && $options['alter'] === true) {
             return $query;
@@ -353,13 +353,13 @@ class SqlitePlatform extends AbstractPlatform
 
         if (isset($options['indexes']) && ! empty($options['indexes'])) {
             foreach ($options['indexes'] as $indexDef) {
-                $query[] = $this->getCreateIndexSQL($indexDef, $tableName);
+                $query[] = $this->getCreateIndexSQL($indexDef, $name);
             }
         }
 
         if (isset($options['unique']) && ! empty($options['unique'])) {
             foreach ($options['unique'] as $indexDef) {
-                $query[] = $this->getCreateIndexSQL($indexDef, $tableName);
+                $query[] = $this->getCreateIndexSQL($indexDef, $name);
             }
         }
 

src/Schema/Table.php

@@ -31,7 +31,7 @@ class Table extends AbstractAsset
     protected $_primaryKeyName = null;
 
     /** @var UniqueConstraint[] */
-    protected $_uniqueConstraints = [];
+    protected $uniqueConstraints = [];
 
     /** @var ForeignKeyConstraint[] */
     protected $_fkConstraints = [];
@@ -149,7 +149,7 @@ class Table extends AbstractAsset
      *
      * @return self
      */
-    public function addUniqueConstraint(array $columnNames, ?string $indexName = null, array $flags = [], array $options = [])
+    public function addUniqueConstraint(array $columnNames, ?string $indexName = null, array $flags = [], array $options = []) : Table
     {
         if ($indexName === null) {
             $indexName = $this->_generateIdentifierName(
@@ -504,56 +504,44 @@ class Table extends AbstractAsset
 
     /**
      * Returns whether this table has a unique constraint with the given name.
-     *
-     * @param string $constraintName
-     *
-     * @return bool
      */
-    public function hasUniqueConstraint($constraintName)
+    public function hasUniqueConstraint(string $name) : bool
     {
-        $constraintName = $this->normalizeIdentifier($constraintName);
+        $name = $this->normalizeIdentifier($name);
 
-        return isset($this->_uniqueConstraints[$constraintName]);
+        return isset($this->uniqueConstraints[$name]);
     }
 
     /**
      * Returns the unique constraint with the given name.
      *
-     * @param string $constraintName The constraint name.
-     *
-     * @return UniqueConstraint
-     *
      * @throws SchemaException If the unique constraint does not exist.
      */
-    public function getUniqueConstraint($constraintName)
+    public function getUniqueConstraint(string $name) : UniqueConstraint
     {
-        $constraintName = $this->normalizeIdentifier($constraintName);
+        $name = $this->normalizeIdentifier($name);
 
-        if (! $this->hasUniqueConstraint($constraintName)) {
-            throw SchemaException::uniqueConstraintDoesNotExist($constraintName, $this->_name);
+        if (! $this->hasUniqueConstraint($name)) {
+            throw SchemaException::uniqueConstraintDoesNotExist($name, $this->_name);
         }
 
-        return $this->_uniqueConstraints[$constraintName];
+        return $this->uniqueConstraints[$name];
     }
 
     /**
      * Removes the unique constraint with the given name.
      *
-     * @param string $constraintName The constraint name.
-     *
-     * @return void
-     *
-     * @throws SchemaException
+     * @throws SchemaException If the unique constraint does not exist.
      */
-    public function removeUniqueConstraint($constraintName)
+    public function removeUniqueConstraint(string $name) : void
     {
-        $constraintName = $this->normalizeIdentifier($constraintName);
+        $name = $this->normalizeIdentifier($name);
 
-        if (! $this->hasForeignKey($constraintName)) {
-            throw SchemaException::uniqueConstraintDoesNotExist($constraintName, $this->_name);
+        if (! $this->hasForeignKey($name)) {
+            throw SchemaException::uniqueConstraintDoesNotExist($name, $this->_name);
         }
 
-        unset($this->_uniqueConstraints[$constraintName]);
+        unset($this->uniqueConstraints[$name]);
     }
 
     /**
@@ -711,7 +699,7 @@ class Table extends AbstractAsset
      */
     public function getUniqueConstraints()
     {
-        return $this->_uniqueConstraints;
+        return $this->uniqueConstraints;
     }
 
     /**
@@ -876,7 +864,7 @@ class Table extends AbstractAsset
     /**
      * @return self
      */
-    protected function _addUniqueConstraint(UniqueConstraint $constraint)
+    protected function _addUniqueConstraint(UniqueConstraint $constraint) : Table
     {
         $mergedNames = array_merge([$this->getName()], $constraint->getColumns());
         $name        = strlen($constraint->getName()) > 0
@@ -885,7 +873,7 @@ class Table extends AbstractAsset
 
         $name = $this->normalizeIdentifier($name);
 
-        $this->_uniqueConstraints[$name] = $constraint;
+        $this->uniqueConstraints[$name] = $constraint;
 
         // If there is already an index that fulfills this requirements drop the request. In the case of __construct
         // calling this method during hydration from schema-details all the explicitly added indexes lead to duplicates.
@@ -984,7 +972,7 @@ class Table extends AbstractAsset
      *
      * @throws SchemaException
      */
-    private function _createUniqueConstraint(array $columnNames, string $indexName, array $flags = [], array $options = [])
+    private function _createUniqueConstraint(array $columnNames, string $indexName, array $flags = [], array $options = []) : UniqueConstraint
     {
         if (preg_match('(([^a-zA-Z0-9_]+))', $this->normalizeIdentifier($indexName)) === 1) {
             throw SchemaException::indexNameInvalid($indexName);

src/Schema/UniqueConstraint.php

@@ -36,14 +36,14 @@ class UniqueConstraint extends AbstractAsset implements Constraint
     private $options = [];
 
     /**
-     * @param string   $indexName
+     * @param string   $name
      * @param string[] $columns
      * @param string[] $flags
      * @param mixed[]  $options
      */
-    public function __construct($indexName, array $columns, array $flags = [], array $options = [])
+    public function __construct(string $name, array $columns, array $flags = [], array $options = [])
     {
-        $this->_setName($indexName);
+        $this->_setName($name);
 
         $this->options = $options;
 
@@ -81,7 +81,7 @@ class UniqueConstraint extends AbstractAsset implements Constraint
     /**
      * @return string[]
      */
-    public function getUnquotedColumns()
+    public function getUnquotedColumns() : array
     {
         return array_map([$this, 'trimQuotes'], $this->getColumns());
     }
@@ -91,7 +91,7 @@ class UniqueConstraint extends AbstractAsset implements Constraint
      *
      * @return string[]
      */
-    public function getFlags()
+    public function getFlags() : array
     {
         return array_keys($this->flags);
     }
@@ -105,7 +105,7 @@ class UniqueConstraint extends AbstractAsset implements Constraint
      *
      * @example $uniqueConstraint->addFlag('CLUSTERED')
      */
-    public function addFlag($flag)
+    public function addFlag(string $flag) : UniqueConstraint
     {
         $this->flags[strtolower($flag)] = true;
 
@@ -119,7 +119,7 @@ class UniqueConstraint extends AbstractAsset implements Constraint
      *
      * @return bool
      */
-    public function hasFlag($flag)
+    public function hasFlag(string $flag) : bool
     {
         return isset($this->flags[strtolower($flag)]);
     }
@@ -131,7 +131,7 @@ class UniqueConstraint extends AbstractAsset implements Constraint
      *
      * @return void
      */
-    public function removeFlag($flag)
+    public function removeFlag(string $flag) : void
     {
         unset($this->flags[strtolower($flag)]);
     }
@@ -141,17 +141,15 @@ class UniqueConstraint extends AbstractAsset implements Constraint
      *
      * @return bool
      */
-    public function hasOption($name)
+    public function hasOption(string $name) : bool
     {
         return isset($this->options[strtolower($name)]);
     }
 
     /**
-     * @param string $name
-     *
      * @return mixed
      */
-    public function getOption($name)
+    public function getOption(string $name) : mixed
     {
         return $this->options[strtolower($name)];
     }
@@ -159,7 +157,7 @@ class UniqueConstraint extends AbstractAsset implements Constraint
     /**
      * @return mixed[]
      */
-    public function getOptions()
+    public function getOptions() : array
     {
         return $this->options;
     }

tests/Platforms/SQLAnywhere16PlatformTest.php

@@ -394,13 +394,13 @@ class SQLAnywhere16PlatformTest extends AbstractPlatformTestCase
             'CONSTRAINT unique_constraint UNIQUE CLUSTERED (a, b)',
             $this->platform->getUniqueConstraintDeclarationSQL(
                 'unique_constraint',
-                new UniqueConstraint(null, ['a', 'b'], ['clustered'])
+                new UniqueConstraint('unique_constraint', ['a', 'b'], ['clustered'])
             )
         );
 
         self::assertEquals(
-            'CONSTRAINT UNIQUE (a, b)',
-            $this->platform->getUniqueConstraintDeclarationSQL(null, new UniqueConstraint(null, ['a', 'b']))
+            'CONSTRAINT foo UNIQUE (a, b)',
+            $this->platform->getUniqueConstraintDeclarationSQL('foo', new UniqueConstraint('foo', ['a', 'b']))
         );
     }