Skip to content

Bump PHPStan to level 8 and fix all errors#1027

Draft
jamisonbryant wants to merge 22 commits into5.nextfrom
fix/phpstan-and-command-props
Draft

Bump PHPStan to level 8 and fix all errors#1027
jamisonbryant wants to merge 22 commits into5.nextfrom
fix/phpstan-and-command-props

Conversation

@jamisonbryant
Copy link
Contributor

Summary

  • Bumps PHPStan analysis level from 7 to 8
  • Makes $io and $args properties non-nullable in BakeSimpleMigrationCommand (they are always set before use in bake())
  • Fixes all 52 PHPStan level 8 errors across the codebase

Changes by category

Command classes

  • BakeSimpleMigrationCommand: Made $io/$args non-nullable — these properties are always assigned in bake() before any method reads them
  • BakeMigrationDiffCommand: Added null guards for getColumn()/getConstraint() nullable returns

Db/Table value objects

  • Column::getNull(): Coalesce nullable ?bool property to false (matches SQL default)
  • ForeignKey::getOnDelete()/getOnUpdate(): Guard null from getDelete()/getUpdate() before calling mapAction()

Db adapters (Mysql, Postgres, Sqlite, Sqlserver, Abstract, TimedOutput)

  • Cast preg_replace results to (string) where hardcoded patterns can't fail
  • Coalesce Constraint::getColumns() nullable return with ?? []
  • Cast ForeignKey::getReferencedTable() and Column::getName() at call sites
  • Assert non-null in AbstractAdapter::getConnection() after lazy init
  • Use null-safe ?-> for optional ConsoleIo in TimedOutputAdapter

Other

  • Db/Plan/Plan.php, Db/Table.php: Same getColumns()/getName() patterns
  • BaseSeed.php: Guard nullable getConfig() offset access
  • Util/ColumnParser.php: Cast preg_replace result
  • Util/TableFinder.php: Null-check association before method call
  • View/Helper/MigrationHelper.php: Use isset() for constraint type check

Test plan

  • tools/phpstan analyse --memory-limit=-1 passes with 0 errors at level 8
  • Existing test suite still passes

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: a null result becomes '', which can lead to generating an empty/incorrect referenced table name. Instead of casting, handle a null return explicitly (e.g., fall back to the original $fieldName or 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 ADmad changed the base branch from 5.x to 5.next February 24, 2026 19:20
@jamisonbryant jamisonbryant added this to the 5.next milestone Feb 24, 2026
dereuromark and others added 20 commits February 24, 2026 15:46
…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.
@jamisonbryant jamisonbryant force-pushed the fix/phpstan-and-command-props branch from c900fea to 7bb0e02 Compare March 11, 2026 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants