From d9651281360dd1cfd6e339efa056d4558eeb631d Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 7 Mar 2025 16:46:35 -0800 Subject: [PATCH 1/7] Deprecate modified foreign keys in TableDiff --- UPGRADE.md | 9 ++++++++ src/Schema/Comparator.php | 23 ++++++++++---------- src/Schema/SQLiteSchemaManager.php | 2 +- src/Schema/TableDiff.php | 24 ++++++++++++++++++++- tests/Schema/AbstractComparatorTestCase.php | 6 ++++-- 5 files changed, 48 insertions(+), 16 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index e23815c562..fabbf57ec4 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -8,6 +8,15 @@ awareness about deprecated code. # Upgrade to 4.3 +## Deprecated handling of modified foreign keys in `TableDiff` + +Passing a non-empty `$modifiedForeignKeys` value to the `TableDiff` constructor is deprecated. Instead, pass dropped +constraints via `$droppedForeignKeys` and added constraints via `$addedForeignKeys`. + +The `TableDiff::getModifiedForeignKeys()` method has been deprecated. The old version of the foreign key constraint is +included in the return value of `TableDiff::getDroppedForeignKeys()`, the new version is included in the return value of +`TableDiff::getModifiedForeignKeys()`. + ## Deprecated not passing `$options` to `AbstractPlatform::_getCreateTableSQL()` Not passing the `$options` argument or any of its following keys to the `AbstractPlatform::_getCreateTableSQL()` method diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php index a2b18b802c..536ea860f4 100644 --- a/src/Schema/Comparator.php +++ b/src/Schema/Comparator.php @@ -145,16 +145,15 @@ public function diffSequence(Sequence $sequence1, Sequence $sequence2): bool */ public function compareTables(Table $oldTable, Table $newTable): TableDiff { - $addedColumns = []; - $modifiedColumns = []; - $droppedColumns = []; - $addedIndexes = []; - $modifiedIndexes = []; - $droppedIndexes = []; - $renamedIndexes = []; - $addedForeignKeys = []; - $modifiedForeignKeys = []; - $droppedForeignKeys = []; + $addedColumns = []; + $modifiedColumns = []; + $droppedColumns = []; + $addedIndexes = []; + $modifiedIndexes = []; + $droppedIndexes = []; + $renamedIndexes = []; + $addedForeignKeys = []; + $droppedForeignKeys = []; $oldColumns = $oldTable->getColumns(); $newColumns = $newTable->getColumns(); @@ -262,7 +261,8 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff unset($oldForeignKeys[$oldKey], $newForeignKeys[$newKey]); } else { if (strtolower($oldForeignKey->getName()) === strtolower($newForeignKey->getName())) { - $modifiedForeignKeys[] = $newForeignKey; + $droppedForeignKeys[$oldKey] = $oldForeignKey; + $addedForeignKeys[$newKey] = $newForeignKey; unset($oldForeignKeys[$oldKey], $newForeignKeys[$newKey]); } @@ -288,7 +288,6 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff droppedIndexes: $droppedIndexes, renamedIndexes: $renamedIndexes, addedForeignKeys: $addedForeignKeys, - modifiedForeignKeys: $modifiedForeignKeys, droppedForeignKeys: $droppedForeignKeys, ); } diff --git a/src/Schema/SQLiteSchemaManager.php b/src/Schema/SQLiteSchemaManager.php index 50db9e4975..9d130e5296 100644 --- a/src/Schema/SQLiteSchemaManager.php +++ b/src/Schema/SQLiteSchemaManager.php @@ -64,7 +64,7 @@ public function createForeignKey(ForeignKeyConstraint $foreignKey, string $table { $table = $this->introspectTable($table); - $this->alterTable(new TableDiff($table, modifiedForeignKeys: [$foreignKey])); + $this->alterTable(new TableDiff($table, addedForeignKeys: [$foreignKey])); } public function dropForeignKey(string $name, string $table): void diff --git a/src/Schema/TableDiff.php b/src/Schema/TableDiff.php index 2bb514bc36..154bf0cb24 100644 --- a/src/Schema/TableDiff.php +++ b/src/Schema/TableDiff.php @@ -44,6 +44,17 @@ public function __construct( private readonly array $modifiedForeignKeys = [], private readonly array $droppedForeignKeys = [], ) { + if (count($modifiedForeignKeys) === 0) { + return; + } + + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6827', + 'Passing a non-empty $modifiedForeignKeys value to %s() is deprecated. Instead, pass dropped' + . ' constraints via $droppedForeignKeys and added constraints via $addedForeignKeys.', + __METHOD__, + ); } public function getOldTable(): Table @@ -173,9 +184,20 @@ public function getAddedForeignKeys(): array return $this->addedForeignKeys; } - /** @return array */ + /** + * @deprecated Use {@see getAddedForeignKeys()} and {@see getDroppedForeignKeys()} instead. + * + * @return array + */ public function getModifiedForeignKeys(): array { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6827', + '%s() is deprecated, use getDroppedForeignKeys() and getAddedForeignKeys() instead.', + __METHOD__, + ); + return $this->modifiedForeignKeys; } diff --git a/tests/Schema/AbstractComparatorTestCase.php b/tests/Schema/AbstractComparatorTestCase.php index d690307dbf..a6dd7ee6a7 100644 --- a/tests/Schema/AbstractComparatorTestCase.php +++ b/tests/Schema/AbstractComparatorTestCase.php @@ -279,7 +279,8 @@ public function testTableUpdateForeignKey(): void $tableDiff = $this->comparator->compareTables($table1, $table2); - self::assertCount(1, $tableDiff->getModifiedForeignKeys()); + self::assertCount(1, $tableDiff->getDroppedForeignKeys()); + self::assertCount(1, $tableDiff->getAddedForeignKeys()); } public function testMovedForeignKeyForeignTable(): void @@ -300,7 +301,8 @@ public function testMovedForeignKeyForeignTable(): void $tableDiff = $this->comparator->compareTables($table1, $table2); - self::assertCount(1, $tableDiff->getModifiedForeignKeys()); + self::assertCount(1, $tableDiff->getDroppedForeignKeys()); + self::assertCount(1, $tableDiff->getAddedForeignKeys()); } public function testTablesCaseInsensitive(): void From 0a01fc63cdeeed87271aa3ff058754153edac1eb Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sun, 9 Mar 2025 09:16:55 -0700 Subject: [PATCH 2/7] Fix dropping introspected primary index on Postgres --- src/Platforms/PostgreSQLPlatform.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Platforms/PostgreSQLPlatform.php b/src/Platforms/PostgreSQLPlatform.php index 78ee55bb9c..5e8b9d334a 100644 --- a/src/Platforms/PostgreSQLPlatform.php +++ b/src/Platforms/PostgreSQLPlatform.php @@ -364,14 +364,14 @@ public function getDropForeignKeySQL(string $foreignKey, string $table): string public function getDropIndexSQL(string $name, string $table): string { - if ($name === '"primary"') { - if (str_ends_with($table, '"')) { - $constraintName = substr($table, 0, -1) . '_pkey"'; - } else { - $constraintName = $table . '_pkey'; - } + if (str_ends_with($table, '"')) { + $primaryKeyName = substr($table, 0, -1) . '_pkey"'; + } else { + $primaryKeyName = $table . '_pkey'; + } - return $this->getDropConstraintSQL($constraintName, $table); + if ($name === '"primary"' || $name === $primaryKeyName) { + return $this->getDropConstraintSQL($primaryKeyName, $table); } if (str_contains($table, '.')) { From f33f98675be866e6c025a5e5ec7948db7e99575a Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sun, 9 Mar 2025 01:04:57 -0800 Subject: [PATCH 3/7] Deprecate modified indexes in TableDiff --- UPGRADE.md | 11 ++++++++++- src/Platforms/AbstractMySQLPlatform.php | 9 +-------- src/Schema/Comparator.php | 5 ++--- src/Schema/TableDiff.php | 23 ++++++++++++++++++++++- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index fabbf57ec4..603c94e2e8 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -8,6 +8,15 @@ awareness about deprecated code. # Upgrade to 4.3 +## Deprecated handling of modified indexes in `TableDiff` + +Passing a non-empty `$modifiedIndexes` value to the `TableDiff` constructor is deprecated. Instead, pass dropped +indexes via `$droppedIndexes` and added indexes via `$addedIndexes`. + +The `TableDiff::getModifiedIndexes()` method has been deprecated. The old version of the index is included in the return +value of `TableDiff::getDroppedIndexes()`, the new version is included in the return value of +`TableDiff::getAddedIndexes()`. + ## Deprecated handling of modified foreign keys in `TableDiff` Passing a non-empty `$modifiedForeignKeys` value to the `TableDiff` constructor is deprecated. Instead, pass dropped @@ -15,7 +24,7 @@ constraints via `$droppedForeignKeys` and added constraints via `$addedForeignKe The `TableDiff::getModifiedForeignKeys()` method has been deprecated. The old version of the foreign key constraint is included in the return value of `TableDiff::getDroppedForeignKeys()`, the new version is included in the return value of -`TableDiff::getModifiedForeignKeys()`. +`TableDiff::getAddedForeignKeys()`. ## Deprecated not passing `$options` to `AbstractPlatform::_getCreateTableSQL()` diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index 97be4022ff..a73d7dd9a1 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -5,7 +5,6 @@ namespace Doctrine\DBAL\Platforms; use Doctrine\DBAL\Connection; -use Doctrine\DBAL\Exception; use Doctrine\DBAL\Exception\InvalidColumnType\ColumnValuesRequired; use Doctrine\DBAL\Platforms\Keywords\KeywordList; use Doctrine\DBAL\Platforms\Keywords\MySQLKeywords; @@ -481,11 +480,7 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array ); } - /** - * @return list - * - * @throws Exception - */ + /** @return list */ private function getPreAlterTableAlterPrimaryKeySQL(TableDiff $diff, Index $index): array { if (! $index->isPrimary()) { @@ -526,8 +521,6 @@ private function getPreAlterTableAlterPrimaryKeySQL(TableDiff $diff, Index $inde * @param TableDiff $diff The table diff to gather the SQL for. * * @return list - * - * @throws Exception */ private function getPreAlterTableAlterIndexForeignKeySQL(TableDiff $diff): array { diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php index 536ea860f4..1f47e7ba1f 100644 --- a/src/Schema/Comparator.php +++ b/src/Schema/Comparator.php @@ -149,7 +149,6 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff $modifiedColumns = []; $droppedColumns = []; $addedIndexes = []; - $modifiedIndexes = []; $droppedIndexes = []; $renamedIndexes = []; $addedForeignKeys = []; @@ -245,7 +244,8 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff continue; } - $modifiedIndexes[] = $newIndex; + $droppedIndexes[$oldIndexName] = $oldIndex; + $addedIndexes[$oldIndexName] = $newIndex; } if ($this->config->getDetectRenamedIndexes()) { @@ -284,7 +284,6 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff changedColumns: $modifiedColumns, droppedColumns: $droppedColumns, addedIndexes: $addedIndexes, - modifiedIndexes: $modifiedIndexes, droppedIndexes: $droppedIndexes, renamedIndexes: $renamedIndexes, addedForeignKeys: $addedForeignKeys, diff --git a/src/Schema/TableDiff.php b/src/Schema/TableDiff.php index 154bf0cb24..45a7394686 100644 --- a/src/Schema/TableDiff.php +++ b/src/Schema/TableDiff.php @@ -44,6 +44,16 @@ public function __construct( private readonly array $modifiedForeignKeys = [], private readonly array $droppedForeignKeys = [], ) { + if (count($this->modifiedIndexes) !== 0) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6831', + 'Passing a non-empty $modifiedIndexes value to %s() is deprecated. Instead, pass dropped' + . ' indexes via $droppedIndexes and added indexes via $addedIndexes.', + __METHOD__, + ); + } + if (count($modifiedForeignKeys) === 0) { return; } @@ -146,9 +156,20 @@ static function (Index $addedIndex) use ($index): bool { ); } - /** @return array */ + /** + * @deprecated Use {@see getAddedIndexes()} and {@see getDroppedIndexes()} instead. + * + * @return array + */ public function getModifiedIndexes(): array { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6831', + '%s() is deprecated, use getAddedIndexes() and getDroppedIndexes() instead.', + __METHOD__, + ); + return $this->modifiedIndexes; } From 724c452a6a983e44d1ffefb3ee3eda8f2270a10c Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sun, 9 Mar 2025 13:00:51 -0700 Subject: [PATCH 4/7] Annotate methods that throw TypesException --- src/Platforms/AbstractPlatform.php | 7 +++++++ src/Schema/AbstractSchemaManager.php | 5 +++++ src/Schema/Table.php | 7 ++++++- src/Types/Type.php | 28 ++++++++++++++++------------ src/Types/TypeRegistry.php | 13 +++++++++---- 5 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index a379a72250..64a8889020 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -37,6 +37,7 @@ use Doctrine\DBAL\TransactionIsolationLevel; use Doctrine\DBAL\Types; use Doctrine\DBAL\Types\Exception\TypeNotFound; +use Doctrine\DBAL\Types\Exception\TypesException; use Doctrine\DBAL\Types\Type; use Doctrine\Deprecations\Deprecation; @@ -161,6 +162,8 @@ abstract protected function initializeDoctrineTypeMappings(): void; /** * Initializes Doctrine Type Mappings with the platform defaults * and with all additional type mappings. + * + * @throws TypesException */ private function initializeAllDoctrineTypeMappings(): void { @@ -372,6 +375,8 @@ public function registerDoctrineTypeMapping(string $dbType, string $doctrineType /** * Gets the Doctrine type that is mapped for the given database column type. + * + * @throws TypesException */ public function getDoctrineTypeMapping(string $dbType): string { @@ -394,6 +399,8 @@ public function getDoctrineTypeMapping(string $dbType): string /** * Checks if a database type is currently supported by this platform. + * + * @throws TypesException */ public function hasDoctrineTypeMappingFor(string $dbType): bool { diff --git a/src/Schema/AbstractSchemaManager.php b/src/Schema/AbstractSchemaManager.php index ab890718ff..7f4aaaa2ba 100644 --- a/src/Schema/AbstractSchemaManager.php +++ b/src/Schema/AbstractSchemaManager.php @@ -12,6 +12,7 @@ use Doctrine\DBAL\Result; use Doctrine\DBAL\Schema\Exception\TableDoesNotExist; use Doctrine\DBAL\Schema\Name\Parsers; +use Doctrine\DBAL\Types\Exception\TypesException; use Doctrine\Deprecations\Deprecation; use Throwable; @@ -795,6 +796,8 @@ protected function _getPortableSequenceDefinition(array $sequence): Sequence * @param array> $rows * * @return array + * + * @throws TypesException */ protected function _getPortableTableColumnList(string $table, string $database, array $rows): array { @@ -813,6 +816,8 @@ protected function _getPortableTableColumnList(string $table, string $database, * Gets Table Column Definition. * * @param array $tableColumn + * + * @throws TypesException */ abstract protected function _getPortableTableColumnDefinition(array $tableColumn): Column; diff --git a/src/Schema/Table.php b/src/Schema/Table.php index 4036d8b30d..f1b7be8ea4 100644 --- a/src/Schema/Table.php +++ b/src/Schema/Table.php @@ -16,6 +16,7 @@ use Doctrine\DBAL\Schema\Name\OptionallyQualifiedName; use Doctrine\DBAL\Schema\Name\Parser\OptionallyQualifiedNameParser; use Doctrine\DBAL\Schema\Name\Parsers; +use Doctrine\DBAL\Types\Exception\TypesException; use Doctrine\DBAL\Types\Type; use Doctrine\Deprecations\Deprecation; use LogicException; @@ -314,7 +315,11 @@ public function columnsAreIndexed(array $columnNames): bool return false; } - /** @param array $options */ + /** + * @param array $options + * + * @throws TypesException + */ public function addColumn(string $name, string $typeName, array $options = []): Column { $column = new Column($name, Type::getType($typeName), $options); diff --git a/src/Types/Type.php b/src/Types/Type.php index cbb0bb058b..e367536c22 100644 --- a/src/Types/Type.php +++ b/src/Types/Type.php @@ -9,6 +9,7 @@ use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Types\Exception\TypeArgumentCountError; +use Doctrine\DBAL\Types\Exception\TypesException; use function array_map; use function is_string; @@ -94,20 +95,21 @@ public function convertToPHPValue(mixed $value, AbstractPlatform $platform): mix */ abstract public function getSQLDeclaration(array $column, AbstractPlatform $platform): string; + /** @throws TypesException */ final public static function getTypeRegistry(): TypeRegistry { return self::$typeRegistry ??= self::createTypeRegistry(); } + /** @throws TypesException */ private static function createTypeRegistry(): TypeRegistry { - $instances = []; - - foreach (self::BUILTIN_TYPES_MAP as $name => $class) { - $instances[$name] = new $class(); - } - - return new TypeRegistry($instances); + return new TypeRegistry( + array_map( + static fn ($class) => new $class(), + self::BUILTIN_TYPES_MAP, + ), + ); } /** @@ -115,7 +117,7 @@ private static function createTypeRegistry(): TypeRegistry * * @param string $name The name of the type. * - * @throws Exception + * @throws TypesException */ public static function getType(string $name): self { @@ -125,7 +127,7 @@ public static function getType(string $name): self /** * Finds a name for the given type. * - * @throws Exception + * @throws TypesException */ public static function lookupName(self $type): string { @@ -159,6 +161,8 @@ public static function addType(string $name, string|Type $type): void * @param string $name The name of the type. * * @return bool TRUE if type is supported; FALSE otherwise. + * + * @throws TypesException */ public static function hasType(string $name): bool { @@ -199,13 +203,13 @@ public function getBindingType(): ParameterType * type class * * @return array + * + * @throws TypesException */ public static function getTypesMap(): array { return array_map( - static function (Type $type): string { - return $type::class; - }, + static fn (Type $type): string => $type::class, self::getTypeRegistry()->getMap(), ); } diff --git a/src/Types/TypeRegistry.php b/src/Types/TypeRegistry.php index 5ef36c3932..fce4ab0b27 100644 --- a/src/Types/TypeRegistry.php +++ b/src/Types/TypeRegistry.php @@ -9,6 +9,7 @@ use Doctrine\DBAL\Types\Exception\TypeNotFound; use Doctrine\DBAL\Types\Exception\TypeNotRegistered; use Doctrine\DBAL\Types\Exception\TypesAlreadyExists; +use Doctrine\DBAL\Types\Exception\TypesException; use Doctrine\DBAL\Types\Exception\UnknownColumnType; use function spl_object_id; @@ -23,7 +24,11 @@ final class TypeRegistry /** @var array */ private array $instancesReverseIndex; - /** @param array $instances */ + /** + * @param array $instances + * + * @throws TypesException + */ public function __construct(array $instances = []) { $this->instances = []; @@ -36,7 +41,7 @@ public function __construct(array $instances = []) /** * Finds a type by the given name. * - * @throws Exception + * @throws TypesException */ public function get(string $name): Type { @@ -51,7 +56,7 @@ public function get(string $name): Type /** * Finds a name for the given type. * - * @throws Exception + * @throws TypesException */ public function lookupName(Type $type): string { @@ -75,7 +80,7 @@ public function has(string $name): bool /** * Registers a custom type to the type map. * - * @throws Exception + * @throws TypesException */ public function register(string $name, Type $type): void { From 8b95f4f5fbcb871c5492c926c33d53d410c9136b Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sun, 9 Mar 2025 13:15:36 -0700 Subject: [PATCH 5/7] Convert driver-level parameter binding exceptions --- src/Connection.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Connection.php b/src/Connection.php index cc0af68248..7ef010f4c4 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -1314,7 +1314,11 @@ private function bindParameters(DriverStatement $stmt, array $params, array $typ $bindingType = ParameterType::STRING; } - $stmt->bindValue($bindIndex, $value, $bindingType); + try { + $stmt->bindValue($bindIndex, $value, $bindingType); + } catch (Driver\Exception $e) { + throw $this->convertException($e); + } ++$bindIndex; } @@ -1328,7 +1332,11 @@ private function bindParameters(DriverStatement $stmt, array $params, array $typ $bindingType = ParameterType::STRING; } - $stmt->bindValue($name, $value, $bindingType); + try { + $stmt->bindValue($name, $value, $bindingType); + } catch (Driver\Exception $e) { + throw $this->convertException($e); + } } } } From 45af77734e2cb2fd8b47aa1fde03e5fa640d2a19 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sun, 9 Mar 2025 13:16:38 -0700 Subject: [PATCH 6/7] Convert parser exception --- src/Connection.php | 9 ++++++++- src/Exception/ParseError.php | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 src/Exception/ParseError.php diff --git a/src/Connection.php b/src/Connection.php index 7ef010f4c4..9f74bed18a 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -20,6 +20,7 @@ use Doctrine\DBAL\Exception\DriverException; use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException; use Doctrine\DBAL\Exception\NoActiveTransaction; +use Doctrine\DBAL\Exception\ParseError; use Doctrine\DBAL\Exception\SavepointsNotSupported; use Doctrine\DBAL\Exception\TransactionRolledBack; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; @@ -1405,6 +1406,8 @@ final public function convertException(Driver\Exception $e): DriverException * list|array, * array, string|ParameterType|Type>|array * } + * + * @throws Exception */ private function expandArrayParameters(string $sql, array $params, array $types): array { @@ -1431,7 +1434,11 @@ private function expandArrayParameters(string $sql, array $params, array $types) $this->parser ??= $this->getDatabasePlatform()->createSQLParser(); $visitor = new ExpandArrayParameters($params, $types); - $this->parser->parse($sql, $visitor); + try { + $this->parser->parse($sql, $visitor); + } catch (Parser\Exception $e) { + throw ParseError::fromParserException($e); + } return [ $visitor->getSQL(), diff --git a/src/Exception/ParseError.php b/src/Exception/ParseError.php new file mode 100644 index 0000000000..76020599e3 --- /dev/null +++ b/src/Exception/ParseError.php @@ -0,0 +1,17 @@ + Date: Wed, 19 Oct 2022 12:48:20 -0700 Subject: [PATCH 7/7] Enable PHPStan checks for exceptions --- phpstan.neon.dist | 14 ++++++++++++++ src/Connection.php | 9 +++++++-- src/Connections/PrimaryReadReplicaConnection.php | 5 +++++ src/Driver.php | 3 +++ src/Driver/PDO/Result.php | 1 + src/Driver/PgSQL/Connection.php | 2 ++ src/Platforms/AbstractMySQLPlatform.php | 9 +-------- src/Platforms/SQLitePlatform.php | 9 +-------- src/Portability/Driver.php | 5 +++++ src/Query/CommonTableExpression.php | 6 +++++- src/Query/Expression/ExpressionBuilder.php | 3 +++ src/Query/QueryBuilder.php | 8 ++++++++ src/Schema/AbstractSchemaManager.php | 1 + src/Schema/ForeignKeyConstraint.php | 10 ++++++++++ src/Schema/PostgreSQLSchemaManager.php | 2 ++ src/Schema/SQLServerSchemaManager.php | 2 ++ src/Schema/SQLiteSchemaManager.php | 1 + 17 files changed, 71 insertions(+), 19 deletions(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 27b53826df..e83e1be4d5 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -7,9 +7,23 @@ parameters: - tests treatPhpDocTypesAsCertain: false reportUnmatchedIgnoredErrors: false + exceptions: + check: + missingCheckedExceptionInThrows: true + uncheckedExceptionClasses: + - ErrorException + - LogicException + - RuntimeException ignoreErrors: - identifier: missingType.generics + # Ignore missing declarations of checked exceptions in tests + - + identifier: missingType.checkedException + paths: + - static-analysis + - tests + # https://github.com/phpstan/phpstan-strict-rules/issues/103 - message: '~^Construct empty\(\) is not allowed. Use more strict comparison\.~' diff --git a/src/Connection.php b/src/Connection.php index 9f74bed18a..72303714cf 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -269,8 +269,7 @@ public function isAutoCommit(): bool * * @see isAutoCommit * - * @throws ConnectionException - * @throws DriverException + * @throws Exception */ public function setAutoCommit(bool $autoCommit): void { @@ -558,6 +557,8 @@ private function extractTypeValues(array $columns, array $types): array * @param string $identifier The identifier to be quoted. * * @return string The quoted identifier. + * + * @throws Exception */ public function quoteIdentifier(string $identifier): string { @@ -587,6 +588,8 @@ public function quoteSingleIdentifier(string $identifier): string /** * The usage of this method is discouraged. Use prepared statements * or {@see AbstractPlatform::quoteStringLiteral()} instead. + * + * @throws Exception */ public function quote(string $value): string { @@ -833,6 +836,7 @@ public function executeCacheQuery(string $sql, array $params, array $types, Quer [$cacheKey, $realKey] = $qcp->generateCacheKeys($sql, $params, $types, $connectionParams); + // @phpstan-ignore missingType.checkedException $item = $resultCache->getItem($cacheKey); if ($item->isHit()) { @@ -1088,6 +1092,7 @@ public function commit(): void } } + /** @throws Exception */ private function updateTransactionStateAfterCommit(): void { if ($this->transactionNestingLevel !== 0) { diff --git a/src/Connections/PrimaryReadReplicaConnection.php b/src/Connections/PrimaryReadReplicaConnection.php index d929e7112d..14b4632c57 100644 --- a/src/Connections/PrimaryReadReplicaConnection.php +++ b/src/Connections/PrimaryReadReplicaConnection.php @@ -141,6 +141,7 @@ public function connect(?string $connectionName = null): DriverConnection return $this->performConnect(); } + /** @throws Exception */ protected function performConnect(?string $connectionName = null): DriverConnection { $requestedConnectionChange = ($connectionName !== null); @@ -192,6 +193,8 @@ protected function performConnect(?string $connectionName = null): DriverConnect * Connects to the primary node of the database cluster. * * All following statements after this will be executed against the primary node. + * + * @throws Exception */ public function ensureConnectedToPrimary(): void { @@ -204,6 +207,8 @@ public function ensureConnectedToPrimary(): void * All following statements after this will be executed against the replica node, * unless the keepReplica option is set to false and a primary connection * was already opened. + * + * @throws Exception */ public function ensureConnectedToReplica(): void { diff --git a/src/Driver.php b/src/Driver.php index 5f305fb5b9..0b8bd16bf5 100644 --- a/src/Driver.php +++ b/src/Driver.php @@ -8,6 +8,7 @@ use Doctrine\DBAL\Driver\Connection as DriverConnection; use Doctrine\DBAL\Driver\Exception; use Doctrine\DBAL\Platforms\AbstractPlatform; +use Doctrine\DBAL\Platforms\Exception\PlatformException; use SensitiveParameter; /** @@ -38,6 +39,8 @@ public function connect( * the platform this driver connects to. * * @return AbstractPlatform The database platform. + * + * @throws PlatformException */ public function getDatabasePlatform(ServerVersionProvider $versionProvider): AbstractPlatform; diff --git a/src/Driver/PDO/Result.php b/src/Driver/PDO/Result.php index a1918943f8..ca2ff455ca 100644 --- a/src/Driver/PDO/Result.php +++ b/src/Driver/PDO/Result.php @@ -75,6 +75,7 @@ public function columnCount(): int } } + /** @throws Exception */ public function getColumnName(int $index): string { try { diff --git a/src/Driver/PgSQL/Connection.php b/src/Driver/PgSQL/Connection.php index 4f280b0dc6..7a2732684f 100644 --- a/src/Driver/PgSQL/Connection.php +++ b/src/Driver/PgSQL/Connection.php @@ -41,6 +41,8 @@ public function __destruct() public function prepare(string $sql): Statement { $visitor = new ConvertParameters(); + + /** @phpstan-ignore missingType.checkedException */ $this->parser->parse($sql, $visitor); $statementName = uniqid('dbal', true); diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index 97be4022ff..a73d7dd9a1 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -5,7 +5,6 @@ namespace Doctrine\DBAL\Platforms; use Doctrine\DBAL\Connection; -use Doctrine\DBAL\Exception; use Doctrine\DBAL\Exception\InvalidColumnType\ColumnValuesRequired; use Doctrine\DBAL\Platforms\Keywords\KeywordList; use Doctrine\DBAL\Platforms\Keywords\MySQLKeywords; @@ -481,11 +480,7 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array ); } - /** - * @return list - * - * @throws Exception - */ + /** @return list */ private function getPreAlterTableAlterPrimaryKeySQL(TableDiff $diff, Index $index): array { if (! $index->isPrimary()) { @@ -526,8 +521,6 @@ private function getPreAlterTableAlterPrimaryKeySQL(TableDiff $diff, Index $inde * @param TableDiff $diff The table diff to gather the SQL for. * * @return list - * - * @throws Exception */ private function getPreAlterTableAlterIndexForeignKeySQL(TableDiff $diff): array { diff --git a/src/Platforms/SQLitePlatform.php b/src/Platforms/SQLitePlatform.php index e97cf81cca..77a2c09a31 100644 --- a/src/Platforms/SQLitePlatform.php +++ b/src/Platforms/SQLitePlatform.php @@ -5,7 +5,6 @@ namespace Doctrine\DBAL\Platforms; use Doctrine\DBAL\Connection; -use Doctrine\DBAL\Exception; use Doctrine\DBAL\Platforms\Exception\NotSupported; use Doctrine\DBAL\Platforms\Keywords\KeywordList; use Doctrine\DBAL\Platforms\Keywords\SQLiteKeywords; @@ -704,8 +703,6 @@ public function getAlterTableSQL(TableDiff $diff): array * @param array $columns * * @return array - * - * @throws Exception */ private function replaceColumn(string $tableName, array $columns, string $columnName, Column $column): array { @@ -724,11 +721,7 @@ private function replaceColumn(string $tableName, array $columns, string $column return array_combine($keys, $values); } - /** - * @return list|false - * - * @throws Exception - */ + /** @return list|false */ private function getSimpleAlterTableSQL(TableDiff $diff): array|false { if ( diff --git a/src/Portability/Driver.php b/src/Portability/Driver.php index bb67c258bf..d2f7de6d45 100644 --- a/src/Portability/Driver.php +++ b/src/Portability/Driver.php @@ -7,7 +7,9 @@ use Doctrine\DBAL\ColumnCase; use Doctrine\DBAL\Driver as DriverInterface; use Doctrine\DBAL\Driver\Connection as ConnectionInterface; +use Doctrine\DBAL\Driver\Exception; use Doctrine\DBAL\Driver\Middleware\AbstractDriverMiddleware; +use Doctrine\DBAL\Platforms\Exception\PlatformException; use PDO; use SensitiveParameter; @@ -26,6 +28,9 @@ public function __construct( /** * {@inheritDoc} + * + * @throws PlatformException + * @throws Exception */ public function connect( #[SensitiveParameter] diff --git a/src/Query/CommonTableExpression.php b/src/Query/CommonTableExpression.php index 81443adc7a..4306ac1ff4 100644 --- a/src/Query/CommonTableExpression.php +++ b/src/Query/CommonTableExpression.php @@ -10,7 +10,11 @@ /** @internal */ final class CommonTableExpression { - /** @param string[]|null $columns */ + /** + * @param string[]|null $columns + * + * @throws QueryException + */ public function __construct( public readonly string $name, public readonly string|QueryBuilder $query, diff --git a/src/Query/Expression/ExpressionBuilder.php b/src/Query/Expression/ExpressionBuilder.php index 14942b2f97..55e14960cc 100644 --- a/src/Query/Expression/ExpressionBuilder.php +++ b/src/Query/Expression/ExpressionBuilder.php @@ -5,6 +5,7 @@ namespace Doctrine\DBAL\Query\Expression; use Doctrine\DBAL\Connection; +use Doctrine\DBAL\Exception; use function implode; use function sprintf; @@ -236,6 +237,8 @@ public function notIn(string $x, string|array $y): string * * The usage of this method is discouraged. Use prepared statements * or {@see AbstractPlatform::quoteStringLiteral()} instead. + * + * @throws Exception */ public function literal(string $input): string { diff --git a/src/Query/QueryBuilder.php b/src/Query/QueryBuilder.php index e41337b972..12129c6c77 100644 --- a/src/Query/QueryBuilder.php +++ b/src/Query/QueryBuilder.php @@ -345,6 +345,8 @@ public function executeStatement(): int|string * * * @return string The SQL query string. + * + * @throws Exception */ public function getSQL(): string { @@ -551,6 +553,8 @@ public function union(string|QueryBuilder $part): self * * * @return $this + * + * @throws QueryException */ public function addUnion(string|QueryBuilder $part, UnionType $type = UnionType::DISTINCT): self { @@ -1417,6 +1421,8 @@ private function getSQLForDelete(): string /** * Converts this instance into a UNION string in SQL. + * + * @throws Exception */ private function getSQLForUnion(): string { @@ -1444,6 +1450,8 @@ private function getSQLForUnion(): string * the final SQL query being constructed. * * @return string The string representation of this QueryBuilder. + * + * @throws Exception */ public function __toString(): string { diff --git a/src/Schema/AbstractSchemaManager.php b/src/Schema/AbstractSchemaManager.php index 7f4aaaa2ba..95f74a8a68 100644 --- a/src/Schema/AbstractSchemaManager.php +++ b/src/Schema/AbstractSchemaManager.php @@ -166,6 +166,7 @@ public function tablesExist(array $names): bool return count($names) === count(array_intersect($names, array_map('strtolower', $this->listTableNames()))); } + /** @throws Exception */ public function tableExists(string $tableName): bool { return $this->tablesExist([$tableName]); diff --git a/src/Schema/ForeignKeyConstraint.php b/src/Schema/ForeignKeyConstraint.php index 00e78850a6..dd042ba759 100644 --- a/src/Schema/ForeignKeyConstraint.php +++ b/src/Schema/ForeignKeyConstraint.php @@ -648,6 +648,11 @@ private function parseMatchType(array $options): ?MatchType { if (isset($options['match'])) { try { + /** + * This looks like a PHPStan bug. + * + * @phpstan-ignore missingType.checkedException + */ return MatchType::from(strtoupper($options['match'])); } catch (ValueError $e) { Deprecation::trigger( @@ -669,6 +674,11 @@ private function parseReferentialAction(array $options, string $option): ?Refere { if (isset($options[$option])) { try { + /** + * This looks like a PHPStan bug. + * + * @phpstan-ignore missingType.checkedException + */ return ReferentialAction::from(strtoupper($options[$option])); } catch (ValueError $e) { Deprecation::trigger( diff --git a/src/Schema/PostgreSQLSchemaManager.php b/src/Schema/PostgreSQLSchemaManager.php index 6f278a4b1e..eb8e2a6fed 100644 --- a/src/Schema/PostgreSQLSchemaManager.php +++ b/src/Schema/PostgreSQLSchemaManager.php @@ -147,6 +147,7 @@ protected function _getPortableViewDefinition(array $view): View */ protected function _getPortableTableDefinition(array $table): string { + // @phpstan-ignore missingType.checkedException $currentSchema = $this->getCurrentSchema(); if ($table['schema_name'] === $currentSchema) { @@ -177,6 +178,7 @@ protected function _getPortableTableIndexesList(array $rows, string $tableName): implode(', ', $colNumbers), ); + // @phpstan-ignore missingType.checkedException $indexColumns = $this->connection->fetchAllAssociative($columnNameSql); // required for getting the order of the columns right. diff --git a/src/Schema/SQLServerSchemaManager.php b/src/Schema/SQLServerSchemaManager.php index 1cd352787d..6017795357 100644 --- a/src/Schema/SQLServerSchemaManager.php +++ b/src/Schema/SQLServerSchemaManager.php @@ -190,6 +190,7 @@ protected function _getPortableTableForeignKeysList(array $rows): array if (! isset($foreignKeys[$name])) { $referencedTableName = $row['ReferenceTableName']; + // @phpstan-ignore missingType.checkedException if ($row['ReferenceSchemaName'] !== $this->getCurrentSchemaName()) { $referencedTableName = $row['ReferenceSchemaName'] . '.' . $referencedTableName; } @@ -248,6 +249,7 @@ protected function _getPortableTableForeignKeyDefinition(array $tableForeignKey) */ protected function _getPortableTableDefinition(array $table): string { + // @phpstan-ignore missingType.checkedException if ($table['schema_name'] !== $this->getCurrentSchemaName()) { return $table['schema_name'] . '.' . $table['table_name']; } diff --git a/src/Schema/SQLiteSchemaManager.php b/src/Schema/SQLiteSchemaManager.php index 9d130e5296..119a6e96cf 100644 --- a/src/Schema/SQLiteSchemaManager.php +++ b/src/Schema/SQLiteSchemaManager.php @@ -228,6 +228,7 @@ protected function _getPortableTableForeignKeysList(array $rows): array // Inferring a shorthand form for the foreign key constraint, where the "to" field is empty. // @see https://www.sqlite.org/foreignkeys.html#fk_indexes. + // @phpstan-ignore missingType.checkedException $foreignTablePrimaryKeyColumnRows = $this->fetchPrimaryKeyColumns($value['foreignTable']); if (count($foreignTablePrimaryKeyColumnRows) < 1) {