Bump PHPStan to level 8 and fix all errors#1027
Draft
jamisonbryant wants to merge 22 commits into5.nextfrom
Draft
Bump PHPStan to level 8 and fix all errors#1027jamisonbryant wants to merge 22 commits into5.nextfrom
jamisonbryant wants to merge 22 commits into5.nextfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Raises static analysis strictness by bumping PHPStan from level 7 to 8 and updates the codebase to satisfy the stricter type expectations (primarily around nullability and string/array shapes) across migrations commands, DB adapters, and helpers.
Changes:
- Bump PHPStan level to 8.
- Add null-safety and type coercions across schema diffing, adapters, and helpers to eliminate level-8 findings.
- Tighten some command properties’ nullability and guard config access in seeding.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/View/Helper/MigrationHelper.php | Adds isset() guard for constraint type access. |
| src/Util/TableFinder.php | Avoids potential null association access when iterating associations. |
| src/Util/ColumnParser.php | Casts preg_replace() result to string for PHPStan. |
| src/Db/Table/ForeignKey.php | Guards nullable getDelete()/getUpdate() before mapping actions. |
| src/Db/Table/Column.php | Coalesces nullable null flag to boolean default. |
| src/Db/Table.php | Adds casts around nullable column type/name usage and PK filtering. |
| src/Db/Plan/Plan.php | Coalesces nullable FK columns to [] when remapping conflicts. |
| src/Db/Adapter/TimedOutputAdapter.php | Uses nullsafe operator for optional ConsoleIo. |
| src/Db/Adapter/SqlserverAdapter.php | Casts regex results to string and coalesces nullable FK columns/table. |
| src/Db/Adapter/SqliteAdapter.php | Casts regex/regex-callback results to string; coalesces nullable FK columns/table. |
| src/Db/Adapter/PostgresAdapter.php | Casts regex results to string; coalesces nullable FK columns and update columns. |
| src/Db/Adapter/MysqlAdapter.php | Casts potentially nullable column name and referenced table. |
| src/Db/Adapter/AbstractAdapter.php | Adds assertion for non-null connection after lazy init; coalesces FK columns. |
| src/Command/BakeSimpleMigrationCommand.php | Makes $io / $args non-nullable properties. |
| src/Command/BakeMigrationDiffCommand.php | Adds null guards around nullable column/constraint lookups. |
| src/BaseSeed.php | Guards nullable config access when building ManagerFactory options. |
| phpstan.neon | Bumps analysis level from 7 to 8. |
Comments suppressed due to low confidence (1)
src/Util/ColumnParser.php:217
- Casting
preg_replace()to string changes the failure mode when PCRE errors occur: anullresult becomes'', which can lead to generating an empty/incorrect referenced table name. Instead of casting, handle anullreturn explicitly (e.g., fall back to the original$fieldNameor abort with a clear exception).
// Remove common suffixes like _id and pluralize
$referencedTable = (string)preg_replace('/_id$/', '', $fieldName);
$referencedTable = Inflector::pluralize($referencedTable);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ADmad
reviewed
Feb 24, 2026
…lict-columns Fix spurious warning when using empty conflictColumns with MySQL
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
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 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
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 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 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
Column::getNull() now coalesces the nullable bool property to false. ForeignKey::getOnDelete()/getOnUpdate() guard against null from getDelete()/getUpdate() before calling mapAction().
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.
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.
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.
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.
…le() 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.
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.
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().
Use early-continue pattern to narrow nullable safeGetColumn() return types before array operations, fixing PHPStan level 8 errors.
c900fea to
7bb0e02
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
$ioand$argsproperties non-nullable inBakeSimpleMigrationCommand(they are always set before use inbake())Changes by category
Command classes
BakeSimpleMigrationCommand: Made$io/$argsnon-nullable — these properties are always assigned inbake()before any method reads themBakeMigrationDiffCommand: Added null guards forgetColumn()/getConstraint()nullable returnsDb/Table value objects
Column::getNull(): Coalesce nullable?boolproperty tofalse(matches SQL default)ForeignKey::getOnDelete()/getOnUpdate(): Guard null fromgetDelete()/getUpdate()before callingmapAction()Db adapters (Mysql, Postgres, Sqlite, Sqlserver, Abstract, TimedOutput)
preg_replaceresults to(string)where hardcoded patterns can't failConstraint::getColumns()nullable return with?? []ForeignKey::getReferencedTable()andColumn::getName()at call sitesAbstractAdapter::getConnection()after lazy init?->for optionalConsoleIoinTimedOutputAdapterOther
Db/Plan/Plan.php,Db/Table.php: SamegetColumns()/getName()patternsBaseSeed.php: Guard nullablegetConfig()offset accessUtil/ColumnParser.php: Castpreg_replaceresultUtil/TableFinder.php: Null-check association before method callView/Helper/MigrationHelper.php: Useisset()for constraint type checkTest plan
tools/phpstan analyse --memory-limit=-1passes with 0 errors at level 8