Skip to content

6.x: Capability interfaces for change() vs up()/down() migrations #1084

@dereuromark

Description

@dereuromark

Background

MigrationInterface does not declare up(), down(), change(), or init(). They are runtime hooks dispatched via method_exists():

// src/Migration/Environment.php
if (method_exists($migration, MigrationInterface::CHANGE)) {
    $migration->change();
} elseif (method_exists($migration, $direction)) {
    $migration->{$direction}();
}

This costs us two permanent PHPStan baseline entries (method.notFound for the up/down dispatch) and prevents IDEs/static analysis from understanding migrations. Adding the methods directly to MigrationInterface is a BC break and is wrong semantically — a migration is either reversible (defines change()) or directional (defines up()/down()), never both.

Proposal

Split the optional hooks into capability interfaces, dispatch via instanceof.

New interfaces

namespace Migrations;

interface ReversibleMigrationInterface extends MigrationInterface
{
    public function change(): void;
}

interface DirectionalMigrationInterface extends MigrationInterface
{
    public function up(): void;
    public function down(): void;
}

(init() could get its own InitializableMigrationInterface if useful, or stay an optional hook — open question.)

Environment dispatch

if ($migration instanceof ReversibleMigrationInterface) {
    if ($direction === MigrationInterface::DOWN) {
        // ... record-adapter wrapping
        $migration->change();
        $recordAdapter->executeInvertedCommands();
    } else {
        $migration->change();
    }
} elseif ($migration instanceof DirectionalMigrationInterface) {
    $direction === MigrationInterface::UP ? $migration->up() : $migration->down();
}

PHPStan narrows correctly; the two baseline entries are removed.

BaseMigration stance

Proposal: opt-in. BaseMigration does NOT implement either capability interface. Every user migration declares its style explicitly:

class CreateProducts extends BaseMigration implements ReversibleMigrationInterface
{
    public function change(): void { /* ... */ }
}

Rationale: cleanest types, the 6.x BC-break window is where this kind of mechanical update is acceptable, and a rector rule can auto-apply the implements clause based on which method the class defines.

Alternative considered: BaseMigration defaults to DirectionalMigrationInterface with empty up()/down(). Zero user friction but PHPStan would treat every migration as having up/down even when only change() is defined — undermines the whole reason for the split.

Migration path for users

  1. For migrations defining change(): add implements ReversibleMigrationInterface.
  2. For migrations defining up()/down(): add implements DirectionalMigrationInterface.
  3. Bake templates emit the correct implements clause for new migrations.
  4. Provide a rector rule (AddMigrationCapabilityInterfaceRector) that scans for the method shape and adds the interface — runnable once in the upgrade.

Tasks

  • Add ReversibleMigrationInterface / DirectionalMigrationInterface (and possibly InitializableMigrationInterface)
  • Replace method_exists dispatch in Environment::executeMigration with instanceof
  • Update bake templates to emit implements ...
  • Provide rector rule for upgrade
  • Drop the two remaining Environment entries from phpstan-baseline.neon
  • Document the new contracts and the upgrade path in the 6.x migration guide

Open questions

  • Does init() warrant its own interface, or is the optional-hook style fine for it?
  • Should we ship the rector rule in this repo, or as a separate cakephp-upgrade task?

Context

Came up while reducing the PHPStan baseline in #1083 — the two remaining baseline entries on Environment.php cannot be removed without an interface change.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions