From 7c95aeacbc889c53b98ad4ad927bb4b1ce03462c Mon Sep 17 00:00:00 2001 From: Matthias Wirtz Date: Tue, 3 Mar 2026 05:06:21 +0100 Subject: [PATCH 01/24] 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 a492413e0ae3b63f28bd11c5752fc0a60d9f396b Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Thu, 5 Mar 2026 01:10:09 -0500 Subject: [PATCH 02/24] 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 55ea1a026734a6ebc21753ee657333dc67935f6a Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Thu, 5 Mar 2026 01:13:22 -0500 Subject: [PATCH 03/24] 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 52b91cf6a0ed17dcb9167384e4a19194a5ccf1cc Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sat, 7 Mar 2026 05:00:26 +0100 Subject: [PATCH 04/24] 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 9486d809a88141894f48a9c78db10c57451f5c00 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sat, 7 Mar 2026 19:21:57 +0100 Subject: [PATCH 05/24] 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 4c4fb535c1e8ee9143c3ab896c016cd4942339e4 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sat, 7 Mar 2026 19:34:14 +0100 Subject: [PATCH 06/24] 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 b656599b0969c7ede2977c023e45cacd31891482 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Sat, 7 Mar 2026 19:38:22 +0100 Subject: [PATCH 07/24] 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 2d358abc4864664d49f115f3ade06f3369d80ac7 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 10:44:52 -0500 Subject: [PATCH 08/24] 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 bc6d6285c48ad399fb2ac44ed652c5d102a0b0cf Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 11:00:56 -0500 Subject: [PATCH 09/24] 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 a97592f0b163f9755c2cdfb0f2524f8e4d4de460 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 21:57:44 -0400 Subject: [PATCH 10/24] 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 5ddb790f923b18ee2440d5eda713dcd1aa006e43 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 13:42:26 -0500 Subject: [PATCH 11/24] 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 7a17b4685821d9fde59fdba13e54774bd22746ca Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 13:42:47 -0500 Subject: [PATCH 12/24] 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 6f475683ea797f41a64fb6925bf96864fff734b9 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 21:58:28 -0400 Subject: [PATCH 13/24] 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 2e5c1cd26477193410b0fe399fc95b43766420be Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 15:41:42 -0500 Subject: [PATCH 14/24] 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 c93988e527de860cffe02c78dd4f88c843fe0835 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 22:02:35 -0400 Subject: [PATCH 15/24] 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 a5e25f3551e0f9268d14128c7711e10176fc092f Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 24 Feb 2026 16:51:46 -0500 Subject: [PATCH 16/24] 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 61d71c219907b22934626661e8206d28eace7ce0 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 16:56:26 -0400 Subject: [PATCH 17/24] 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 728ed6b846e1c0face4159a57cc8876b2b489296 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 22:03:21 -0400 Subject: [PATCH 18/24] 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 55c53d6a2b72724d28b33577452013906a78a44e Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 22:38:53 -0400 Subject: [PATCH 19/24] 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 90e0c1721abe0a45645f3483e0e6d20dbf963b90 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Tue, 10 Mar 2026 22:53:33 -0400 Subject: [PATCH 20/24] 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, From c5e69fc1fe92cf034b63047592c392ca314b132a Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Wed, 11 Mar 2026 09:03:37 -0400 Subject: [PATCH 21/24] Fix phpcs coding standard violations Remove unused InvalidArgumentException import from RecordingAdapter, extract assignment out of if condition in PostgresAdapter, and remove blank line before closing brace in ForeignKeyTest. --- src/Db/Adapter/PostgresAdapter.php | 3 ++- src/Db/Adapter/RecordingAdapter.php | 1 - tests/TestCase/Db/Table/ForeignKeyTest.php | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index f349767c..899c1ee9 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -505,7 +505,8 @@ protected function getChangeColumnInstructions( 'ALTER COLUMN %s', $quotedColumnName, ); - if ($newColumn->isIdentity() && ($generated = $newColumn->getGenerated()) !== null) { + $generated = $newColumn->getGenerated(); + if ($newColumn->isIdentity() && $generated !== null) { if ($column->isIdentity()) { $sql .= sprintf(' SET GENERATED %s', $generated); } else { diff --git a/src/Db/Adapter/RecordingAdapter.php b/src/Db/Adapter/RecordingAdapter.php index 6eb493e2..9f1e90b8 100644 --- a/src/Db/Adapter/RecordingAdapter.php +++ b/src/Db/Adapter/RecordingAdapter.php @@ -12,7 +12,6 @@ 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; diff --git a/tests/TestCase/Db/Table/ForeignKeyTest.php b/tests/TestCase/Db/Table/ForeignKeyTest.php index 6ce31b66..c2fce98a 100644 --- a/tests/TestCase/Db/Table/ForeignKeyTest.php +++ b/tests/TestCase/Db/Table/ForeignKeyTest.php @@ -151,5 +151,4 @@ public function testThrowsErrorForInvalidDeferrableValue(): void $this->expectException(InvalidArgumentException::class); $this->fk->setDeferrableMode('invalid_value'); } - } From e9045c9a4923eed9de61636415c338457cf6f976 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Wed, 11 Mar 2026 15:51:36 -0400 Subject: [PATCH 22/24] Consolidate null guard conditions in BakeMigrationDiffCommand --- src/Command/BakeMigrationDiffCommand.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Command/BakeMigrationDiffCommand.php b/src/Command/BakeMigrationDiffCommand.php index bf4c2d3a..a67ef3c5 100644 --- a/src/Command/BakeMigrationDiffCommand.php +++ b/src/Command/BakeMigrationDiffCommand.php @@ -279,11 +279,7 @@ 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)) { + if ($column === null || !in_array($columnName, $oldColumns, true)) { continue; } From 611e1bbd0940516f6a23b20a831775d8035931a2 Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Wed, 11 Mar 2026 15:51:43 -0400 Subject: [PATCH 23/24] Fix inverted condition in ChangeColumn name fallback --- src/Db/Action/ChangeColumn.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Db/Action/ChangeColumn.php b/src/Db/Action/ChangeColumn.php index 3fd96033..f4939ebd 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 (strlen($column->getName()) === 0) { + if (!$column->getName()) { $column->setName($columnName); } } From 1d4bf946d9183868d12b9ac1f2b2d6588015843a Mon Sep 17 00:00:00 2001 From: Jamison Bryant Date: Wed, 11 Mar 2026 15:51:57 -0400 Subject: [PATCH 24/24] Accept null in Column::setFixed() for snapshot compatibility Generated migration snapshots can emit 'fixed' => null for binary columns, which causes a TypeError since setFixed() only accepted bool. Widen the parameter to ?bool to match the property and getter types. Fixes #1046. --- src/Db/Table/Column.php | 4 ++-- tests/TestCase/Db/Table/ColumnTest.php | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Db/Table/Column.php b/src/Db/Table/Column.php index 7e2004dc..750e74c4 100644 --- a/src/Db/Table/Column.php +++ b/src/Db/Table/Column.php @@ -785,10 +785,10 @@ public function getLock(): ?string * * When true, binary columns will use BINARY(n) instead of VARBINARY(n). * - * @param bool $fixed Fixed + * @param bool|null $fixed Fixed * @return $this */ - public function setFixed(bool $fixed) + public function setFixed(?bool $fixed) { $this->fixed = $fixed; diff --git a/tests/TestCase/Db/Table/ColumnTest.php b/tests/TestCase/Db/Table/ColumnTest.php index 2d5ccd1a..41af49f7 100644 --- a/tests/TestCase/Db/Table/ColumnTest.php +++ b/tests/TestCase/Db/Table/ColumnTest.php @@ -308,6 +308,17 @@ public function testSetOptionsWithFixed(): void $this->assertTrue($column->getFixed()); $this->assertSame(20, $column->getLimit()); + + // Null via setter should be accepted and preserved + $column2 = new Column(); + $column2->setName('data')->setType('binary')->setFixed(null); + $this->assertNull($column2->getFixed()); + + // Null via setOptions (as generated by migration snapshots) + $column3 = new Column(); + $column3->setName('data')->setType('binary'); + $column3->setOptions(['fixed' => null, 'limit' => null, 'null' => false]); + $this->assertNull($column3->getFixed()); } public function testToArrayIncludesFixed(): void