From 74a30e95a2d8f1609785b526e0f92f2a8a6e9595 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 15:20:50 -0500 Subject: [PATCH 01/21] Add additional check for empty array --- src/Db/Adapter/AbstractAdapter.php | 2 +- .../TestCase/Db/Adapter/MysqlAdapterTest.php | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index 3bb4a81b..556a6078 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -745,7 +745,7 @@ protected function getUpsertClause(?InsertMode $mode, ?array $updateColumns, ?ar return ''; } - if ($conflictColumns !== null) { + if ($conflictColumns !== null && $conflictColumns !== []) { trigger_error( 'The $conflictColumns parameter is ignored by MySQL. ' . 'MySQL\'s ON DUPLICATE KEY UPDATE applies to all unique constraints on the table.', diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index aeee6316..9b0e0510 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -3080,6 +3080,38 @@ public function testInsertOrUpdateModeResetsAfterSave() ])->save(); } + public function testInsertOrUpdateWithEmptyConflictColumnsDoesNotWarn() + { + $table = new Table('currencies', [], $this->adapter); + $table->addColumn('code', 'string', ['limit' => 3]) + ->addColumn('rate', 'decimal', ['precision' => 10, 'scale' => 4]) + ->addIndex('code', ['unique' => true]) + ->create(); + + $warning = null; + set_error_handler(function (int $errno, string $errstr) use (&$warning) { + $warning = $errstr; + + return true; + }, E_USER_WARNING); + + try { + $table->insertOrUpdate([ + ['code' => 'USD', 'rate' => 1.0000], + ['code' => 'EUR', 'rate' => 0.9000], + ], ['rate'], [])->save(); + } finally { + restore_error_handler(); + } + + $this->assertNull($warning, 'Empty conflictColumns should not trigger a warning for MySQL'); + + $rows = $this->adapter->fetchAll('SELECT * FROM currencies ORDER BY code'); + $this->assertCount(2, $rows); + $this->assertEquals('0.9000', $rows[0]['rate']); + $this->assertEquals('1.0000', $rows[1]['rate']); + } + public function testCreateTableWithRangeColumnsPartitioning() { // MySQL requires RANGE COLUMNS for DATE columns From 69dde0eaa0b704df612fd32c691478e294982db0 Mon Sep 17 00:00:00 2001 From: Matthias Wirtz Date: Tue, 3 Mar 2026 05:06:21 +0100 Subject: [PATCH 02/21] add using when changing column type to json (#1031) --- src/Db/Adapter/PostgresAdapter.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index 5251ce4a..878f81ee 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -476,6 +476,12 @@ protected function getChangeColumnInstructions( $quotedColumnName, ); } + if (in_array($newColumn->getType(), ['json'])) { + $sql .= sprintf( + ' USING (%s::jsonb)', + $quotedColumnName, + ); + } // NULL and DEFAULT cannot be set while changing column type $sql = preg_replace('/ NOT NULL/', '', $sql); $sql = preg_replace('/ DEFAULT NULL/', '', $sql); From 944c4e20808dc846d4bd95690e727f767adb1403 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Thu, 5 Mar 2026 01:10:09 -0500 Subject: [PATCH 03/21] Fix CI failures on MySQL/MariaDB (#1034) 1. Handle uninitialized Column::$fixed property in BakeMigrationDiffCommand When TableSchema::getColumn() is called on cached/serialized schema objects, the Column::$fixed property may not be initialized, causing an Error. Added safeGetColumn() helper that catches this error and uses reflection to initialize uninitialized properties before retry. 2. Fix CURRENT_TIMESTAMP test assertion for MySQL/MariaDB Different versions of MySQL and MariaDB return CURRENT_TIMESTAMP in different formats (CURRENT_TIMESTAMP, current_timestamp(), CURRENT_TIMESTAMP()). Changed the test to use a regex that accepts all valid formats case-insensitively. Fixes #1033 --- src/Command/BakeMigrationDiffCommand.php | 73 ++++++++++++++++++++++-- tests/TestCase/MigrationsTest.php | 16 ++++-- 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/src/Command/BakeMigrationDiffCommand.php b/src/Command/BakeMigrationDiffCommand.php index 68bcd71b..d0490daf 100644 --- a/src/Command/BakeMigrationDiffCommand.php +++ b/src/Command/BakeMigrationDiffCommand.php @@ -27,13 +27,17 @@ use Cake\Database\Schema\ForeignKey; use Cake\Database\Schema\Index; use Cake\Database\Schema\TableSchema; +use Cake\Database\Schema\TableSchemaInterface; use Cake\Database\Schema\UniqueKey; use Cake\Datasource\ConnectionManager; use Cake\Event\Event; use Cake\Event\EventManager; +use Error; use Migrations\Migration\ManagerFactory; use Migrations\Util\TableFinder; use Migrations\Util\UtilTrait; +use ReflectionException; +use ReflectionProperty; /** * Task class for generating migration diff files. @@ -259,7 +263,7 @@ protected function getColumns(): void // brand new columns $addedColumns = array_diff($currentColumns, $oldColumns); foreach ($addedColumns as $columnName) { - $column = $currentSchema->getColumn($columnName); + $column = $this->safeGetColumn($currentSchema, $columnName); /** @var int $key */ $key = array_search($columnName, $currentColumns); if ($key > 0) { @@ -274,8 +278,8 @@ protected function getColumns(): void // changes in columns meta-data foreach ($currentColumns as $columnName) { - $column = $currentSchema->getColumn($columnName); - $oldColumn = $this->dumpSchema[$table]->getColumn($columnName); + $column = $this->safeGetColumn($currentSchema, $columnName); + $oldColumn = $this->safeGetColumn($this->dumpSchema[$table], $columnName); unset( $column['collate'], $column['fixed'], @@ -351,7 +355,7 @@ protected function getColumns(): void $removedColumns = array_diff($oldColumns, $currentColumns); if ($removedColumns) { foreach ($removedColumns as $columnName) { - $column = $this->dumpSchema[$table]->getColumn($columnName); + $column = $this->safeGetColumn($this->dumpSchema[$table], $columnName); /** @var int $key */ $key = array_search($columnName, $oldColumns); if ($key > 0) { @@ -621,6 +625,67 @@ public function template(): string return 'Migrations.config/diff'; } + /** + * Safely get column information from a TableSchema. + * + * This method handles the case where Column::$fixed property may not be + * initialized (e.g., when loaded from a cached/serialized schema). + * + * @param \Cake\Database\Schema\TableSchemaInterface $schema The table schema + * @param string $columnName The column name + * @return array|null Column data array or null if column doesn't exist + */ + protected function safeGetColumn(TableSchemaInterface $schema, string $columnName): ?array + { + try { + return $schema->getColumn($columnName); + } catch (Error $e) { + // Handle uninitialized typed property errors (e.g., Column::$fixed) + // This can happen with cached/serialized schema objects + if (str_contains($e->getMessage(), 'must not be accessed before initialization')) { + // Initialize uninitialized properties using reflection and retry + $this->initializeColumnProperties($schema, $columnName); + + return $schema->getColumn($columnName); + } + throw $e; + } + } + + /** + * Initialize potentially uninitialized Column properties using reflection. + * + * @param \Cake\Database\Schema\TableSchemaInterface $schema The table schema + * @param string $columnName The column name + * @return void + */ + protected function initializeColumnProperties(TableSchemaInterface $schema, string $columnName): void + { + // Access the internal columns array via reflection + $reflection = new ReflectionProperty($schema, '_columns'); + $columns = $reflection->getValue($schema); + + if (!isset($columns[$columnName]) || !($columns[$columnName] instanceof Column)) { + return; + } + + $column = $columns[$columnName]; + + // List of nullable properties that might not be initialized + $nullableProperties = ['fixed', 'collate', 'unsigned', 'generated', 'srid', 'onUpdate']; + + foreach ($nullableProperties as $propertyName) { + try { + $propReflection = new ReflectionProperty(Column::class, $propertyName); + if (!$propReflection->isInitialized($column)) { + $propReflection->setValue($column, null); + } + } catch (Error | ReflectionException) { + // Property doesn't exist or can't be accessed, skip it + } + } + } + /** * Gets the option parser instance and configures it. * diff --git a/tests/TestCase/MigrationsTest.php b/tests/TestCase/MigrationsTest.php index 5601a4ff..2830b88c 100644 --- a/tests/TestCase/MigrationsTest.php +++ b/tests/TestCase/MigrationsTest.php @@ -15,7 +15,6 @@ use Cake\Core\Configure; use Cake\Core\Plugin; -use Cake\Database\Driver\Mysql; use Cake\Database\Driver\Sqlserver; use Cake\Datasource\ConnectionManager; use Cake\TestSuite\TestCase; @@ -218,14 +217,19 @@ public function testMigrateAndRollback() $expected = ['id', 'name', 'created', 'updated']; $this->assertEquals($expected, $columns); $createdColumn = $storesTable->getSchema()->getColumn('created'); - $expected = 'CURRENT_TIMESTAMP'; $driver = $this->Connection->getDriver(); - if ($driver instanceof Mysql && $driver->isMariadb()) { - $expected = 'current_timestamp()'; - } elseif ($driver instanceof Sqlserver) { + if ($driver instanceof Sqlserver) { $expected = 'getdate()'; + $this->assertEquals($expected, $createdColumn['default']); + } else { + // MySQL and MariaDB may return CURRENT_TIMESTAMP in different formats + // depending on version: CURRENT_TIMESTAMP, current_timestamp(), CURRENT_TIMESTAMP() + $this->assertMatchesRegularExpression( + '/^current_timestamp(\(\))?$/i', + $createdColumn['default'], + 'Default value should be CURRENT_TIMESTAMP in some form', + ); } - $this->assertEquals($expected, $createdColumn['default']); // Rollback last $rollback = $this->migrations->rollback(); From d6720e14e1772f07483f3152e66dc1198c8763fb Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Thu, 5 Mar 2026 01:13:22 -0500 Subject: [PATCH 04/21] Fix TEXT column variants not round-tripping correctly (#1032) When using migration_diff, TEXT column variants (TINYTEXT, MEDIUMTEXT, LONGTEXT) were not being properly mapped back from the database. The BLOB type handling already used rawType to distinguish BLOB variants, but TEXT variants were missing equivalent handling. This adds similar rawType-based mapping for TEXT columns in mapColumnType() and includes round-trip tests. Fixes #1029 --- src/Db/Adapter/MysqlAdapter.php | 14 +++++++++ .../TestCase/Db/Adapter/MysqlAdapterTest.php | 31 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index b1d8a182..c07cfe6f 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -594,6 +594,20 @@ protected function mapColumnType(array $columnData): array } } // else: keep as binary or varbinary (actual BINARY/VARBINARY column) + } elseif ($type === TableSchema::TYPE_TEXT) { + // CakePHP returns TEXT columns as 'text' with specific lengths + // Check the raw MySQL type to distinguish TEXT variants + $rawType = $columnData['rawType'] ?? ''; + if (str_contains($rawType, 'tinytext')) { + $length = static::TEXT_TINY; + } elseif (str_contains($rawType, 'mediumtext')) { + $length = static::TEXT_MEDIUM; + } elseif (str_contains($rawType, 'longtext')) { + $length = static::TEXT_LONG; + } else { + // Regular TEXT - use null to indicate default TEXT type + $length = null; + } } return [$type, $length]; diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 9b0e0510..85aded71 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -1369,6 +1369,37 @@ public function testBlobRoundTrip(string $type, ?int $limit, string $expectedTyp $this->adapter->dropTable('blob_round_trip_test'); } + public static function textRoundTripData() + { + return [ + // type, limit, expected type after round-trip, expected limit after round-trip + ['text', null, 'text', null], + ['text', MysqlAdapter::TEXT_TINY, 'text', MysqlAdapter::TEXT_TINY], + ['text', MysqlAdapter::TEXT_MEDIUM, 'text', MysqlAdapter::TEXT_MEDIUM], + ['text', MysqlAdapter::TEXT_LONG, 'text', MysqlAdapter::TEXT_LONG], + ]; + } + + #[DataProvider('textRoundTripData')] + public function testTextRoundTrip(string $type, ?int $limit, string $expectedType, ?int $expectedLimit) + { + // Create a table with a TEXT column + $table = new Table('text_round_trip_test', [], $this->adapter); + $table->addColumn('text_col', $type, ['limit' => $limit]) + ->save(); + + // Read the column back from the database + $columns = $this->adapter->getColumns('text_round_trip_test'); + + $textColumn = $columns[1]; + $this->assertNotNull($textColumn, 'TEXT column not found'); + $this->assertSame($expectedType, $textColumn->getType(), 'Type mismatch after round-trip'); + $this->assertSame($expectedLimit, $textColumn->getLimit(), 'Limit mismatch after round-trip'); + + // Clean up + $this->adapter->dropTable('text_round_trip_test'); + } + public function testTimestampInvalidLimit() { $this->adapter->connect(); From 9d74ef7f224b963177afed9eeeaf0f17e9a3a261 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sat, 7 Mar 2026 05:00:26 +0100 Subject: [PATCH 05/21] Add fixed option for binary column type (#1014) Add support for the `fixed` attribute on binary columns to distinguish between fixed-length BINARY and variable-length VARBINARY types. This mirrors cakephp/cakephp#19207. * Add test cases for fixed option on binary columns Tests cover: - Column class getter/setter for fixed option - Column::toArray() including fixed in output - Column::setOptions() accepting fixed option - MigrationHelper::getColumnOption() including/excluding fixed - MysqlAdapter creating BINARY vs VARBINARY based on fixed option * Add column type assertions to binary fixed test --- src/Db/Adapter/MysqlAdapter.php | 3 ++ src/Db/Table/Column.php | 32 ++++++++++++ src/View/Helper/MigrationHelper.php | 3 +- .../TestCase/Db/Adapter/MysqlAdapterTest.php | 49 +++++++++++++++++ tests/TestCase/Db/Table/ColumnTest.php | 52 +++++++++++++++++++ .../View/Helper/MigrationHelperTest.php | 34 ++++++++++++ 6 files changed, 172 insertions(+), 1 deletion(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index c07cfe6f..a8aa1e8c 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -651,6 +651,9 @@ public function getColumns(string $tableName): array if ($record['onUpdate'] ?? false) { $column->setUpdate($record['onUpdate']); } + if ($record['fixed'] ?? false) { + $column->setFixed(true); + } $columns[] = $column; } diff --git a/src/Db/Table/Column.php b/src/Db/Table/Column.php index 98f28b31..e608560e 100644 --- a/src/Db/Table/Column.php +++ b/src/Db/Table/Column.php @@ -117,6 +117,11 @@ class Column extends DatabaseColumn */ protected ?string $lock = null; + /** + * @var bool|null + */ + protected ?bool $fixed = null; + /** * Column constructor * @@ -772,6 +777,31 @@ public function getLock(): ?string return $this->lock; } + /** + * Sets whether field should use fixed-length storage (for binary columns). + * + * When true, binary columns will use BINARY(n) instead of VARBINARY(n). + * + * @param bool $fixed Fixed + * @return $this + */ + public function setFixed(bool $fixed) + { + $this->fixed = $fixed; + + return $this; + } + + /** + * Gets whether field should use fixed-length storage. + * + * @return bool|null + */ + public function getFixed(): ?bool + { + return $this->fixed; + } + /** * Gets all allowed options. Each option must have a corresponding `setFoo` method. * @@ -802,6 +832,7 @@ protected function getValidOptions(): array 'generated', 'algorithm', 'lock', + 'fixed', ]; } @@ -894,6 +925,7 @@ public function toArray(): array 'default' => $default, 'generated' => $this->getGenerated(), 'unsigned' => $this->getUnsigned(), + 'fixed' => $this->getFixed(), 'onUpdate' => $this->getUpdate(), 'collate' => $this->getCollation(), 'precision' => $precision, diff --git a/src/View/Helper/MigrationHelper.php b/src/View/Helper/MigrationHelper.php index 3302e7e5..75e04cbb 100644 --- a/src/View/Helper/MigrationHelper.php +++ b/src/View/Helper/MigrationHelper.php @@ -389,6 +389,7 @@ public function getColumnOption(array $options): array 'scale', 'after', 'collate', + 'fixed', ]); $columnOptions = array_intersect_key($options, $wantedOptions); if (empty($columnOptions['comment'])) { @@ -495,7 +496,7 @@ public function attributes(TableSchemaInterface|string $table, string $column): 'comment', 'unsigned', 'signed', 'properties', 'autoIncrement', 'unique', - 'collate', + 'collate', 'fixed', ]; $attributes = []; diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 85aded71..1ab36b5c 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -3541,4 +3541,53 @@ public function testCombinedPartitionAndColumnOperations(): void $this->assertCount(1, $rows); $this->assertEquals('A description', $rows[0]['description']); } + + public function testBinaryColumnWithFixedOption(): void + { + $table = new Table('binary_fixed_test', [], $this->adapter); + $table->addColumn('hash', 'binary', ['limit' => 20, 'fixed' => true]) + ->addColumn('data', 'binary', ['limit' => 20]) + ->save(); + + $this->assertTrue($this->adapter->hasColumn('binary_fixed_test', 'hash')); + $this->assertTrue($this->adapter->hasColumn('binary_fixed_test', 'data')); + + // Check that the fixed column is created as BINARY and the non-fixed as VARBINARY + $rows = $this->adapter->fetchAll('SHOW COLUMNS FROM binary_fixed_test'); + $hashColumn = null; + $dataColumn = null; + foreach ($rows as $row) { + if ($row['Field'] === 'hash') { + $hashColumn = $row; + } + if ($row['Field'] === 'data') { + $dataColumn = $row; + } + } + + $this->assertNotNull($hashColumn); + $this->assertNotNull($dataColumn); + $this->assertSame('binary(20)', $hashColumn['Type']); + $this->assertSame('varbinary(20)', $dataColumn['Type']); + + // Verify the fixed attribute is reflected back + $columns = $this->adapter->getColumns('binary_fixed_test'); + $hashCol = null; + $dataCol = null; + foreach ($columns as $col) { + if ($col->getName() === 'hash') { + $hashCol = $col; + } + if ($col->getName() === 'data') { + $dataCol = $col; + } + } + + $this->assertNotNull($hashCol); + $this->assertNotNull($dataCol); + $this->assertSame('binary', $hashCol->getType()); + $this->assertSame('binary', $dataCol->getType()); + $this->assertTrue($hashCol->getFixed()); + $this->assertNull($dataCol->getFixed()); + } } diff --git a/tests/TestCase/Db/Table/ColumnTest.php b/tests/TestCase/Db/Table/ColumnTest.php index 0652149f..2d5ccd1a 100644 --- a/tests/TestCase/Db/Table/ColumnTest.php +++ b/tests/TestCase/Db/Table/ColumnTest.php @@ -275,4 +275,56 @@ public function testUnsignedConfigurationDoesNotAffectNonIntegerTypes(): void $decimalColumn->setName('price')->setType('decimal'); $this->assertFalse($decimalColumn->isUnsigned()); } + + public function testFixedOptionDefaultsToNull(): void + { + $column = new Column(); + $column->setName('data')->setType('binary'); + + $this->assertNull($column->getFixed()); + } + + public function testSetFixedTrue(): void + { + $column = new Column(); + $column->setName('hash')->setType('binary')->setFixed(true); + + $this->assertTrue($column->getFixed()); + } + + public function testSetFixedFalse(): void + { + $column = new Column(); + $column->setName('data')->setType('binary')->setFixed(false); + + $this->assertFalse($column->getFixed()); + } + + public function testSetOptionsWithFixed(): void + { + $column = new Column(); + $column->setName('hash')->setType('binary'); + $column->setOptions(['fixed' => true, 'limit' => 20]); + + $this->assertTrue($column->getFixed()); + $this->assertSame(20, $column->getLimit()); + } + + public function testToArrayIncludesFixed(): void + { + $column = new Column(); + $column->setName('hash')->setType('binary')->setFixed(true)->setLimit(20); + + $result = $column->toArray(); + $this->assertTrue($result['fixed']); + } + + public function testToArrayFixedNullByDefault(): void + { + $column = new Column(); + $column->setName('data')->setType('binary')->setLimit(20); + + $result = $column->toArray(); + $this->assertNull($result['fixed']); + } } diff --git a/tests/TestCase/View/Helper/MigrationHelperTest.php b/tests/TestCase/View/Helper/MigrationHelperTest.php index c7800936..fd2dfe91 100644 --- a/tests/TestCase/View/Helper/MigrationHelperTest.php +++ b/tests/TestCase/View/Helper/MigrationHelperTest.php @@ -456,4 +456,38 @@ public function testGetColumnOptionConvertsCollateToCollation(): void $this->assertArrayHasKey('collation', $result, 'collation should be set from collate value'); $this->assertSame('en_US.UTF-8', $result['collation']); } + + /** + * Test that getColumnOption includes the fixed option for binary columns + */ + public function testGetColumnOptionIncludesFixed(): void + { + $options = [ + 'length' => 20, + 'null' => true, + 'default' => null, + 'fixed' => true, + ]; + + $result = $this->helper->getColumnOption($options); + + $this->assertArrayHasKey('fixed', $result); + $this->assertTrue($result['fixed']); + } + + /** + * Test that getColumnOption excludes fixed when not set + */ + public function testGetColumnOptionExcludesFixedWhenNotSet(): void + { + $options = [ + 'length' => 20, + 'null' => true, + 'default' => null, + ]; + + $result = $this->helper->getColumnOption($options); + + $this->assertArrayNotHasKey('fixed', $result); + } } From ca1b74e523c105a12aa35e6c61d196b6997742f8 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sat, 7 Mar 2026 19:21:57 +0100 Subject: [PATCH 06/21] Fix misleading next steps message in upgrade command (#1037) Only show 'Set legacyTables => false' step after tables have been dropped, as the upgrade command becomes unavailable once this config is set. Fixes #1036 --- src/Command/UpgradeCommand.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Command/UpgradeCommand.php b/src/Command/UpgradeCommand.php index 5897c458..dece3c09 100644 --- a/src/Command/UpgradeCommand.php +++ b/src/Command/UpgradeCommand.php @@ -156,10 +156,13 @@ public function execute(Arguments $args, ConsoleIo $io): ?int $io->success('Upgrade complete!'); $io->out(''); $io->out('Next steps:'); - $io->out(' 1. Set \'Migrations\' => [\'legacyTables\' => false] in your config'); - $io->out(' 2. Test your application'); - if (!$dropTables) { - $io->out(' 3. Optionally drop the empty phinxlog tables (re-run `bin/cake migrations upgrade --drop-tables`)'); + if ($dropTables) { + $io->out(' 1. Set \'Migrations\' => [\'legacyTables\' => false] in your config'); + $io->out(' 2. Test your application'); + } else { + $io->out(' 1. Test your application'); + $io->out(' 2. Drop the phinxlog tables (re-run `bin/cake migrations upgrade --drop-tables`)'); + $io->out(' 3. Set \'Migrations\' => [\'legacyTables\' => false] in your config'); } } else { $io->out(''); From 8d27e00a7ea0db8bae4d808fd6bcc2847bab40fc Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sat, 7 Mar 2026 19:34:14 +0100 Subject: [PATCH 07/21] Fix upgrade command not matching plugins with slashes (#1039) * Fix upgrade command not matching plugins with slashes When upgrading from legacy phinxlog tables, the plugin name was being derived by camelizing the table prefix (e.g. cake_d_c_users -> CakeDCUsers). This didn't work for plugins with slashes in their names like CakeDC/Users since the slash was replaced with underscore during table name generation. This fix builds a map of loaded plugin names to their expected table prefixes, allowing proper matching of plugins like CakeDC/Users. Fixes #1038 * Fix test for SQL Server compatibility --- src/Command/UpgradeCommand.php | 34 ++++++++++- tests/TestCase/Command/UpgradeCommandTest.php | 61 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/Command/UpgradeCommand.php b/src/Command/UpgradeCommand.php index dece3c09..140eb02f 100644 --- a/src/Command/UpgradeCommand.php +++ b/src/Command/UpgradeCommand.php @@ -17,6 +17,7 @@ use Cake\Console\Arguments; use Cake\Console\ConsoleIo; use Cake\Console\ConsoleOptionParser; +use Cake\Core\Plugin; use Cake\Database\Connection; use Cake\Database\Exception\QueryException; use Cake\Datasource\ConnectionManager; @@ -184,13 +185,24 @@ protected function findLegacyTables(Connection $connection): array $tables = $schema->listTables(); $legacyTables = []; + // Build a map of expected table prefixes to plugin names for loaded plugins + // This allows matching plugins with special characters like CakeDC/Users + $pluginPrefixMap = $this->buildPluginPrefixMap(); + foreach ($tables as $table) { if ($table === 'phinxlog') { $legacyTables[$table] = null; } elseif (str_ends_with($table, '_phinxlog')) { // Extract plugin name from table name $prefix = substr($table, 0, -9); // Remove '_phinxlog' - $plugin = Inflector::camelize($prefix); + + // Try to match against loaded plugins first + if (isset($pluginPrefixMap[$prefix])) { + $plugin = $pluginPrefixMap[$prefix]; + } else { + // Fall back to camelizing the prefix + $plugin = Inflector::camelize($prefix); + } $legacyTables[$table] = $plugin; } } @@ -198,6 +210,26 @@ protected function findLegacyTables(Connection $connection): array return $legacyTables; } + /** + * Build a map of table prefixes to plugin names for all loaded plugins. + * + * This handles plugins with special characters like CakeDC/Users where + * the table prefix is cake_d_c_users but the plugin name is CakeDC/Users. + * + * @return array Map of table prefix => plugin name + */ + protected function buildPluginPrefixMap(): array + { + $map = []; + foreach (Plugin::loaded() as $plugin) { + $prefix = Inflector::underscore($plugin); + $prefix = str_replace(['\\', '/', '.'], '_', $prefix); + $map[$prefix] = $plugin; + } + + return $map; + } + /** * Check if a table exists. * diff --git a/tests/TestCase/Command/UpgradeCommandTest.php b/tests/TestCase/Command/UpgradeCommandTest.php index ceddd953..c7900bd7 100644 --- a/tests/TestCase/Command/UpgradeCommandTest.php +++ b/tests/TestCase/Command/UpgradeCommandTest.php @@ -166,4 +166,65 @@ public function testExecuteWithMigrations(): void $this->assertCount(1, $rows); } + + /** + * Test that plugins with slashes (like CakeDC/Users) are correctly identified + * during upgrade from legacy phinxlog tables. + */ + public function testExecuteWithSlashInPluginName(): void + { + Configure::write('Migrations.legacyTables', true); + + // Create the plugin's phinxlog table using the adapter for cross-database compatibility + $config = ConnectionManager::getConfig('test'); + $environment = new Environment('default', [ + 'connection' => 'test', + 'database' => $config['database'], + 'migration_table' => 'cake_d_c_users_phinxlog', + ]); + $adapter = $environment->getAdapter(); + try { + $adapter->createSchemaTable(); + } catch (Exception $e) { + // Table probably exists + } + + // Insert a migration record + $adapter->getInsertBuilder() + ->insert(['version', 'migration_name', 'breakpoint']) + ->into('cake_d_c_users_phinxlog') + ->values([ + 'version' => '20250118143003', + 'migration_name' => 'SlashPluginMigration', + 'breakpoint' => 0, + ]) + ->execute(); + + // Load a fake plugin with a slash in the name using loadPlugins + // which properly integrates with the console application + $this->loadPlugins(['CakeDC/Users' => ['path' => TMP]]); + + try { + $this->exec('migrations upgrade -c test'); + $this->assertExitSuccess(); + + $this->assertOutputContains('cake_d_c_users_phinxlog (CakeDC/Users)'); + + // Verify the plugin column has the correct value with slash + $rows = $this->getAdapter()->getSelectBuilder() + ->select(['version', 'migration_name', 'plugin']) + ->from('cake_migrations') + ->where(['migration_name' => 'SlashPluginMigration']) + ->all(); + + $this->assertCount(1, $rows); + $this->assertSame('CakeDC/Users', $rows[0]['plugin']); + } finally { + // Cleanup + /** @var \Cake\Database\Connection $connection */ + $connection = ConnectionManager::get('test'); + $connection->execute('DROP TABLE ' . $connection->getDriver()->quoteIdentifier('cake_d_c_users_phinxlog')); + $this->removePlugins(['CakeDC/Users']); + } + } } From 612764c7d69601b78b8efb43de861fc97cce1b03 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sat, 7 Mar 2026 19:38:22 +0100 Subject: [PATCH 08/21] Add TYPE_BIT constant to AdapterInterface (#1013) * Add TYPE_BIT constant to AdapterInterface Adds the TYPE_BIT constant mapping to TableSchemaInterface::TYPE_BIT for consistency with the core BIT type support added in cakephp/cakephp#19223. This allows migration users to reference AdapterInterface::TYPE_BIT when defining BIT columns in MySQL migrations. * Bump cakephp/database constraint to ^5.3.2 for TYPE_BIT support --- composer.json | 2 +- src/Db/Adapter/AdapterInterface.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 0d0597f1..1c3779aa 100644 --- a/composer.json +++ b/composer.json @@ -24,7 +24,7 @@ "require": { "php": ">=8.2", "cakephp/cache": "^5.3.0", - "cakephp/database": "^5.3.0", + "cakephp/database": "^5.3.2", "cakephp/orm": "^5.3.0" }, "require-dev": { diff --git a/src/Db/Adapter/AdapterInterface.php b/src/Db/Adapter/AdapterInterface.php index 74d43406..0dad1ca3 100644 --- a/src/Db/Adapter/AdapterInterface.php +++ b/src/Db/Adapter/AdapterInterface.php @@ -63,6 +63,7 @@ interface AdapterInterface // only for mysql so far public const TYPE_YEAR = TableSchemaInterface::TYPE_YEAR; + public const TYPE_BIT = TableSchemaInterface::TYPE_BIT; // only for postgresql so far public const TYPE_CIDR = TableSchemaInterface::TYPE_CIDR; From 69446ab066eb7790bd8aee04ee41163bffd10097 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 10:44:52 -0500 Subject: [PATCH 09/21] Bump PHPStan level +1 --- phpstan.neon | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon b/phpstan.neon index d3803bde..b35bf011 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -2,7 +2,7 @@ includes: - phpstan-baseline.neon parameters: - level: 7 + level: 8 paths: - src/ bootstrapFiles: From 027c7c8020e46ecdf90e84e6ff4e990fbae764a0 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 11:00:56 -0500 Subject: [PATCH 10/21] Make props non-nullable --- src/Command/BakeSimpleMigrationCommand.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Command/BakeSimpleMigrationCommand.php b/src/Command/BakeSimpleMigrationCommand.php index 622b61fc..08b703e3 100644 --- a/src/Command/BakeSimpleMigrationCommand.php +++ b/src/Command/BakeSimpleMigrationCommand.php @@ -52,16 +52,16 @@ abstract class BakeSimpleMigrationCommand extends SimpleBakeCommand /** * Console IO * - * @var \Cake\Console\ConsoleIo|null + * @var \Cake\Console\ConsoleIo */ - protected ?ConsoleIo $io = null; + protected ConsoleIo $io; /** * Arguments * - * @var \Cake\Console\Arguments|null + * @var \Cake\Console\Arguments */ - protected ?Arguments $args = null; + protected Arguments $args; /** * @inheritDoc From 65dcf7e6ebb8c1bc67b4e21152b269575a75bc58 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 21:57:44 -0400 Subject: [PATCH 11/21] Add null guards for getConstraint() in BakeMigrationDiffCommand --- src/Command/BakeMigrationDiffCommand.php | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Command/BakeMigrationDiffCommand.php b/src/Command/BakeMigrationDiffCommand.php index d0490daf..666fa0fd 100644 --- a/src/Command/BakeMigrationDiffCommand.php +++ b/src/Command/BakeMigrationDiffCommand.php @@ -283,12 +283,17 @@ protected function getColumns(): void unset( $column['collate'], $column['fixed'], - $oldColumn['collate'], - $oldColumn['fixed'], ); + if ($oldColumn !== null) { + unset( + $oldColumn['collate'], + $oldColumn['fixed'], + ); + } if ( in_array($columnName, $oldColumns, true) && + $oldColumn !== null && $column !== $oldColumn ) { $changedAttributes = array_diff_assoc($column, $oldColumn); @@ -385,9 +390,10 @@ protected function getConstraints(): void // brand new constraints $addedConstraints = array_diff($currentConstraints, $oldConstraints); foreach ($addedConstraints as $constraintName) { - $this->templateData[$table]['constraints']['add'][$constraintName] = - $currentSchema->getConstraint($constraintName); $constraint = $currentSchema->getConstraint($constraintName); + if ($constraint === null) { + continue; + } if ($constraint['type'] === TableSchema::CONSTRAINT_FOREIGN) { $this->templateData[$table]['constraints']['add'][$constraintName] = $constraint; } else { @@ -415,6 +421,9 @@ protected function getConstraints(): void $removedConstraints = array_diff($oldConstraints, $currentConstraints); foreach ($removedConstraints as $constraintName) { $constraint = $this->dumpSchema[$table]->getConstraint($constraintName); + if ($constraint === null) { + continue; + } if ($constraint['type'] === TableSchema::CONSTRAINT_FOREIGN) { $this->templateData[$table]['constraints']['remove'][$constraintName] = $constraint; } else { From e14379349b48ddb0440ac8ea3e72b336ac492d7d Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 13:42:26 -0500 Subject: [PATCH 12/21] Fix nullable return types in Column and ForeignKey Column::getNull() now coalesces the nullable bool property to false. ForeignKey::getOnDelete()/getOnUpdate() guard against null from getDelete()/getUpdate() before calling mapAction(). --- src/Db/Table/Column.php | 2 +- src/Db/Table/ForeignKey.php | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Db/Table/Column.php b/src/Db/Table/Column.php index e608560e..698a79b1 100644 --- a/src/Db/Table/Column.php +++ b/src/Db/Table/Column.php @@ -234,7 +234,7 @@ public function setNull(bool $null) */ public function getNull(): bool { - return $this->null; + return $this->null ?? false; } /** diff --git a/src/Db/Table/ForeignKey.php b/src/Db/Table/ForeignKey.php index 907e5f37..929930a9 100644 --- a/src/Db/Table/ForeignKey.php +++ b/src/Db/Table/ForeignKey.php @@ -246,7 +246,9 @@ public function setOnDelete(string $onDelete) */ public function getOnDelete(): ?string { - return $this->mapAction($this->getDelete()); + $delete = $this->getDelete(); + + return $delete !== null ? $this->mapAction($delete) : null; } /** @@ -271,6 +273,8 @@ public function setOnUpdate(string $onUpdate) */ public function getOnUpdate(): ?string { - return $this->mapAction($this->getUpdate()); + $update = $this->getUpdate(); + + return $update !== null ? $this->mapAction($update) : null; } } From dc5ea90e492ff167366648a3ac698df4044fd58c Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 13:42:47 -0500 Subject: [PATCH 13/21] Fix null safety in Db adapters and domain classes Cast preg_replace results to string, coalesce nullable getColumns() returns, cast nullable getReferencedTable()/getName() at call sites, and use null-safe operator for optional ConsoleIo. --- src/Db/Adapter/AbstractAdapter.php | 3 ++- src/Db/Adapter/MysqlAdapter.php | 6 +++--- src/Db/Adapter/PostgresAdapter.php | 18 +++++++++--------- src/Db/Adapter/SqliteAdapter.php | 14 +++++++------- src/Db/Adapter/SqlserverAdapter.php | 10 +++++----- src/Db/Adapter/TimedOutputAdapter.php | 2 +- src/Db/Plan/Plan.php | 2 +- src/Db/Table.php | 6 +++--- 8 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index 556a6078..e9c83313 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -201,6 +201,7 @@ public function getConnection(): Connection $this->connection = $this->getOption('connection'); $this->connect(); } + assert($this->connection !== null); return $this->connection; } @@ -1681,7 +1682,7 @@ public function executeActions(TableMetadata $table, array $actions): void /** @var \Migrations\Db\Action\DropForeignKey $action */ $instructions->merge($this->getDropForeignKeyByColumnsInstructions( $table->getName(), - $action->getForeignKey()->getColumns(), + $action->getForeignKey()->getColumns() ?? [], )); break; diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index a8aa1e8c..8a66b91f 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -727,7 +727,7 @@ protected function getRenameColumnInstructions(string $tableName, string $column $targetColumn = null; foreach ($columns as $column) { - if (strcasecmp($column->getName(), $columnName) === 0) { + if (strcasecmp((string)$column->getName(), $columnName) === 0) { $targetColumn = $column; break; } @@ -1201,7 +1201,7 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string $def .= ' CONSTRAINT ' . $this->quoteColumnName((string)$foreignKey->getName()); } $columnNames = []; - foreach ($foreignKey->getColumns() as $column) { + foreach ($foreignKey->getColumns() ?? [] as $column) { $columnNames[] = $this->quoteColumnName($column); } $def .= ' FOREIGN KEY (' . implode(',', $columnNames) . ')'; @@ -1209,7 +1209,7 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string foreach ($foreignKey->getReferencedColumns() as $column) { $refColumnNames[] = $this->quoteColumnName($column); } - $def .= ' REFERENCES ' . $this->quoteTableName($foreignKey->getReferencedTable()) . ' (' . implode(',', $refColumnNames) . ')'; + $def .= ' REFERENCES ' . $this->quoteTableName((string)$foreignKey->getReferencedTable()) . ' (' . implode(',', $refColumnNames) . ')'; $onDelete = $foreignKey->getOnDelete(); if ($onDelete) { $def .= ' ON DELETE ' . $onDelete; diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index 878f81ee..b90641e3 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -455,9 +455,9 @@ protected function getChangeColumnInstructions( $columnSql = $dialect->columnDefinitionSql($this->mapColumnData($newColumn->toArray())); // Remove the column name from $columnSql - $columnType = preg_replace('/^"?(?:[^"]+)"?\s+/', '', $columnSql); + $columnType = (string)preg_replace('/^"?(?:[^"]+)"?\s+/', '', $columnSql); // Remove generated clause - $columnType = preg_replace('/GENERATED (?:ALWAYS|BY DEFAULT) AS IDENTITY/', '', $columnType); + $columnType = (string)preg_replace('/GENERATED (?:ALWAYS|BY DEFAULT) AS IDENTITY/', '', $columnType); $sql = sprintf( 'ALTER COLUMN %s TYPE %s', @@ -483,10 +483,10 @@ protected function getChangeColumnInstructions( ); } // NULL and DEFAULT cannot be set while changing column type - $sql = preg_replace('/ NOT NULL/', '', $sql); - $sql = preg_replace('/ DEFAULT NULL/', '', $sql); + $sql = (string)preg_replace('/ NOT NULL/', '', $sql); + $sql = (string)preg_replace('/ DEFAULT NULL/', '', $sql); // If it is set, DEFAULT is the last definition - $sql = preg_replace('/DEFAULT .*/', '', $sql); + $sql = (string)preg_replace('/DEFAULT .*/', '', $sql); if ($newColumn->getType() === 'boolean') { $sql .= sprintf( ' USING (CASE WHEN %s IS NULL THEN NULL WHEN %s::int=0 THEN FALSE ELSE TRUE END)', @@ -952,13 +952,13 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey, string $ta $parts = $this->getSchemaName($tableName); $constraintName = $foreignKey->getName() ?: ( - $parts['table'] . '_' . implode('_', $foreignKey->getColumns()) . '_fkey' + $parts['table'] . '_' . implode('_', $foreignKey->getColumns() ?? []) . '_fkey' ); - $columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns())); + $columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns() ?? [])); $refColumnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getReferencedColumns())); $def = ' CONSTRAINT ' . $this->quoteColumnName($constraintName) . ' FOREIGN KEY (' . $columnList . ')' . - ' REFERENCES ' . $this->quoteTableName($foreignKey->getReferencedTable()) . ' (' . $refColumnList . ')'; + ' REFERENCES ' . $this->quoteTableName((string)$foreignKey->getReferencedTable()) . ' (' . $refColumnList . ')'; if ($foreignKey->getOnDelete()) { $def .= " ON DELETE {$foreignKey->getOnDelete()}"; } @@ -1327,7 +1327,7 @@ protected function getConflictClause( } $quotedConflictColumns = array_map($this->quoteColumnName(...), $conflictColumns); $updates = []; - foreach ($updateColumns as $column) { + foreach ($updateColumns ?? [] as $column) { $quotedColumn = $this->quoteColumnName($column); $updates[] = $quotedColumn . ' = EXCLUDED.' . $quotedColumn; } diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php index 9145e0cb..00d3024d 100644 --- a/src/Db/Adapter/SqliteAdapter.php +++ b/src/Db/Adapter/SqliteAdapter.php @@ -570,7 +570,7 @@ protected function getAddColumnInstructions(TableMetadata $table, Column $column return $state; } $finalColumnName = end($columns)->getName(); - $sql = preg_replace( + $sql = (string)preg_replace( sprintf( "/(%s(?:\/\*.*?\*\/|\([^)]+\)|'[^']*?'|[^,])+)([,)])/", $this->quoteColumnName((string)$finalColumnName), @@ -619,7 +619,7 @@ protected function getDeclaringSql(string $tableName): string $columnNamePattern = "\"$columnName\"|`$columnName`|\\[$columnName\\]|$columnName"; $columnNamePattern = "#([\(,]+\\s*)($columnNamePattern)(\\s)#iU"; - $sql = preg_replace_callback( + $sql = (string)preg_replace_callback( $columnNamePattern, function ($matches) use ($column) { return $matches[1] . $this->quoteColumnName($column['name']) . $matches[3]; @@ -631,7 +631,7 @@ function ($matches) use ($column) { $tableNamePattern = "\"$tableName\"|`$tableName`|\\[$tableName\\]|$tableName"; $tableNamePattern = "#^(CREATE TABLE)\s*($tableNamePattern)\s*(\()#Ui"; - $sql = preg_replace($tableNamePattern, "$1 `$tableName` $3", $sql, 1); + $sql = (string)preg_replace($tableNamePattern, "$1 `$tableName` $3", $sql, 1); return $sql; } @@ -1115,7 +1115,7 @@ protected function getChangeColumnInstructions(string $tableName, string $column $newColumnName = (string)$newColumn->getName(); $instructions->addPostStep(function ($state) use ($columnName, $newColumn) { $dialect = $this->getSchemaDialect(); - $sql = preg_replace( + $sql = (string)preg_replace( sprintf("/%s(?:\/\*.*?\*\/|\([^)]+\)|'[^']*?'|[^,])+([,)])/", $this->quoteColumnName($columnName)), sprintf('%s$1', $dialect->columnDefinitionSql($newColumn->toArray())), (string)$state['createSQL'], @@ -1149,7 +1149,7 @@ protected function getDropColumnInstructions(string $tableName, string $columnNa }); $instructions->addPostStep(function ($state) use ($columnName) { - $sql = preg_replace( + $sql = (string)preg_replace( sprintf("/%s\s\w+.*(,\s(?!')|\)$)/U", preg_quote($this->quoteColumnName($columnName))), '', (string)$state['createSQL'], @@ -1679,7 +1679,7 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string $def .= ' CONSTRAINT ' . $this->quoteColumnName((string)$foreignKey->getName()); } $columnNames = []; - foreach ($foreignKey->getColumns() as $column) { + foreach ($foreignKey->getColumns() ?? [] as $column) { $columnNames[] = $this->quoteColumnName($column); } $def .= ' FOREIGN KEY (' . implode(',', $columnNames) . ')'; @@ -1687,7 +1687,7 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string foreach ($foreignKey->getReferencedColumns() as $column) { $refColumnNames[] = $this->quoteColumnName($column); } - $def .= ' REFERENCES ' . $this->quoteTableName($foreignKey->getReferencedTable()) . ' (' . implode(',', $refColumnNames) . ')'; + $def .= ' REFERENCES ' . $this->quoteTableName((string)$foreignKey->getReferencedTable()) . ' (' . implode(',', $refColumnNames) . ')'; if ($foreignKey->getOnDelete()) { $def .= ' ON DELETE ' . $foreignKey->getOnDelete(); } diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index e804d590..d2ebd336 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -338,7 +338,7 @@ protected function parseDefault(?string $default): int|string|null $result = preg_replace(["/\('(.*)'\)/", "/\(\((.*)\)\)/", "/\((.*)\)/"], '$1', $default); - if (strtoupper($result) === 'NULL') { + if (strtoupper((string)$result) === 'NULL') { $result = null; } elseif (is_numeric($result)) { $result = (int)$result; @@ -475,7 +475,7 @@ protected function getChangeColumnInstructions(string $tableName, string $column $dialect->columnDefinitionSql($columnData), ); $alterColumn = preg_replace('/DEFAULT NULL/', '', $alterColumn); - $instructions->addPostStep($alterColumn); + $instructions->addPostStep((string)$alterColumn); // change column comment if needed if ($newColumn->getComment()) { @@ -864,13 +864,13 @@ protected function getIndexSqlDefinition(Index $index, string $tableName): strin */ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey, string $tableName): string { - $constraintName = $foreignKey->getName() ?: $tableName . '_' . implode('_', $foreignKey->getColumns()); - $columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns())); + $constraintName = $foreignKey->getName() ?: $tableName . '_' . implode('_', $foreignKey->getColumns() ?? []); + $columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns() ?? [])); $refColumnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getReferencedColumns())); $def = ' CONSTRAINT ' . $this->quoteColumnName($constraintName); $def .= ' FOREIGN KEY (' . $columnList . ')'; - $def .= ' REFERENCES ' . $this->quoteTableName($foreignKey->getReferencedTable()) . ' (' . $refColumnList . ')'; + $def .= ' REFERENCES ' . $this->quoteTableName((string)$foreignKey->getReferencedTable()) . ' (' . $refColumnList . ')'; if ($foreignKey->getOnDelete()) { $def .= " ON DELETE {$foreignKey->getOnDelete()}"; } diff --git a/src/Db/Adapter/TimedOutputAdapter.php b/src/Db/Adapter/TimedOutputAdapter.php index f5e11be6..fa5b47a1 100644 --- a/src/Db/Adapter/TimedOutputAdapter.php +++ b/src/Db/Adapter/TimedOutputAdapter.php @@ -78,7 +78,7 @@ function ($value) { return; } - $this->getIo()->verbose(' -- ' . $command); + $this->getIo()?->verbose(' -- ' . $command); } /** diff --git a/src/Db/Plan/Plan.php b/src/Db/Plan/Plan.php index dcbaa718..8969ebef 100644 --- a/src/Db/Plan/Plan.php +++ b/src/Db/Plan/Plan.php @@ -284,7 +284,7 @@ protected function remapContraintAndIndexConflicts(AlterTable $alter): AlterTabl if ($action instanceof DropForeignKey) { [$this->indexes, $dropIndexActions] = $this->forgetDropIndex( $action->getTable(), - $action->getForeignKey()->getColumns(), + $action->getForeignKey()->getColumns() ?? [], $this->indexes, ); foreach ($dropIndexActions as $dropIndexAction) { diff --git a/src/Db/Table.php b/src/Db/Table.php index aeeb0906..31aee372 100644 --- a/src/Db/Table.php +++ b/src/Db/Table.php @@ -362,7 +362,7 @@ public function addColumn(string|Column $columnName, ?string $type = null, array if ($columnName instanceof Column) { $action = new AddColumn($this->table, $columnName); } else { - $action = new AddColumn($this->table, $this->getAdapter()->getColumnForType($columnName, $type, $options)); + $action = new AddColumn($this->table, $this->getAdapter()->getColumnForType($columnName, (string)$type, $options)); } // Delegate to Adapters to check column type @@ -902,7 +902,7 @@ protected function filterPrimaryKey(array $options): void return $action->getColumn(); }); $primaryKeyColumns = $columnsCollection->filter(function (Column $columnDef, $key) use ($primaryKey) { - return isset($primaryKey[$columnDef->getName()]); + return isset($primaryKey[(string)$columnDef->getName()]); })->toArray(); if (!$primaryKeyColumns) { @@ -911,7 +911,7 @@ protected function filterPrimaryKey(array $options): void foreach ($primaryKeyColumns as $primaryKeyColumn) { if ($primaryKeyColumn->isIdentity()) { - unset($primaryKey[$primaryKeyColumn->getName()]); + unset($primaryKey[(string)$primaryKeyColumn->getName()]); } } From 61d82f16d07e0bb0e441108b7982128af08e32ec Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 21:58:28 -0400 Subject: [PATCH 14/21] Fix remaining PHPStan level 8 null safety issues Add null guards for constraint diff, fail-fast for null column type, fix import order, and remaining null safety in BaseSeed, ColumnParser, TableFinder, and MigrationHelper. --- src/BaseSeed.php | 4 ++-- src/Command/BakeMigrationDiffCommand.php | 11 ++++++++--- src/Db/Adapter/RecordingAdapter.php | 1 + src/Db/Table.php | 5 +++-- src/Util/ColumnParser.php | 2 +- src/Util/TableFinder.php | 5 +++-- src/View/Helper/MigrationHelper.php | 2 +- tests/TestCase/Db/Table/TableTest.php | 9 +++++++++ 8 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/BaseSeed.php b/src/BaseSeed.php index 146abc4e..ab7c9304 100644 --- a/src/BaseSeed.php +++ b/src/BaseSeed.php @@ -278,8 +278,8 @@ protected function runCall(string $seeder, array $options = []): void $options += [ 'connection' => $connection, - 'plugin' => $pluginName ?? $config['plugin'], - 'source' => $config['source'], + 'plugin' => $pluginName ?? ($config !== null ? $config['plugin'] : null), + 'source' => $config !== null ? $config['source'] : null, ]; $factory = new ManagerFactory([ 'connection' => $options['connection'], diff --git a/src/Command/BakeMigrationDiffCommand.php b/src/Command/BakeMigrationDiffCommand.php index 666fa0fd..e205bf71 100644 --- a/src/Command/BakeMigrationDiffCommand.php +++ b/src/Command/BakeMigrationDiffCommand.php @@ -405,13 +405,18 @@ protected function getConstraints(): void // if present in both, check if they are the same : if not, remove the old one and add the new one foreach ($currentConstraints as $constraintName) { $constraint = $currentSchema->getConstraint($constraintName); + if ($constraint === null) { + continue; + } + $oldConstraint = $this->dumpSchema[$table]->getConstraint($constraintName); if ( in_array($constraintName, $oldConstraints, true) && - $constraint !== $this->dumpSchema[$table]->getConstraint($constraintName) + $constraint !== $oldConstraint ) { - $this->templateData[$table]['constraints']['remove'][$constraintName] = - $this->dumpSchema[$table]->getConstraint($constraintName); + if ($oldConstraint !== null) { + $this->templateData[$table]['constraints']['remove'][$constraintName] = $oldConstraint; + } $this->templateData[$table]['constraints']['add'][$constraintName] = $constraint; } diff --git a/src/Db/Adapter/RecordingAdapter.php b/src/Db/Adapter/RecordingAdapter.php index aab4b41b..00054a01 100644 --- a/src/Db/Adapter/RecordingAdapter.php +++ b/src/Db/Adapter/RecordingAdapter.php @@ -8,6 +8,7 @@ namespace Migrations\Db\Adapter; +use InvalidArgumentException; use Migrations\Db\Action\AddColumn; use Migrations\Db\Action\AddForeignKey; use Migrations\Db\Action\AddIndex; diff --git a/src/Db/Table.php b/src/Db/Table.php index 31aee372..cb026b0e 100644 --- a/src/Db/Table.php +++ b/src/Db/Table.php @@ -358,11 +358,12 @@ public function addPrimaryKey(string|array $columns) */ public function addColumn(string|Column $columnName, ?string $type = null, array $options = []) { - assert($columnName instanceof Column || $type !== null); if ($columnName instanceof Column) { $action = new AddColumn($this->table, $columnName); + } elseif ($type === null) { + throw new InvalidArgumentException('Column type must not be null when column name is a string.'); } else { - $action = new AddColumn($this->table, $this->getAdapter()->getColumnForType($columnName, (string)$type, $options)); + $action = new AddColumn($this->table, $this->getAdapter()->getColumnForType($columnName, $type, $options)); } // Delegate to Adapters to check column type diff --git a/src/Util/ColumnParser.php b/src/Util/ColumnParser.php index 97b9efdc..e16d1ab8 100644 --- a/src/Util/ColumnParser.php +++ b/src/Util/ColumnParser.php @@ -213,7 +213,7 @@ public function parseForeignKeys(array $arguments): array $referencedTable = $indexType; if (!$referencedTable) { // Remove common suffixes like _id and pluralize - $referencedTable = preg_replace('/_id$/', '', $fieldName); + $referencedTable = (string)preg_replace('/_id$/', '', $fieldName); $referencedTable = Inflector::pluralize($referencedTable); } diff --git a/src/Util/TableFinder.php b/src/Util/TableFinder.php index a96b4e03..5e75c8d6 100644 --- a/src/Util/TableFinder.php +++ b/src/Util/TableFinder.php @@ -183,9 +183,10 @@ public function fetchTableName(string $className, ?string $pluginName = null): a $table = TableRegistry::getTableLocator()->get($className); foreach ($table->associations()->keys() as $key) { - if ($table->associations()->get($key)->type() === 'belongsToMany') { + $association = $table->associations()->get($key); + if ($association !== null && $association->type() === 'belongsToMany') { /** @var \Cake\ORM\Association\BelongsToMany $belongsToMany */ - $belongsToMany = $table->associations()->get($key); + $belongsToMany = $association; $tables[] = $belongsToMany->junction()->getTable(); } } diff --git a/src/View/Helper/MigrationHelper.php b/src/View/Helper/MigrationHelper.php index 75e04cbb..c933c627 100644 --- a/src/View/Helper/MigrationHelper.php +++ b/src/View/Helper/MigrationHelper.php @@ -693,7 +693,7 @@ public function getCreateTableData(TableSchemaInterface|string $table): array $indexes = $this->indexes($table); $foreignKeys = []; foreach ($constraints as $constraint) { - if ($constraint['type'] === 'foreign') { + if (isset($constraint['type']) && $constraint['type'] === 'foreign') { $foreignKeys[] = $constraint['columns']; } } diff --git a/tests/TestCase/Db/Table/TableTest.php b/tests/TestCase/Db/Table/TableTest.php index 21912fd5..1be0f352 100644 --- a/tests/TestCase/Db/Table/TableTest.php +++ b/tests/TestCase/Db/Table/TableTest.php @@ -80,6 +80,15 @@ public function testAddColumnWithColumnObject() $this->assertSame($column, $actions[0]->getColumn()); } + public function testAddColumnWithNullTypeThrows() + { + $adapter = new MysqlAdapter([]); + $table = new Table('ntable', [], $adapter); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Column type must not be null when column name is a string.'); + $table->addColumn('email', null); + } + public function testAddColumnWithNoAdapterSpecified() { try { From 2ceab0b5eeb01ee69d973a8bf815ff630e693ea3 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 15:41:42 -0500 Subject: [PATCH 15/21] Replace assert with RuntimeException in AbstractAdapter::getConnection() Assertions can be disabled in production, which would allow a null connection to slip through silently. Throw a RuntimeException instead to fail fast in all environments. --- src/Db/Adapter/AbstractAdapter.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index e9c83313..ba371019 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -201,7 +201,9 @@ public function getConnection(): Connection $this->connection = $this->getOption('connection'); $this->connect(); } - assert($this->connection !== null); + if ($this->connection === null) { + throw new RuntimeException('Unable to establish database connection. Ensure a connection is configured.'); + } return $this->connection; } From fe754cdcfebb3cbc39d170b191f8135150000221 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 22:02:35 -0400 Subject: [PATCH 16/21] Use truthiness checks for optional nullable getters Replace (string) casts with assign-then-check pattern for optional values that can legitimately be null: Index::getWhere(), ForeignKey::getName(), Index::getName(), Column::getGenerated(). This avoids silently converting null to empty string. --- src/Db/Adapter/AbstractAdapter.php | 6 ++++-- src/Db/Adapter/MysqlAdapter.php | 5 +++-- src/Db/Adapter/PostgresAdapter.php | 13 +++++++------ src/Db/Adapter/SqliteAdapter.php | 12 +++++++----- src/Db/Adapter/SqlserverAdapter.php | 7 ++++--- 5 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index ba371019..709504aa 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -1690,17 +1690,19 @@ public function executeActions(TableMetadata $table, array $actions): void case $action instanceof DropForeignKey && $action->getForeignKey()->getName(): /** @var \Migrations\Db\Action\DropForeignKey $action */ + $fkName = $action->getForeignKey()->getName(); $instructions->merge($this->getDropForeignKeyInstructions( $table->getName(), - (string)$action->getForeignKey()->getName(), + $fkName, )); break; case $action instanceof DropIndex && $action->getIndex()->getName(): /** @var \Migrations\Db\Action\DropIndex $action */ + $indexName = $action->getIndex()->getName(); $instructions->merge($this->getDropIndexByNameInstructions( $table->getName(), - (string)$action->getIndex()->getName(), + $indexName, )); break; diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 8a66b91f..ab6b9cf1 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -1197,8 +1197,9 @@ protected function getIndexSqlDefinition(Index $index): string protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string { $def = ''; - if ($foreignKey->getName()) { - $def .= ' CONSTRAINT ' . $this->quoteColumnName((string)$foreignKey->getName()); + $name = $foreignKey->getName(); + if ($name) { + $def .= ' CONSTRAINT ' . $this->quoteColumnName($name); } $columnNames = []; foreach ($foreignKey->getColumns() ?? [] as $column) { diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index b90641e3..3b02b598 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -505,11 +505,11 @@ protected function getChangeColumnInstructions( 'ALTER COLUMN %s', $quotedColumnName, ); - if ($newColumn->isIdentity() && $newColumn->getGenerated() !== null) { + if ($newColumn->isIdentity() && ($generated = $newColumn->getGenerated()) !== null) { if ($column->isIdentity()) { - $sql .= sprintf(' SET GENERATED %s', (string)$newColumn->getGenerated()); + $sql .= sprintf(' SET GENERATED %s', $generated); } else { - $sql .= sprintf(' ADD GENERATED %s AS IDENTITY', (string)$newColumn->getGenerated()); + $sql .= sprintf(' ADD GENERATED %s AS IDENTITY', $generated); } } else { $sql .= ' DROP IDENTITY IF EXISTS'; @@ -923,9 +923,10 @@ protected function getIndexSqlDefinition(Index $index, string $tableName): strin } else { $createIndexSentence .= '(%s)%s%s;'; } - $where = (string)$index->getWhere(); - if ($where) { - $where = ' WHERE ' . $where; + $where = ''; + $whereClause = $index->getWhere(); + if ($whereClause) { + $where = ' WHERE ' . $whereClause; } return sprintf( diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php index 00d3024d..c24e3665 100644 --- a/src/Db/Adapter/SqliteAdapter.php +++ b/src/Db/Adapter/SqliteAdapter.php @@ -1214,9 +1214,10 @@ protected function getAddIndexInstructions(TableMetadata $table, Index $index): $indexColumnArray[] = sprintf('%s ASC', $this->quoteColumnName($column)); } $indexColumns = implode(',', $indexColumnArray); - $where = (string)$index->getWhere(); - if ($where) { - $where = ' WHERE ' . $where; + $where = ''; + $whereClause = $index->getWhere(); + if ($whereClause) { + $where = ' WHERE ' . $whereClause; } $sql = sprintf( 'CREATE %s ON %s (%s)%s', @@ -1675,8 +1676,9 @@ public function getColumnTypes(): array protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string { $def = ''; - if ($foreignKey->getName()) { - $def .= ' CONSTRAINT ' . $this->quoteColumnName((string)$foreignKey->getName()); + $name = $foreignKey->getName(); + if ($name) { + $def .= ' CONSTRAINT ' . $this->quoteColumnName($name); } $columnNames = []; foreach ($foreignKey->getColumns() ?? [] as $column) { diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index d2ebd336..054f7145 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -839,9 +839,10 @@ protected function getIndexSqlDefinition(Index $index, string $tableName): strin $include = $index->getInclude(); $includedColumns = $include ? sprintf(' INCLUDE ([%s])', implode('],[', $include)) : ''; - $where = (string)$index->getWhere(); - if ($where) { - $where = ' WHERE ' . $where; + $where = ''; + $whereClause = $index->getWhere(); + if ($whereClause) { + $where = ' WHERE ' . $whereClause; } return sprintf( From 84e4c4ff06f1d810ab2ffd37bc00eef66c1470b4 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 16:51:46 -0500 Subject: [PATCH 17/21] Throw on null for required Column::getName() and FK::getReferencedTable() Replace (string) casts with explicit null checks and InvalidArgumentException throws for values that must be present: Column::getName() in SQL generation and ForeignKey::getReferencedTable() in FK definition builders. A column without a name or a foreign key without a referenced table are always programming errors that should fail fast rather than silently produce broken SQL. --- src/Db/Action/ChangeColumn.php | 2 +- src/Db/Adapter/AbstractAdapter.php | 12 ++++++++-- src/Db/Adapter/MysqlAdapter.php | 8 +++++-- src/Db/Adapter/PostgresAdapter.php | 21 ++++++++++++---- src/Db/Adapter/RecordingAdapter.php | 6 ++++- src/Db/Adapter/SqliteAdapter.php | 11 +++++++-- src/Db/Adapter/SqlserverAdapter.php | 37 ++++++++++++++++++++++------- src/Db/Table.php | 21 ++++++++++++---- 8 files changed, 93 insertions(+), 25 deletions(-) diff --git a/src/Db/Action/ChangeColumn.php b/src/Db/Action/ChangeColumn.php index 267e30aa..ce53e73e 100644 --- a/src/Db/Action/ChangeColumn.php +++ b/src/Db/Action/ChangeColumn.php @@ -41,7 +41,7 @@ public function __construct(TableMetadata $table, string $columnName, Column $co $this->column = $column; // if the name was omitted use the existing column name - if ($column->getName() === null || strlen((string)$column->getName()) === 0) { + if ($column->getName() === null || strlen($column->getName()) === 0) { $column->setName($columnName); } } diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index 709504aa..81f78aaa 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -1723,17 +1723,25 @@ public function executeActions(TableMetadata $table, array $actions): void case $action instanceof RemoveColumn: /** @var \Migrations\Db\Action\RemoveColumn $action */ + $columnName = $action->getColumn()->getName(); + if ($columnName === null) { + throw new InvalidArgumentException('Column name must be set.'); + } $instructions->merge($this->getDropColumnInstructions( $table->getName(), - (string)$action->getColumn()->getName(), + $columnName, )); break; case $action instanceof RenameColumn: /** @var \Migrations\Db\Action\RenameColumn $action */ + $columnName = $action->getColumn()->getName(); + if ($columnName === null) { + throw new InvalidArgumentException('Column name must be set.'); + } $instructions->merge($this->getRenameColumnInstructions( $table->getName(), - (string)$action->getColumn()->getName(), + $columnName, $action->getNewName(), )); break; diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index ab6b9cf1..1d0dda66 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -727,7 +727,7 @@ protected function getRenameColumnInstructions(string $tableName, string $column $targetColumn = null; foreach ($columns as $column) { - if (strcasecmp((string)$column->getName(), $columnName) === 0) { + if ($column->getName() !== null && strcasecmp($column->getName(), $columnName) === 0) { $targetColumn = $column; break; } @@ -1210,7 +1210,11 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string foreach ($foreignKey->getReferencedColumns() as $column) { $refColumnNames[] = $this->quoteColumnName($column); } - $def .= ' REFERENCES ' . $this->quoteTableName((string)$foreignKey->getReferencedTable()) . ' (' . implode(',', $refColumnNames) . ')'; + $referencedTable = $foreignKey->getReferencedTable(); + if ($referencedTable === null) { + throw new InvalidArgumentException('Foreign key must have a referenced table.'); + } + $def .= ' REFERENCES ' . $this->quoteTableName($referencedTable) . ' (' . implode(',', $refColumnNames) . ')'; $onDelete = $foreignKey->getOnDelete(); if ($onDelete) { $def .= ' ON DELETE ' . $onDelete; diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index 3b02b598..7b4428c9 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -546,12 +546,16 @@ protected function getChangeColumnInstructions( } // rename column - if ($columnName !== $newColumn->getName()) { + $newColumnName = $newColumn->getName(); + if ($columnName !== $newColumnName) { + if ($newColumnName === null) { + throw new InvalidArgumentException('Column name must be set.'); + } $instructions->addPostStep(sprintf( 'ALTER TABLE %s RENAME COLUMN %s TO %s', $this->quoteTableName($tableName), $quotedColumnName, - $this->quoteColumnName((string)$newColumn->getName()), + $this->quoteColumnName($newColumnName), )); } @@ -873,6 +877,11 @@ public function dropDatabase($name): void */ protected function getColumnCommentSqlDefinition(Column $column, string $tableName): string { + $columnName = $column->getName(); + if ($columnName === null) { + throw new InvalidArgumentException('Column name must be set.'); + } + $comment = (string)$column->getComment(); // passing 'null' is to remove column comment $comment = strcasecmp($comment, 'NULL') !== 0 @@ -882,7 +891,7 @@ protected function getColumnCommentSqlDefinition(Column $column, string $tableNa return sprintf( 'COMMENT ON COLUMN %s.%s IS %s;', $this->quoteTableName($tableName), - $this->quoteColumnName((string)$column->getName()), + $this->quoteColumnName($columnName), $comment, ); } @@ -957,9 +966,13 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey, string $ta ); $columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns() ?? [])); $refColumnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getReferencedColumns())); + $referencedTable = $foreignKey->getReferencedTable(); + if ($referencedTable === null) { + throw new InvalidArgumentException('Foreign key must have a referenced table.'); + } $def = ' CONSTRAINT ' . $this->quoteColumnName($constraintName) . ' FOREIGN KEY (' . $columnList . ')' . - ' REFERENCES ' . $this->quoteTableName((string)$foreignKey->getReferencedTable()) . ' (' . $refColumnList . ')'; + ' REFERENCES ' . $this->quoteTableName($referencedTable) . ' (' . $refColumnList . ')'; if ($foreignKey->getOnDelete()) { $def .= " ON DELETE {$foreignKey->getOnDelete()}"; } diff --git a/src/Db/Adapter/RecordingAdapter.php b/src/Db/Adapter/RecordingAdapter.php index 00054a01..4c0e90f9 100644 --- a/src/Db/Adapter/RecordingAdapter.php +++ b/src/Db/Adapter/RecordingAdapter.php @@ -13,6 +13,7 @@ use Migrations\Db\Action\AddForeignKey; use Migrations\Db\Action\AddIndex; use Migrations\Db\Action\CreateTable; +use InvalidArgumentException; use Migrations\Db\Action\DropForeignKey; use Migrations\Db\Action\DropIndex; use Migrations\Db\Action\DropTable; @@ -90,7 +91,10 @@ public function getInvertedCommands(): Intent case $command instanceof RenameColumn: /** @var \Migrations\Db\Action\RenameColumn $command */ $column = clone $command->getColumn(); - $name = (string)$column->getName(); + $name = $column->getName(); + if ($name === null) { + throw new InvalidArgumentException('Column name must be set.'); + } $column->setName($command->getNewName()); $inverted->addAction(new RenameColumn($command->getTable(), $column, $name)); break; diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php index c24e3665..957d8d8d 100644 --- a/src/Db/Adapter/SqliteAdapter.php +++ b/src/Db/Adapter/SqliteAdapter.php @@ -1112,7 +1112,10 @@ protected function getChangeColumnInstructions(string $tableName, string $column { $instructions = $this->beginAlterByCopyTable($tableName); - $newColumnName = (string)$newColumn->getName(); + $newColumnName = $newColumn->getName(); + if ($newColumnName === null) { + throw new InvalidArgumentException('Column name must be set.'); + } $instructions->addPostStep(function ($state) use ($columnName, $newColumn) { $dialect = $this->getSchemaDialect(); $sql = (string)preg_replace( @@ -1689,7 +1692,11 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string foreach ($foreignKey->getReferencedColumns() as $column) { $refColumnNames[] = $this->quoteColumnName($column); } - $def .= ' REFERENCES ' . $this->quoteTableName((string)$foreignKey->getReferencedTable()) . ' (' . implode(',', $refColumnNames) . ')'; + $referencedTable = $foreignKey->getReferencedTable(); + if ($referencedTable === null) { + throw new InvalidArgumentException('Foreign key must have a referenced table.'); + } + $def .= ' REFERENCES ' . $this->quoteTableName($referencedTable) . ' (' . implode(',', $refColumnNames) . ')'; if ($foreignKey->getOnDelete()) { $def .= ' ON DELETE ' . $foreignKey->getOnDelete(); } diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index 054f7145..996d65bd 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -213,8 +213,16 @@ protected function getChangeCommentInstructions(TableMetadata $table, ?string $n */ protected function getColumnCommentSqlDefinition(Column $column, ?string $tableName): string { + $columnName = $column->getName(); + if ($columnName === null) { + throw new InvalidArgumentException('Column name must be set.'); + } + if ($tableName === null) { + throw new InvalidArgumentException('Table name must be set.'); + } + // passing 'null' is to remove column comment - $currentComment = $this->getColumnComment((string)$tableName, $column->getName()); + $currentComment = $this->getColumnComment($tableName, $columnName); $comment = strcasecmp((string)$column->getComment(), 'NULL') !== 0 ? $this->quoteString((string)$column->getComment()) : '\'\''; $command = $currentComment === null ? 'sp_addextendedproperty' : 'sp_updateextendedproperty'; @@ -224,8 +232,8 @@ protected function getColumnCommentSqlDefinition(Column $column, ?string $tableN $command, $comment, $this->schema, - (string)$tableName, - (string)$column->getName(), + $tableName, + $columnName, ); } @@ -420,12 +428,16 @@ protected function getChangeDefault(string $tableName, Column $newColumn): Alter return $instructions; } + $newColumnName = $newColumn->getName(); + if ($newColumnName === null) { + throw new InvalidArgumentException('Column name must be set.'); + } $instructions->addPostStep(sprintf( 'ALTER TABLE %s ADD CONSTRAINT %s %s FOR %s', $this->quoteTableName($tableName), $constraintName, $default, - $this->quoteColumnName((string)$newColumn->getName()), + $this->quoteColumnName($newColumnName), )); return $instructions; @@ -455,14 +467,19 @@ protected function getChangeColumnInstructions(string $tableName, string $column $instructions = new AlterInstructions(); $dialect = $this->getSchemaDialect(); - if ($columnName !== $newColumn->getName()) { + $newColumnName = $newColumn->getName(); + if ($newColumnName === null) { + throw new InvalidArgumentException('Column name must be set.'); + } + + if ($columnName !== $newColumnName) { $instructions->merge( - $this->getRenameColumnInstructions($tableName, $columnName, (string)$newColumn->getName()), + $this->getRenameColumnInstructions($tableName, $columnName, $newColumnName), ); } if ($changeDefault) { - $instructions->merge($this->getDropDefaultConstraint($tableName, (string)$newColumn->getName())); + $instructions->merge($this->getDropDefaultConstraint($tableName, $newColumnName)); } // Sqlserver doesn't support defaults @@ -871,7 +888,11 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey, string $ta $def = ' CONSTRAINT ' . $this->quoteColumnName($constraintName); $def .= ' FOREIGN KEY (' . $columnList . ')'; - $def .= ' REFERENCES ' . $this->quoteTableName((string)$foreignKey->getReferencedTable()) . ' (' . $refColumnList . ')'; + $referencedTable = $foreignKey->getReferencedTable(); + if ($referencedTable === null) { + throw new InvalidArgumentException('Foreign key must have a referenced table.'); + } + $def .= ' REFERENCES ' . $this->quoteTableName($referencedTable) . ' (' . $refColumnList . ')'; if ($foreignKey->getOnDelete()) { $def .= " ON DELETE {$foreignKey->getOnDelete()}"; } diff --git a/src/Db/Table.php b/src/Db/Table.php index cb026b0e..de77eb81 100644 --- a/src/Db/Table.php +++ b/src/Db/Table.php @@ -367,11 +367,16 @@ public function addColumn(string|Column $columnName, ?string $type = null, array } // Delegate to Adapters to check column type - if (!$this->getAdapter()->isValidColumnType($action->getColumn())) { + $column = $action->getColumn(); + $colName = $column->getName(); + if ($colName === null) { + throw new InvalidArgumentException('Column name must be set.'); + } + if (!$this->getAdapter()->isValidColumnType($column)) { throw new InvalidArgumentException(sprintf( 'An invalid column type "%s" was specified for column "%s".', - (string)$action->getColumn()->getType(), - (string)$action->getColumn()->getName(), + $column->getType(), + $colName, )); } @@ -903,7 +908,9 @@ protected function filterPrimaryKey(array $options): void return $action->getColumn(); }); $primaryKeyColumns = $columnsCollection->filter(function (Column $columnDef, $key) use ($primaryKey) { - return isset($primaryKey[(string)$columnDef->getName()]); + $name = $columnDef->getName(); + + return $name !== null && isset($primaryKey[$name]); })->toArray(); if (!$primaryKeyColumns) { @@ -912,7 +919,11 @@ protected function filterPrimaryKey(array $options): void foreach ($primaryKeyColumns as $primaryKeyColumn) { if ($primaryKeyColumn->isIdentity()) { - unset($primaryKey[(string)$primaryKeyColumn->getName()]); + $pkColName = $primaryKeyColumn->getName(); + if ($pkColName === null) { + continue; + } + unset($primaryKey[$pkColName]); } } From 14b239ae5683591bd56449d5eb0ee595ea1d5355 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 16:56:26 -0400 Subject: [PATCH 18/21] Narrow ForeignKey::getColumns() return type and remove null guards Override getColumns() in Migrations ForeignKey to return array instead of the parent's ?array. The $columns property is always initialized as [] in the constructor, making null unreachable. This is a covariant return type narrowing, same pattern used for Column::getNull(). Removes the now-unnecessary ?? [] fallbacks from all 6 call sites across the adapter and plan classes. --- src/Db/Adapter/AbstractAdapter.php | 2 +- src/Db/Adapter/MysqlAdapter.php | 2 +- src/Db/Adapter/PostgresAdapter.php | 4 ++-- src/Db/Adapter/SqliteAdapter.php | 2 +- src/Db/Adapter/SqlserverAdapter.php | 4 ++-- src/Db/Plan/Plan.php | 2 +- src/Db/Table/ForeignKey.php | 13 +++++++++++++ tests/TestCase/Db/Table/ForeignKeyTest.php | 1 + 8 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index 81f78aaa..d9d3b927 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -1684,7 +1684,7 @@ public function executeActions(TableMetadata $table, array $actions): void /** @var \Migrations\Db\Action\DropForeignKey $action */ $instructions->merge($this->getDropForeignKeyByColumnsInstructions( $table->getName(), - $action->getForeignKey()->getColumns() ?? [], + $action->getForeignKey()->getColumns(), )); break; diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 1d0dda66..c1b4abcb 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -1202,7 +1202,7 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string $def .= ' CONSTRAINT ' . $this->quoteColumnName($name); } $columnNames = []; - foreach ($foreignKey->getColumns() ?? [] as $column) { + foreach ($foreignKey->getColumns() as $column) { $columnNames[] = $this->quoteColumnName($column); } $def .= ' FOREIGN KEY (' . implode(',', $columnNames) . ')'; diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index 7b4428c9..f20aafe3 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -962,9 +962,9 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey, string $ta $parts = $this->getSchemaName($tableName); $constraintName = $foreignKey->getName() ?: ( - $parts['table'] . '_' . implode('_', $foreignKey->getColumns() ?? []) . '_fkey' + $parts['table'] . '_' . implode('_', $foreignKey->getColumns()) . '_fkey' ); - $columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns() ?? [])); + $columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns())); $refColumnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getReferencedColumns())); $referencedTable = $foreignKey->getReferencedTable(); if ($referencedTable === null) { diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php index 957d8d8d..e45c1889 100644 --- a/src/Db/Adapter/SqliteAdapter.php +++ b/src/Db/Adapter/SqliteAdapter.php @@ -1684,7 +1684,7 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string $def .= ' CONSTRAINT ' . $this->quoteColumnName($name); } $columnNames = []; - foreach ($foreignKey->getColumns() ?? [] as $column) { + foreach ($foreignKey->getColumns() as $column) { $columnNames[] = $this->quoteColumnName($column); } $def .= ' FOREIGN KEY (' . implode(',', $columnNames) . ')'; diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index 996d65bd..519c2245 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -882,8 +882,8 @@ protected function getIndexSqlDefinition(Index $index, string $tableName): strin */ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey, string $tableName): string { - $constraintName = $foreignKey->getName() ?: $tableName . '_' . implode('_', $foreignKey->getColumns() ?? []); - $columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns() ?? [])); + $constraintName = $foreignKey->getName() ?: $tableName . '_' . implode('_', $foreignKey->getColumns()); + $columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns())); $refColumnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getReferencedColumns())); $def = ' CONSTRAINT ' . $this->quoteColumnName($constraintName); diff --git a/src/Db/Plan/Plan.php b/src/Db/Plan/Plan.php index 8969ebef..dcbaa718 100644 --- a/src/Db/Plan/Plan.php +++ b/src/Db/Plan/Plan.php @@ -284,7 +284,7 @@ protected function remapContraintAndIndexConflicts(AlterTable $alter): AlterTabl if ($action instanceof DropForeignKey) { [$this->indexes, $dropIndexActions] = $this->forgetDropIndex( $action->getTable(), - $action->getForeignKey()->getColumns() ?? [], + $action->getForeignKey()->getColumns(), $this->indexes, ); foreach ($dropIndexActions as $dropIndexAction) { diff --git a/src/Db/Table/ForeignKey.php b/src/Db/Table/ForeignKey.php index 929930a9..425b9f68 100644 --- a/src/Db/Table/ForeignKey.php +++ b/src/Db/Table/ForeignKey.php @@ -86,6 +86,19 @@ public function __construct( } } + /** + * {@inheritDoc} + * + * Narrows the return type from the parent's ?array to array, + * since $columns is always initialized as [] in this class. + * + * @return array + */ + public function getColumns(): array + { + return $this->columns; + } + /** * Utility method that maps an array of index options to this object's methods. * diff --git a/tests/TestCase/Db/Table/ForeignKeyTest.php b/tests/TestCase/Db/Table/ForeignKeyTest.php index c2fce98a..6ce31b66 100644 --- a/tests/TestCase/Db/Table/ForeignKeyTest.php +++ b/tests/TestCase/Db/Table/ForeignKeyTest.php @@ -151,4 +151,5 @@ public function testThrowsErrorForInvalidDeferrableValue(): void $this->expectException(InvalidArgumentException::class); $this->fk->setDeferrableMode('invalid_value'); } + } From fd66adfb4ef63528736895364e82d84a7733d63d Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 22:03:21 -0400 Subject: [PATCH 19/21] Narrow Column::getName() return type and remove dead null checks Override getName() in Migrations Column to return string instead of the parent's ?string. The $name property is typed as string (not ?string) in the constructor, making null unreachable. This is a covariant return type narrowing, same pattern used for ForeignKey::getColumns(). Removes now-unnecessary null checks across adapter/action files and the dead null guard in Table::setPrimaryKey(). --- src/Db/Action/ChangeColumn.php | 2 +- src/Db/Adapter/AbstractAdapter.php | 12 ++---------- src/Db/Adapter/MysqlAdapter.php | 2 +- src/Db/Adapter/PostgresAdapter.php | 7 ------- src/Db/Adapter/RecordingAdapter.php | 4 ---- src/Db/Adapter/SqliteAdapter.php | 3 --- src/Db/Adapter/SqlserverAdapter.php | 10 ---------- src/Db/Table.php | 16 +++------------- src/Db/Table/Column.php | 7 +++++-- 9 files changed, 12 insertions(+), 51 deletions(-) diff --git a/src/Db/Action/ChangeColumn.php b/src/Db/Action/ChangeColumn.php index ce53e73e..3fd96033 100644 --- a/src/Db/Action/ChangeColumn.php +++ b/src/Db/Action/ChangeColumn.php @@ -41,7 +41,7 @@ public function __construct(TableMetadata $table, string $columnName, Column $co $this->column = $column; // if the name was omitted use the existing column name - if ($column->getName() === null || strlen($column->getName()) === 0) { + if (strlen($column->getName()) === 0) { $column->setName($columnName); } } diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index d9d3b927..fd82cbdf 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -1723,25 +1723,17 @@ public function executeActions(TableMetadata $table, array $actions): void case $action instanceof RemoveColumn: /** @var \Migrations\Db\Action\RemoveColumn $action */ - $columnName = $action->getColumn()->getName(); - if ($columnName === null) { - throw new InvalidArgumentException('Column name must be set.'); - } $instructions->merge($this->getDropColumnInstructions( $table->getName(), - $columnName, + $action->getColumn()->getName(), )); break; case $action instanceof RenameColumn: /** @var \Migrations\Db\Action\RenameColumn $action */ - $columnName = $action->getColumn()->getName(); - if ($columnName === null) { - throw new InvalidArgumentException('Column name must be set.'); - } $instructions->merge($this->getRenameColumnInstructions( $table->getName(), - $columnName, + $action->getColumn()->getName(), $action->getNewName(), )); break; diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index c1b4abcb..6dd6c0ba 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -727,7 +727,7 @@ protected function getRenameColumnInstructions(string $tableName, string $column $targetColumn = null; foreach ($columns as $column) { - if ($column->getName() !== null && strcasecmp($column->getName(), $columnName) === 0) { + if (strcasecmp($column->getName(), $columnName) === 0) { $targetColumn = $column; break; } diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index f20aafe3..f349767c 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -548,9 +548,6 @@ protected function getChangeColumnInstructions( // rename column $newColumnName = $newColumn->getName(); if ($columnName !== $newColumnName) { - if ($newColumnName === null) { - throw new InvalidArgumentException('Column name must be set.'); - } $instructions->addPostStep(sprintf( 'ALTER TABLE %s RENAME COLUMN %s TO %s', $this->quoteTableName($tableName), @@ -878,10 +875,6 @@ public function dropDatabase($name): void protected function getColumnCommentSqlDefinition(Column $column, string $tableName): string { $columnName = $column->getName(); - if ($columnName === null) { - throw new InvalidArgumentException('Column name must be set.'); - } - $comment = (string)$column->getComment(); // passing 'null' is to remove column comment $comment = strcasecmp($comment, 'NULL') !== 0 diff --git a/src/Db/Adapter/RecordingAdapter.php b/src/Db/Adapter/RecordingAdapter.php index 4c0e90f9..6eb493e2 100644 --- a/src/Db/Adapter/RecordingAdapter.php +++ b/src/Db/Adapter/RecordingAdapter.php @@ -8,7 +8,6 @@ namespace Migrations\Db\Adapter; -use InvalidArgumentException; use Migrations\Db\Action\AddColumn; use Migrations\Db\Action\AddForeignKey; use Migrations\Db\Action\AddIndex; @@ -92,9 +91,6 @@ public function getInvertedCommands(): Intent /** @var \Migrations\Db\Action\RenameColumn $command */ $column = clone $command->getColumn(); $name = $column->getName(); - if ($name === null) { - throw new InvalidArgumentException('Column name must be set.'); - } $column->setName($command->getNewName()); $inverted->addAction(new RenameColumn($command->getTable(), $column, $name)); break; diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php index e45c1889..2cd6894d 100644 --- a/src/Db/Adapter/SqliteAdapter.php +++ b/src/Db/Adapter/SqliteAdapter.php @@ -1113,9 +1113,6 @@ protected function getChangeColumnInstructions(string $tableName, string $column $instructions = $this->beginAlterByCopyTable($tableName); $newColumnName = $newColumn->getName(); - if ($newColumnName === null) { - throw new InvalidArgumentException('Column name must be set.'); - } $instructions->addPostStep(function ($state) use ($columnName, $newColumn) { $dialect = $this->getSchemaDialect(); $sql = (string)preg_replace( diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index 519c2245..43ede29c 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -214,9 +214,6 @@ protected function getChangeCommentInstructions(TableMetadata $table, ?string $n protected function getColumnCommentSqlDefinition(Column $column, ?string $tableName): string { $columnName = $column->getName(); - if ($columnName === null) { - throw new InvalidArgumentException('Column name must be set.'); - } if ($tableName === null) { throw new InvalidArgumentException('Table name must be set.'); } @@ -429,9 +426,6 @@ protected function getChangeDefault(string $tableName, Column $newColumn): Alter } $newColumnName = $newColumn->getName(); - if ($newColumnName === null) { - throw new InvalidArgumentException('Column name must be set.'); - } $instructions->addPostStep(sprintf( 'ALTER TABLE %s ADD CONSTRAINT %s %s FOR %s', $this->quoteTableName($tableName), @@ -468,10 +462,6 @@ protected function getChangeColumnInstructions(string $tableName, string $column $dialect = $this->getSchemaDialect(); $newColumnName = $newColumn->getName(); - if ($newColumnName === null) { - throw new InvalidArgumentException('Column name must be set.'); - } - if ($columnName !== $newColumnName) { $instructions->merge( $this->getRenameColumnInstructions($tableName, $columnName, $newColumnName), diff --git a/src/Db/Table.php b/src/Db/Table.php index de77eb81..c83d7fd5 100644 --- a/src/Db/Table.php +++ b/src/Db/Table.php @@ -368,15 +368,11 @@ public function addColumn(string|Column $columnName, ?string $type = null, array // Delegate to Adapters to check column type $column = $action->getColumn(); - $colName = $column->getName(); - if ($colName === null) { - throw new InvalidArgumentException('Column name must be set.'); - } if (!$this->getAdapter()->isValidColumnType($column)) { throw new InvalidArgumentException(sprintf( 'An invalid column type "%s" was specified for column "%s".', $column->getType(), - $colName, + $column->getName(), )); } @@ -908,9 +904,7 @@ protected function filterPrimaryKey(array $options): void return $action->getColumn(); }); $primaryKeyColumns = $columnsCollection->filter(function (Column $columnDef, $key) use ($primaryKey) { - $name = $columnDef->getName(); - - return $name !== null && isset($primaryKey[$name]); + return isset($primaryKey[$columnDef->getName()]); })->toArray(); if (!$primaryKeyColumns) { @@ -919,11 +913,7 @@ protected function filterPrimaryKey(array $options): void foreach ($primaryKeyColumns as $primaryKeyColumn) { if ($primaryKeyColumn->isIdentity()) { - $pkColName = $primaryKeyColumn->getName(); - if ($pkColName === null) { - continue; - } - unset($primaryKey[$pkColName]); + unset($primaryKey[$primaryKeyColumn->getName()]); } } diff --git a/src/Db/Table/Column.php b/src/Db/Table/Column.php index 698a79b1..7e2004dc 100644 --- a/src/Db/Table/Column.php +++ b/src/Db/Table/Column.php @@ -182,9 +182,12 @@ public function setName(string $name) /** * Gets the column name. * - * @return string|null + * Narrows the return type from the parent's ?string to string, + * since $name is typed as string (not ?string) in this class. + * + * @return string */ - public function getName(): ?string + public function getName(): string { return $this->name; } From 7bb0e023f6670961cd9b3da49d6244fe7659a300 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 22:38:53 -0400 Subject: [PATCH 20/21] Add null guards for column diff loop in BakeMigrationDiffCommand Use early-continue pattern to narrow nullable safeGetColumn() return types before array operations, fixing PHPStan level 8 errors. --- src/Command/BakeMigrationDiffCommand.php | 26 ++++++++++++++---------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/Command/BakeMigrationDiffCommand.php b/src/Command/BakeMigrationDiffCommand.php index e205bf71..bf4c2d3a 100644 --- a/src/Command/BakeMigrationDiffCommand.php +++ b/src/Command/BakeMigrationDiffCommand.php @@ -279,23 +279,27 @@ protected function getColumns(): void // changes in columns meta-data foreach ($currentColumns as $columnName) { $column = $this->safeGetColumn($currentSchema, $columnName); + if ($column === null) { + continue; + } + + if (!in_array($columnName, $oldColumns, true)) { + continue; + } + $oldColumn = $this->safeGetColumn($this->dumpSchema[$table], $columnName); + if ($oldColumn === null) { + continue; + } + unset( $column['collate'], $column['fixed'], + $oldColumn['collate'], + $oldColumn['fixed'], ); - if ($oldColumn !== null) { - unset( - $oldColumn['collate'], - $oldColumn['fixed'], - ); - } - if ( - in_array($columnName, $oldColumns, true) && - $oldColumn !== null && - $column !== $oldColumn - ) { + if ($column !== $oldColumn) { $changedAttributes = array_diff_assoc($column, $oldColumn); foreach (['type', 'length', 'null', 'default'] as $attribute) { From 458235012159de3a2bbcfe8b3d0b994434b3b49a Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 22:53:33 -0400 Subject: [PATCH 21/21] Cast nullable getName() to string for drop FK/index instructions --- src/Db/Adapter/AbstractAdapter.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index fd82cbdf..367e89cc 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -1690,7 +1690,7 @@ public function executeActions(TableMetadata $table, array $actions): void case $action instanceof DropForeignKey && $action->getForeignKey()->getName(): /** @var \Migrations\Db\Action\DropForeignKey $action */ - $fkName = $action->getForeignKey()->getName(); + $fkName = (string)$action->getForeignKey()->getName(); $instructions->merge($this->getDropForeignKeyInstructions( $table->getName(), $fkName, @@ -1699,7 +1699,7 @@ public function executeActions(TableMetadata $table, array $actions): void case $action instanceof DropIndex && $action->getIndex()->getName(): /** @var \Migrations\Db\Action\DropIndex $action */ - $indexName = $action->getIndex()->getName(); + $indexName = (string)$action->getIndex()->getName(); $instructions->merge($this->getDropIndexByNameInstructions( $table->getName(), $indexName,