Türker Yıldırım

The method that stopped overriding anything in 2019

Deep dive10 min read

Renaming a parent method is a silent break in every child that overrides it: the child keeps compiling, keeps its method, and the method is never called again. PHP 8.3 adds #[Override], which is a way of saying that a method is meant to be replacing something.

The symptom

// 2016
abstract class Report
{
    protected function formatRow(array $row): string { /* ... */ }
}

final class VatReport extends Report
{
    protected function formatRow(array $row): string { /* 40 lines */ }
}

// 2019: the parent was refactored to
//   protected function renderRow(RowData $row): string
//
// VatReport::formatRow still exists. it is never called.
// the VAT column vanished from one report.
the bug report, filed 2019-11:

  "the VAT column is missing from the quarterly export"

  closed 2020-02: could not reproduce.
  (the person reproducing it used the monthly export,
   which extends a different base class.)

four years. the column was missing the whole time.

Why it happens

An overriding method has no syntactic marker, so it is indistinguishable from a method the class happens to define. Nothing in the language expresses the intent, and therefore nothing can check it.

The fix

What the attribute checks, and when

class Report
{
    protected function renderRow(RowData $row): string { /* ... */ }
}

final class VatReport extends Report
{
    #[Override]
    protected function formatRow(array $row): string { /* ... */ }
    // Fatal error: VatReport::formatRow() has #[Override]
    // attribute, but no matching parent method exists
}

The check runs at compile time, when the class is linked, not at runtime when the method is called — which means a class that is never instantiated still fails. That is the correct severity: an override that overrides nothing is dead code, and dead code that claims to be live is worse than dead code.

Interfaces too, which is the part people miss

interface Exportable { public function export(): string; }

final class CsvExport implements Exportable
{
    #[Override]
    public function export(): string { /* ... */ }   // ok

    #[Override]
    public function exportAll(): string { /* ... */ }
    // Fatal: no matching method in any parent or interface
}

An interface method implementation counts, which is more useful than it first sounds: removing a method from an interface leaves every implementation with an orphan, and the attribute turns that into a compile error rather than a set of methods nobody calls.

Adding it across nine hundred methods

$ vendor/bin/rector process src tests --config=rector-add-override.php
  412 files, 904 methods gained #[Override]

# a parse check does not link classes, so it finds nothing
$ vendor/bin/phpunit 2>&1 | grep 'has #[Override]'
  VatReport::formatRow()
  MonthlyDigest::buildSubject()
  ApiClient::handleResponse()
  ImportCommand::configureOptions()

Four failures out of nine hundred and four, and the test suite is what surfaced them because linking happens on autoload. A parse check does not catch this — the class has to be loaded, which means the sweep is only as complete as the code paths the tests reach.

The four

  VatReport::formatRow   2019 parent rename. the
    four-year bug.
  MonthlyDigest::buildSubject   2021. the parent was made
    final and the method inlined; the child stayed.
  ApiClient::handleResponse   2022. a library upgrade
    renamed the hook, and the library's default behaviour
    had applied ever since — nearly correctly.
  ImportCommand::configureOptions   the console component
    calls configure(). this had NEVER run, since 2018.

The last one is the most instructive: it was never correct, not broken by a rename. Somebody guessed a method name in 2018, the guess was wrong, and the options it configured had defaults that happened to be acceptable — so nobody noticed for five years.

The one that failed for a good reason

trait Timestamps { public function touch(): void {} }
interface Touchable { public function touch(): void; }

final class Order implements Touchable { use Timestamps; }

// the trait satisfies the interface. adding #[Override]
// to the trait method is a fatal error in the trait's own
// context, because a trait has no parent.

A trait method cannot carry the attribute, because the trait does not know what it will be composed into. That is the one place the assertion cannot be made, and it is worth knowing before adding the attribute to a trait and discovering it at load time.

What it does not catch

a signature that CHANGED rather than a name is a
substitution violation, which the engine already refuses
with or without the attribute — so nothing is added there.

and it does not catch a method that correctly overrides
something nobody calls. #[Override] asserts a
relationship, not usefulness.

Keeping it, with a lint rule

parameters:
  level: 8
  rules:
    - AppPHPStanRequireOverrideAttributeRule

# which reports a method that overrides a parent or
# implements an interface method and has no #[Override].
# 0 violations after the sweep, and it fails the build
# on a new one.

The attribute is only a guarantee if it is on every override, and nothing in the language requires that — a method added next year without it reverts to the old behaviour silently. The rule is what makes this a property of the codebase rather than of one afternoon.

Verifying it worked

$ vendor/bin/phpunit && vendor/bin/phpstan analyse
  Tests: 1,414 passed
 [OK] No errors

$ grep -rc '#[Override]' src/ tests/ | awk -F: '{s+=$2} END {print s}'
904

# the deliberate check: rename a parent method
$ sed -i 's/renderRow/renderRowV2/' src/Reporting/Report.php
Fatal error: VatReport::renderRow() has #[Override]
attribute, but no matching parent method exists

$ php artisan report:export --period=quarter | head -1
reference,net,vat,total          # vat is back

Renaming a parent method on purpose and confirming the fatal error is what establishes that the protection is live. The VAT column reappearing in the quarterly export is the outcome, and it needed explaining to finance for the same reason every old bug does.

What this costs

An attribute on nearly every override, forever, which is nine hundred and four lines of ceremony. That is the trade and it is the same one made by every language that has this feature — the noise is uniform and the failure it prevents is silent.

The lint rule is also a rule that will be disabled by somebody generating code, because a code generator emitting overrides has to know about the attribute. Ours does now; the next one will not, and the rule will be the thing that surfaces it — which is the correct outcome and will feel like an obstruction.