Skip to content

Commit 9cdbada

Browse files
jamisonbryantswifferdereuromarkJamison Bryant
authored
Raise PHPStan level to 8 with null safety fixes (#1044)
* add using when changing column type to json (#1031) * 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 * 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 * 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 * 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 * 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 * 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 * Bump PHPStan level +1 * Make props non-nullable * Add null guards for getConstraint() in BakeMigrationDiffCommand * 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(). * 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. * 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. * 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. * 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. * 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. * 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. * 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(). * 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. * Cast nullable getName() to string for drop FK/index instructions * 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. * Consolidate null guard conditions in BakeMigrationDiffCommand * Fix inverted condition in ChangeColumn name fallback * 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. --------- Co-authored-by: Matthias Wirtz <matthias.wirtz@hotmail.de> Co-authored-by: Mark Scherer <dereuromark@users.noreply.github.com> Co-authored-by: Jamison Bryant <jbryant@ticketsauce.com>
1 parent 66e53bd commit 9cdbada

20 files changed

+175
-82
lines changed

‎phpstan.neon‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ includes:
22
- phpstan-baseline.neon
33

44
parameters:
5-
level: 7
5+
level: 8
66
paths:
77
- src/
88
bootstrapFiles:

‎src/BaseSeed.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,8 @@ protected function runCall(string $seeder, array $options = []): void
278278

279279
$options += [
280280
'connection' => $connection,
281-
'plugin' => $pluginName ?? $config['plugin'],
282-
'source' => $config['source'],
281+
'plugin' => $pluginName ?? ($config !== null ? $config['plugin'] : null),
282+
'source' => $config !== null ? $config['source'] : null,
283283
];
284284
$factory = new ManagerFactory([
285285
'connection' => $options['connection'],

‎src/Command/BakeMigrationDiffCommand.php‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -279,18 +279,23 @@ protected function getColumns(): void
279279
// changes in columns meta-data
280280
foreach ($currentColumns as $columnName) {
281281
$column = $this->safeGetColumn($currentSchema, $columnName);
282+
if ($column === null || !in_array($columnName, $oldColumns, true)) {
283+
continue;
284+
}
285+
282286
$oldColumn = $this->safeGetColumn($this->dumpSchema[$table], $columnName);
287+
if ($oldColumn === null) {
288+
continue;
289+
}
290+
283291
unset(
284292
$column['collate'],
285293
$column['fixed'],
286294
$oldColumn['collate'],
287295
$oldColumn['fixed'],
288296
);
289297

290-
if (
291-
in_array($columnName, $oldColumns, true) &&
292-
$column !== $oldColumn
293-
) {
298+
if ($column !== $oldColumn) {
294299
$changedAttributes = array_diff_assoc($column, $oldColumn);
295300

296301
foreach (['type', 'length', 'null', 'default'] as $attribute) {
@@ -385,9 +390,10 @@ protected function getConstraints(): void
385390
// brand new constraints
386391
$addedConstraints = array_diff($currentConstraints, $oldConstraints);
387392
foreach ($addedConstraints as $constraintName) {
388-
$this->templateData[$table]['constraints']['add'][$constraintName] =
389-
$currentSchema->getConstraint($constraintName);
390393
$constraint = $currentSchema->getConstraint($constraintName);
394+
if ($constraint === null) {
395+
continue;
396+
}
391397
if ($constraint['type'] === TableSchema::CONSTRAINT_FOREIGN) {
392398
$this->templateData[$table]['constraints']['add'][$constraintName] = $constraint;
393399
} else {
@@ -399,13 +405,18 @@ protected function getConstraints(): void
399405
// if present in both, check if they are the same : if not, remove the old one and add the new one
400406
foreach ($currentConstraints as $constraintName) {
401407
$constraint = $currentSchema->getConstraint($constraintName);
408+
if ($constraint === null) {
409+
continue;
410+
}
402411

412+
$oldConstraint = $this->dumpSchema[$table]->getConstraint($constraintName);
403413
if (
404414
in_array($constraintName, $oldConstraints, true) &&
405-
$constraint !== $this->dumpSchema[$table]->getConstraint($constraintName)
415+
$constraint !== $oldConstraint
406416
) {
407-
$this->templateData[$table]['constraints']['remove'][$constraintName] =
408-
$this->dumpSchema[$table]->getConstraint($constraintName);
417+
if ($oldConstraint !== null) {
418+
$this->templateData[$table]['constraints']['remove'][$constraintName] = $oldConstraint;
419+
}
409420
$this->templateData[$table]['constraints']['add'][$constraintName] =
410421
$constraint;
411422
}
@@ -415,6 +426,9 @@ protected function getConstraints(): void
415426
$removedConstraints = array_diff($oldConstraints, $currentConstraints);
416427
foreach ($removedConstraints as $constraintName) {
417428
$constraint = $this->dumpSchema[$table]->getConstraint($constraintName);
429+
if ($constraint === null) {
430+
continue;
431+
}
418432
if ($constraint['type'] === TableSchema::CONSTRAINT_FOREIGN) {
419433
$this->templateData[$table]['constraints']['remove'][$constraintName] = $constraint;
420434
} else {

‎src/Command/BakeSimpleMigrationCommand.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,16 @@ abstract class BakeSimpleMigrationCommand extends SimpleBakeCommand
5252
/**
5353
* Console IO
5454
*
55-
* @var \Cake\Console\ConsoleIo|null
55+
* @var \Cake\Console\ConsoleIo
5656
*/
57-
protected ?ConsoleIo $io = null;
57+
protected ConsoleIo $io;
5858

5959
/**
6060
* Arguments
6161
*
62-
* @var \Cake\Console\Arguments|null
62+
* @var \Cake\Console\Arguments
6363
*/
64-
protected ?Arguments $args = null;
64+
protected Arguments $args;
6565

6666
/**
6767
* @inheritDoc

‎src/Db/Action/ChangeColumn.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function __construct(TableMetadata $table, string $columnName, Column $co
4141
$this->column = $column;
4242

4343
// if the name was omitted use the existing column name
44-
if ($column->getName() === null || strlen((string)$column->getName()) === 0) {
44+
if (!$column->getName()) {
4545
$column->setName($columnName);
4646
}
4747
}

‎src/Db/Adapter/AbstractAdapter.php‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,9 @@ public function getConnection(): Connection
201201
$this->connection = $this->getOption('connection');
202202
$this->connect();
203203
}
204+
if ($this->connection === null) {
205+
throw new RuntimeException('Unable to establish database connection. Ensure a connection is configured.');
206+
}
204207

205208
return $this->connection;
206209
}
@@ -1687,17 +1690,19 @@ public function executeActions(TableMetadata $table, array $actions): void
16871690

16881691
case $action instanceof DropForeignKey && $action->getForeignKey()->getName():
16891692
/** @var \Migrations\Db\Action\DropForeignKey $action */
1693+
$fkName = (string)$action->getForeignKey()->getName();
16901694
$instructions->merge($this->getDropForeignKeyInstructions(
16911695
$table->getName(),
1692-
(string)$action->getForeignKey()->getName(),
1696+
$fkName,
16931697
));
16941698
break;
16951699

16961700
case $action instanceof DropIndex && $action->getIndex()->getName():
16971701
/** @var \Migrations\Db\Action\DropIndex $action */
1702+
$indexName = (string)$action->getIndex()->getName();
16981703
$instructions->merge($this->getDropIndexByNameInstructions(
16991704
$table->getName(),
1700-
(string)$action->getIndex()->getName(),
1705+
$indexName,
17011706
));
17021707
break;
17031708

@@ -1720,15 +1725,15 @@ public function executeActions(TableMetadata $table, array $actions): void
17201725
/** @var \Migrations\Db\Action\RemoveColumn $action */
17211726
$instructions->merge($this->getDropColumnInstructions(
17221727
$table->getName(),
1723-
(string)$action->getColumn()->getName(),
1728+
$action->getColumn()->getName(),
17241729
));
17251730
break;
17261731

17271732
case $action instanceof RenameColumn:
17281733
/** @var \Migrations\Db\Action\RenameColumn $action */
17291734
$instructions->merge($this->getRenameColumnInstructions(
17301735
$table->getName(),
1731-
(string)$action->getColumn()->getName(),
1736+
$action->getColumn()->getName(),
17321737
$action->getNewName(),
17331738
));
17341739
break;

‎src/Db/Adapter/MysqlAdapter.php‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,8 +1197,9 @@ protected function getIndexSqlDefinition(Index $index): string
11971197
protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string
11981198
{
11991199
$def = '';
1200-
if ($foreignKey->getName()) {
1201-
$def .= ' CONSTRAINT ' . $this->quoteColumnName((string)$foreignKey->getName());
1200+
$name = $foreignKey->getName();
1201+
if ($name) {
1202+
$def .= ' CONSTRAINT ' . $this->quoteColumnName($name);
12021203
}
12031204
$columnNames = [];
12041205
foreach ($foreignKey->getColumns() as $column) {
@@ -1209,7 +1210,11 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey): string
12091210
foreach ($foreignKey->getReferencedColumns() as $column) {
12101211
$refColumnNames[] = $this->quoteColumnName($column);
12111212
}
1212-
$def .= ' REFERENCES ' . $this->quoteTableName($foreignKey->getReferencedTable()) . ' (' . implode(',', $refColumnNames) . ')';
1213+
$referencedTable = $foreignKey->getReferencedTable();
1214+
if ($referencedTable === null) {
1215+
throw new InvalidArgumentException('Foreign key must have a referenced table.');
1216+
}
1217+
$def .= ' REFERENCES ' . $this->quoteTableName($referencedTable) . ' (' . implode(',', $refColumnNames) . ')';
12131218
$onDelete = $foreignKey->getOnDelete();
12141219
if ($onDelete) {
12151220
$def .= ' ON DELETE ' . $onDelete;

‎src/Db/Adapter/PostgresAdapter.php‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -455,9 +455,9 @@ protected function getChangeColumnInstructions(
455455

456456
$columnSql = $dialect->columnDefinitionSql($this->mapColumnData($newColumn->toArray()));
457457
// Remove the column name from $columnSql
458-
$columnType = preg_replace('/^"?(?:[^"]+)"?\s+/', '', $columnSql);
458+
$columnType = (string)preg_replace('/^"?(?:[^"]+)"?\s+/', '', $columnSql);
459459
// Remove generated clause
460-
$columnType = preg_replace('/GENERATED (?:ALWAYS|BY DEFAULT) AS IDENTITY/', '', $columnType);
460+
$columnType = (string)preg_replace('/GENERATED (?:ALWAYS|BY DEFAULT) AS IDENTITY/', '', $columnType);
461461

462462
$sql = sprintf(
463463
'ALTER COLUMN %s TYPE %s',
@@ -483,10 +483,10 @@ protected function getChangeColumnInstructions(
483483
);
484484
}
485485
// NULL and DEFAULT cannot be set while changing column type
486-
$sql = preg_replace('/ NOT NULL/', '', $sql);
487-
$sql = preg_replace('/ DEFAULT NULL/', '', $sql);
486+
$sql = (string)preg_replace('/ NOT NULL/', '', $sql);
487+
$sql = (string)preg_replace('/ DEFAULT NULL/', '', $sql);
488488
// If it is set, DEFAULT is the last definition
489-
$sql = preg_replace('/DEFAULT .*/', '', $sql);
489+
$sql = (string)preg_replace('/DEFAULT .*/', '', $sql);
490490
if ($newColumn->getType() === 'boolean') {
491491
$sql .= sprintf(
492492
' USING (CASE WHEN %s IS NULL THEN NULL WHEN %s::int=0 THEN FALSE ELSE TRUE END)',
@@ -505,11 +505,12 @@ protected function getChangeColumnInstructions(
505505
'ALTER COLUMN %s',
506506
$quotedColumnName,
507507
);
508-
if ($newColumn->isIdentity() && $newColumn->getGenerated() !== null) {
508+
$generated = $newColumn->getGenerated();
509+
if ($newColumn->isIdentity() && $generated !== null) {
509510
if ($column->isIdentity()) {
510-
$sql .= sprintf(' SET GENERATED %s', (string)$newColumn->getGenerated());
511+
$sql .= sprintf(' SET GENERATED %s', $generated);
511512
} else {
512-
$sql .= sprintf(' ADD GENERATED %s AS IDENTITY', (string)$newColumn->getGenerated());
513+
$sql .= sprintf(' ADD GENERATED %s AS IDENTITY', $generated);
513514
}
514515
} else {
515516
$sql .= ' DROP IDENTITY IF EXISTS';
@@ -546,12 +547,13 @@ protected function getChangeColumnInstructions(
546547
}
547548

548549
// rename column
549-
if ($columnName !== $newColumn->getName()) {
550+
$newColumnName = $newColumn->getName();
551+
if ($columnName !== $newColumnName) {
550552
$instructions->addPostStep(sprintf(
551553
'ALTER TABLE %s RENAME COLUMN %s TO %s',
552554
$this->quoteTableName($tableName),
553555
$quotedColumnName,
554-
$this->quoteColumnName((string)$newColumn->getName()),
556+
$this->quoteColumnName($newColumnName),
555557
));
556558
}
557559

@@ -873,6 +875,7 @@ public function dropDatabase($name): void
873875
*/
874876
protected function getColumnCommentSqlDefinition(Column $column, string $tableName): string
875877
{
878+
$columnName = $column->getName();
876879
$comment = (string)$column->getComment();
877880
// passing 'null' is to remove column comment
878881
$comment = strcasecmp($comment, 'NULL') !== 0
@@ -882,7 +885,7 @@ protected function getColumnCommentSqlDefinition(Column $column, string $tableNa
882885
return sprintf(
883886
'COMMENT ON COLUMN %s.%s IS %s;',
884887
$this->quoteTableName($tableName),
885-
$this->quoteColumnName((string)$column->getName()),
888+
$this->quoteColumnName($columnName),
886889
$comment,
887890
);
888891
}
@@ -923,9 +926,10 @@ protected function getIndexSqlDefinition(Index $index, string $tableName): strin
923926
} else {
924927
$createIndexSentence .= '(%s)%s%s;';
925928
}
926-
$where = (string)$index->getWhere();
927-
if ($where) {
928-
$where = ' WHERE ' . $where;
929+
$where = '';
930+
$whereClause = $index->getWhere();
931+
if ($whereClause) {
932+
$where = ' WHERE ' . $whereClause;
929933
}
930934

931935
return sprintf(
@@ -956,9 +960,13 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey, string $ta
956960
);
957961
$columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns()));
958962
$refColumnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getReferencedColumns()));
963+
$referencedTable = $foreignKey->getReferencedTable();
964+
if ($referencedTable === null) {
965+
throw new InvalidArgumentException('Foreign key must have a referenced table.');
966+
}
959967
$def = ' CONSTRAINT ' . $this->quoteColumnName($constraintName) .
960968
' FOREIGN KEY (' . $columnList . ')' .
961-
' REFERENCES ' . $this->quoteTableName($foreignKey->getReferencedTable()) . ' (' . $refColumnList . ')';
969+
' REFERENCES ' . $this->quoteTableName($referencedTable) . ' (' . $refColumnList . ')';
962970
if ($foreignKey->getOnDelete()) {
963971
$def .= " ON DELETE {$foreignKey->getOnDelete()}";
964972
}
@@ -1327,7 +1335,7 @@ protected function getConflictClause(
13271335
}
13281336
$quotedConflictColumns = array_map($this->quoteColumnName(...), $conflictColumns);
13291337
$updates = [];
1330-
foreach ($updateColumns as $column) {
1338+
foreach ($updateColumns ?? [] as $column) {
13311339
$quotedColumn = $this->quoteColumnName($column);
13321340
$updates[] = $quotedColumn . ' = EXCLUDED.' . $quotedColumn;
13331341
}

‎src/Db/Adapter/RecordingAdapter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public function getInvertedCommands(): Intent
8989
case $command instanceof RenameColumn:
9090
/** @var \Migrations\Db\Action\RenameColumn $command */
9191
$column = clone $command->getColumn();
92-
$name = (string)$column->getName();
92+
$name = $column->getName();
9393
$column->setName($command->getNewName());
9494
$inverted->addAction(new RenameColumn($command->getTable(), $column, $name));
9595
break;

0 commit comments

Comments
 (0)