Static analysis at level 8, and the six months it took

PHPStan had been in the pipeline since 2019 at level 0, where it finds calls to functions that do not exist and almost nothing else. It had never reported a finding anybody remembered acting on, and it took forty seconds on every push. In November it reached 1.0 with a backward-compatibility promise, which made raising the level a decision worth making.

The symptom

$ vendor/bin/phpstan analyse --level=0
 [OK] No errors

$ vendor/bin/phpstan analyse --level=5
 [ERROR] Found 2,841 errors

$ vendor/bin/phpstan analyse --level=8
 [ERROR] Found 11,204 errors

# and the two bugs that reached production that quarter,
# both of which level 8 would have found:
#   a method called on a repository result that was null
#   an array key that only exists on one branch

Eleven thousand errors is a number that produces a decision not to look, which is why level 0 had survived for two years. The two production bugs are the argument for looking anyway.

Why it happens

The levels are cumulative and each adds a category of check, so the jump from 0 to 8 is eight categories at once against a codebase written without any of them in mind. There is no partial credit and no way to see progress.

It is also genuinely hard on a framework-heavy application, because a service container returning mixed and a model with magic properties defeat the analyser before any of your own code is examined.

The fix

The framework extension, first

$ composer require --dev nunomaduro/larastan

$ vendor/bin/phpstan analyse --level=5
 [ERROR] Found 402 errors        # was 2,841

# what the extension teaches the analyser:
#   app(Foo::class) returns Foo, not mixed
#   Model::where() returns a Builder
#   ->first() may return null; ->firstOrFail() may not
#   the facade signatures
#   model properties, from a generated docblock

Two thousand four hundred of the errors were the analyser not understanding the framework, and the extension removes them without changing a line of application code. Attempting any of this without it is why people conclude static analysis does not work on their stack.

Model properties are the remaining gap: they come from the database and the analyser cannot see the schema. A generated docblock per model closes it and goes stale, so generating them in CI and failing on a diff is what keeps them true.

A baseline that can only shrink

# phpstan.neon
parameters:
  level: 8
  paths: [app, src]
  reportUnmatchedIgnoredErrors: true

includes:
  - phpstan-baseline.neon
$ vendor/bin/phpstan analyse --generate-baseline
$ wc -l phpstan-baseline.neon
8412

# and the CI rule that makes it honest:
$ before=$(git show origin/main:phpstan-baseline.neon | wc -l)
$ after=$(wc -l < phpstan-baseline.neon)
$ [ "" -le "" ] || { echo 'baseline grew'; exit 1; }

The baseline is what lets the level be raised today and the existing violations be fixed over time. Without the ratchet it becomes a place to hide, and regenerating it to make a build pass is indistinguishable from a legitimate regeneration unless the count is checked.

New code is analysed at level 8 with no exceptions from the first day, which is the half that delivers the value immediately. The eight thousand existing violations are debt with a repayment schedule rather than a blocker.

Level by level, and which two hurt

0  unknown classes, functions, methods
1  undefined variables, unknown magic properties
2  unknown methods on all expressions, phpdoc validity
3  return types, property assignment types
4  dead code — always-false conditions, unreachable
5  ARGUMENT TYPES                    ← the first hard one
6  missing type hints entirely
7  partially wrong union types
8  NULLABILITY                       ← the second hard one
9  strict mixed

5 and 8 are where the work is. 6 is a large mechanical
diff. 9 is a different project entirely.

Level 5 is hard because it requires the argument types to be right everywhere, which means the docblocks that had been decorative become load-bearing. Level 8 is hard because a codebase written without nullability in mind assumes everything is present.

Level 6 is the one people dread and is mostly mechanical — it demands a type on everything untyped, and Rector generates most of them. Doing 6 before 7 and 8 is worth it because the added types make the later levels find real problems instead of missing information.

What level 8 actually reports

// level 7: fine.  level 8: Cannot call method total() on Order|null
$total = $this->orders->find($id)->total();

// the three fixes, in order of preference

// 1. narrow the return type — removes the class
$total = $this->orders->findOrFail($id)->total();

// 2. handle it explicitly — the null is real
$order = $this->orders->find($id);

if ($order === null) {
    throw new OrderNotFound($id);
}

// 3. nullsafe — ONLY when null is a legitimate outcome
$total = $this->orders->find($id)?->total() ?? Money::zero();

The temptation at level 8 is option three everywhere, which silences the analyser and pushes the null further along — the error becomes a wrong value rather than a crash, which is worse. Counting how many of the fixes were option three is a useful review metric.

Option one is the fix that removes the category rather than the message, and it frequently means a repository gains a findOrFail alongside its find. That is a better API regardless of the analyser.

Generics, which are docblocks the engine ignores

/**
 * @template T of Model
 */
final class Repository
{
    /** @param class-string<T> $class */
    public function __construct(private string $class) {}

