From 185bdd3182d943924eeab3330ceb13c7846205c8 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Wed, 4 May 2022 20:01:36 -0700 Subject: [PATCH] Add runtime deprecations for default string column length --- UPGRADE.md | 5 +++++ phpstan.neon.dist | 7 +++++++ psalm.xml.dist | 5 +++++ src/Platforms/AbstractMySQLPlatform.php | 24 +++++++++++++++++++++-- src/Platforms/AbstractPlatform.php | 26 ++++++++++++------------- src/Platforms/DB2Platform.php | 24 ++++++++++++++++++++--- src/Platforms/OraclePlatform.php | 24 ++++++++++++++++++++--- src/Platforms/SQLServerPlatform.php | 24 +++++++++++++++++++++-- 8 files changed, 116 insertions(+), 23 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 46d995786c1..83f1658789c 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -8,6 +8,11 @@ awareness about deprecated code. # Upgrade to 3.4 +## Added runtime deprecations for the default string column length. + +In addition to the formal deprecation introduced in DBAL 3.2, the library will now emit a deprecation message at runtime +if the string or binary column length is omitted, but it's required by the target database platform. + ## Deprecated `AbstractPlatform::getVarcharTypeDeclarationSQL()` The `AbstractPlatform::getVarcharTypeDeclarationSQL()` method has been deprecated. diff --git a/phpstan.neon.dist b/phpstan.neon.dist index d3e8ca6bed3..08c5249da59 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -134,5 +134,12 @@ parameters: paths: - src/Query/QueryBuilder.php + # https://github.com/doctrine/dbal/issues/3263 + # TODO: remove in 4.0.0 + - + message: '~^Method Doctrine\\DBAL\\Platforms\\AbstractPlatform::get(Binary|Varchar)TypeDeclarationSQLSnippet\(\) invoked with 3 parameters, 2 required\.$~' + paths: + - src/Platforms/AbstractPlatform.php + includes: - vendor/phpstan/phpstan-strict-rules/rules.neon diff --git a/psalm.xml.dist b/psalm.xml.dist index bd48125fa01..b2d36b3c1d2 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -474,6 +474,11 @@ + + diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index c9819a2769b..35faf1d850b 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -19,7 +19,9 @@ use function array_unique; use function array_values; use function count; +use function func_get_arg; use function func_get_args; +use function func_num_args; use function implode; use function in_array; use function is_numeric; @@ -203,8 +205,17 @@ public function getListTableForeignKeysSQL($table, $database = null) /** * {@inheritDoc} */ - protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed) + protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed/*, $lengthOmitted = false*/) { + if ($length <= 0 || (func_num_args() > 2 && func_get_arg(2))) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/issues/3263', + 'Relying on the default string column length on MySQL is deprecated' + . ', specify the length explicitly.' + ); + } + return $fixed ? ($length > 0 ? 'CHAR(' . $length . ')' : 'CHAR(255)') : ($length > 0 ? 'VARCHAR(' . $length . ')' : 'VARCHAR(255)'); } @@ -212,8 +223,17 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed) /** * {@inheritdoc} */ - protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed) + protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed/*, $lengthOmitted = false*/) { + if ($length <= 0 || (func_num_args() > 2 && func_get_arg(2))) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/issues/3263', + 'Relying on the default binary column length on MySQL is deprecated' + . ', specify the length explicitly.' + ); + } + return $fixed ? 'BINARY(' . ($length > 0 ? $length : 255) . ')' : 'VARBINARY(' . ($length > 0 ? $length : 255) . ')'; diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index 59c73121bba..33dcfa764b8 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -205,8 +205,11 @@ public function getAsciiStringTypeDeclarationSQL(array $column): string */ public function getVarcharTypeDeclarationSQL(array $column) { - if (! isset($column['length'])) { + if (isset($column['length'])) { + $lengthOmitted = false; + } else { $column['length'] = $this->getVarcharDefaultLength(); + $lengthOmitted = true; } $fixed = $column['fixed'] ?? false; @@ -219,7 +222,7 @@ public function getVarcharTypeDeclarationSQL(array $column) return $this->getClobTypeDeclarationSQL($column); } - return $this->getVarcharTypeDeclarationSQLSnippet($column['length'], $fixed); + return $this->getVarcharTypeDeclarationSQLSnippet($column['length'], $fixed, $lengthOmitted); } /** @@ -243,8 +246,11 @@ public function getStringTypeDeclarationSQL(array $column) */ public function getBinaryTypeDeclarationSQL(array $column) { - if (! isset($column['length'])) { + if (isset($column['length'])) { + $lengthOmitted = false; + } else { $column['length'] = $this->getBinaryDefaultLength(); + $lengthOmitted = true; } $fixed = $column['fixed'] ?? false; @@ -266,7 +272,7 @@ public function getBinaryTypeDeclarationSQL(array $column) return $this->getBlobTypeDeclarationSQL($column); } - return $this->getBinaryTypeDeclarationSQLSnippet($column['length'], $fixed); + return $this->getBinaryTypeDeclarationSQLSnippet($column['length'], $fixed, $lengthOmitted); } /** @@ -310,7 +316,7 @@ public function getJsonTypeDeclarationSQL(array $column) * * @throws Exception If not supported on this platform. */ - protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed) + protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed/*, $lengthOmitted = false*/) { throw Exception::notSupported('VARCHARs not supported by Platform.'); } @@ -325,7 +331,7 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed) * * @throws Exception If not supported on this platform. */ - protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed) + protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed/*, $lengthOmitted = false*/) { throw Exception::notSupported('BINARY/VARBINARY column types are not supported by this platform.'); } @@ -4027,17 +4033,11 @@ private function columnToArray(Column $column): array { $name = $column->getQuotedName($this); - $columnData = array_merge($column->toArray(), [ + return array_merge($column->toArray(), [ 'name' => $name, 'version' => $column->hasPlatformOption('version') ? $column->getPlatformOption('version') : false, 'comment' => $this->getColumnComment($column), ]); - - if ($columnData['type'] instanceof Types\StringType && $columnData['length'] === null) { - $columnData['length'] = $this->getVarcharDefaultLength(); - } - - return $columnData; } /** diff --git a/src/Platforms/DB2Platform.php b/src/Platforms/DB2Platform.php index 5b90d475d9f..640a5e6d886 100644 --- a/src/Platforms/DB2Platform.php +++ b/src/Platforms/DB2Platform.php @@ -114,8 +114,17 @@ public function isCommentedDoctrineType(Type $doctrineType) /** * {@inheritDoc} */ - protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed) - { + protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed/*, $lengthOmitted = false*/) + { + if ($length <= 0 || (func_num_args() > 2 && func_get_arg(2))) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/issues/3263', + 'Relying on the default string column length on IBM DB2 is deprecated' + . ', specify the length explicitly.' + ); + } + return $fixed ? ($length > 0 ? 'CHAR(' . $length . ')' : 'CHAR(254)') : ($length > 0 ? 'VARCHAR(' . $length . ')' : 'VARCHAR(255)'); } @@ -123,8 +132,17 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed) /** * {@inheritdoc} */ - protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed) + protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed/*, $lengthOmitted = false*/) { + if ($length <= 0 || (func_num_args() > 2 && func_get_arg(2))) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/issues/3263', + 'Relying on the default binary column length on IBM DB2 is deprecated' + . ', specify the length explicitly.' + ); + } + return $this->getVarcharTypeDeclarationSQLSnippet($length, $fixed) . ' FOR BIT DATA'; } diff --git a/src/Platforms/OraclePlatform.php b/src/Platforms/OraclePlatform.php index 32bce016776..bfc510d7b34 100644 --- a/src/Platforms/OraclePlatform.php +++ b/src/Platforms/OraclePlatform.php @@ -348,8 +348,17 @@ protected function _getCommonIntegerTypeDeclarationSQL(array $column) /** * {@inheritDoc} */ - protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed) - { + protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed/*, $lengthOmitted = false*/) + { + if ($length <= 0 || (func_num_args() > 2 && func_get_arg(2))) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/issues/3263', + 'Relying on the default string column length on Oracle is deprecated' + . ', specify the length explicitly.' + ); + } + return $fixed ? ($length > 0 ? 'CHAR(' . $length . ')' : 'CHAR(2000)') : ($length > 0 ? 'VARCHAR2(' . $length . ')' : 'VARCHAR2(4000)'); } @@ -357,8 +366,17 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed) /** * {@inheritdoc} */ - protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed) + protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed/*, $lengthOmitted = false*/) { + if ($length <= 0 || (func_num_args() > 2 && func_get_arg(2))) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/issues/3263', + 'Relying on the default binary column length on Oracle is deprecated' + . ', specify the length explicitly.' + ); + } + return 'RAW(' . ($length > 0 ? $length : $this->getBinaryMaxLength()) . ')'; } diff --git a/src/Platforms/SQLServerPlatform.php b/src/Platforms/SQLServerPlatform.php index 7a27a61f458..25bc6a2d7e3 100644 --- a/src/Platforms/SQLServerPlatform.php +++ b/src/Platforms/SQLServerPlatform.php @@ -22,7 +22,9 @@ use function crc32; use function dechex; use function explode; +use function func_get_arg; use function func_get_args; +use function func_num_args; use function implode; use function is_array; use function is_bool; @@ -1203,8 +1205,17 @@ public function getAsciiStringTypeDeclarationSQL(array $column): string /** * {@inheritDoc} */ - protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed) + protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed/*, $lengthOmitted = false*/) { + if ($length <= 0 || (func_num_args() > 2 && func_get_arg(2))) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/issues/3263', + 'Relying on the default string column length on SQL Server is deprecated' + . ', specify the length explicitly.' + ); + } + return $fixed ? ($length > 0 ? 'NCHAR(' . $length . ')' : 'CHAR(255)') : ($length > 0 ? 'NVARCHAR(' . $length . ')' : 'NVARCHAR(255)'); @@ -1213,8 +1224,17 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed) /** * {@inheritdoc} */ - protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed) + protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed/*, $lengthOmitted = false*/) { + if ($length <= 0 || (func_num_args() > 2 && func_get_arg(2))) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/issues/3263', + 'Relying on the default binary column length on SQL Server is deprecated' + . ', specify the length explicitly.' + ); + } + return $fixed ? 'BINARY(' . ($length > 0 ? $length : 255) . ')' : 'VARBINARY(' . ($length > 0 ? $length : 255) . ')';