    /** @return T|null */
    public function find(int $id): ?Model { /* ... */ }

    /** @return Collection<int, T> */
    public function all(): Collection { /* ... */ }
}

// $repo = new Repository(Order::class);
// $repo->find(1)  is Order|null, to the analyser
// $repo->all()    is Collection<int, Order>

PHP has no generics, so all of this is a promise checked by the analyser and by nothing at runtime — a function returning the wrong type produces no error. class-string<T> is the piece that makes a generic factory work and is the least obvious part of the syntax.

The whole scheme depends on the analyser running in CI, because a docblock nobody checks is worse than none: it is a claim people trust. Adding generics without the gate is actively harmful.

Ignoring a rule versus fixing the code

parameters:
  ignoreErrors:
    # brittle: the wording changed between 0.12 and 1.0
    - '#Cannot call method [a-z]+() on mixed#'

    # scoped, counted, and reported when it stops matching
    -
      message: '#no value type specified in iterable type#'
      path: src/Legacy/Importer.php
      count: 3

  reportUnmatchedIgnoredErrors: true

A count means fixing two of three occurrences also fails the build, which is the right kind of annoying. reportUnmatchedIgnoredErrors turns an obsolete ignore into a failure rather than a silent hole, and it is on by default and worth keeping.

The policy that got written down: a baseline entry needs no justification because it is historical, and an ignoreErrors entry needs a comment saying why the analyser is wrong. If the analyser is not wrong, it is a baseline entry or a fix.

The findings that were real bugs

Most of eleven thousand errors are missing type information rather than defects, which is the fair criticism of this kind of exercise. Recording the ones that were genuine bugs is what justified continuing past January.

// 1. a branch that could not be reached, hiding a
//    condition somebody meant to write
if ($order->state === OrderState::Shipped) {
    // ...
} elseif ($order->state === 'shipped') {
    $this->notify($order);        // level 4: unreachable
}

// 2. an array key that exists on one branch only
$context = $failed ? ['error' => $e] : ['result' => $r];
$this->log($context['error']);    // level 8: may not exist

// 3. a return type that had been wrong since 2018
/** @return Order[] */
public function recent(): Collection   // level 6: mismatch

The first is the shape that produces a feature quietly not working: a comparison against a string after the column became an object, in a branch nothing covered. The second had been logging null into an error tracker for two years and nobody had noticed the errors had no messages.

Across the six months there were fourteen findings of that kind out of roughly eleven thousand messages, which is a ratio worth stating plainly — the value is not that the analyser finds many bugs but that it finds the ones nothing else would, and that the cost per finding falls sharply once the baseline exists.

The schedule that made it happen

  Nov   extension + baseline at level 8. new code clean.
  Dec   level 6 types via Rector — 402 files, one commit
  Jan   the ORM layer:  8,412 → 6,104
  Feb   the HTTP layer: 6,104 → 3,988
  Mar   the domain:     3,988 → 1,204
  Apr   the legacy importer: 1,204 → 611
  May   the rest:         611 → 0, baseline deleted

and the rule that made it survive contact with a roadmap:
one afternoon a fortnight, on the module somebody is
already working in. never a dedicated branch.

Six months of a fortnightly afternoon is about sixty hours, spread thinly enough that no release was ever blocked on it. A dedicated branch would have conflicted with everything and been abandoned in February, which is what happened the first time this was attempted in 2020.

Verifying it worked

$ vendor/bin/phpstan analyse
 [OK] No errors

$ ls phpstan-baseline.neon
ls: cannot access: No such file or directory

$ vendor/bin/phpstan analyse --level=9 | tail -1
 [ERROR] Found 1,841 errors        # a different project

# and the number that justified it: production bugs of
# the class level 8 detects
#   the six months before:  7
#   the six months after:   1

Deleting the baseline is the moment the exercise is finished and it is worth doing rather than leaving an empty file. Seven null-related production bugs becoming one is the outcome; the remaining one came from data rather than from code, which no analyser could have caught.

Level 9 reporting 1,841 errors is the honest note to end on — this is a level, not a finish line, and 9 is about mixed everywhere and is a genuinely larger undertaking.

What this costs

Docblocks that are now load-bearing and unenforced at runtime, which is a strange position: the type information the application depends on for correctness reviews lives in comments. That only works because the analyser runs on every push, and an organisation that skips it under deadline pressure has a codebase making claims nothing checks.

The analysis also takes ninety seconds on a warm cache and four minutes cold, which is a real addition to every pipeline. Running it only on changed files in a pull request and fully on the default branch is the compromise, and it means a change that breaks an unchanged file is found after merge rather than before.

The deepest cost is that level 8 changes how code is written, mostly for the better and occasionally not. Narrowing return types to avoid nullability produces more methods with more specific names, which is good; the pressure to add ?-> to make a message go away is constant and produces code that fails later and more quietly. Reviewing for that specifically, rather than trusting the green build, is the part that no tool does.