/

PHPStan level 6 on a codebase from 2014

1,895 words, about 8 min read

The first run took eleven minutes and printed 417 errors. About 190 of them were the same complaint in different files: a class we deleted in 2017, still referenced from a service definition and two command classes nobody calls. That is what level 0 buys you on an application whose first commit is dated March 2014.

The codebase in question is the oldest thing on my desk: 2,140 PHP files, roughly 162,000 lines, Symfony 2.5 originally, dragged to 3.4 in 2018 and to 4.4 last year, Doctrine ORM underneath, PHP 7.4 now. It has 611 tests and they pass. It also has a habit of throwing Call to a member function on null in production about once a month, which is why I spent two weeks in May and June putting PHPStan on it. We are on 0.12.90, tagged yesterday.

The ladder, and where the numbers jumped

PHPStan’s levels are the whole reason this is possible on old code. As of 0.12.90 there are nine of them, 0 to 8, and they are cumulative: level 5 runs everything from 0 to 4 as well. The rule levels page spells out what each one adds, and the short version of the ladder is this: 0 checks unknown classes, unknown functions, methods called on $this, argument counts and always-undefined variables; 1 adds possibly undefined variables and magic methods; 2 checks methods on every expression rather than just $this, and validates PHPDocs; 3 does return types and types assigned to properties; 4 is basic dead code, always-false instanceof, unreachable statements; 5 checks the types of arguments passed to methods and functions; 6 reports missing typehints; 7 reports partially wrong union types; 8 reports calls on nullable types.

Levels 0 through 4 were cheap because most of what they found was rot. Dead classes, a controller action referencing a form type that had been renamed, two commands with an argument count that stopped matching in 2016 and are never invoked from cron anymore. Level 3 found fourteen methods whose PHPDoc return type disagreed with what they actually return, which is not a crash but is a lie the next person will believe.

Level 5 is where it got interesting, and level 6 is where it got loud.

LevelErrors reportedGenuine bugs in there
04179
31,74014
52,21031
68,9960 at first, 22 once annotated

“Genuine bug” here means I could describe the input that breaks it. Everything else was a missing annotation, a wrong annotation, or code so dead it should have been deleted years ago.

The jump from 2,210 to 8,996 is almost entirely one rule. Level 6 turns on checkMissingIterableValueType, so array stops being an acceptable type and Shipment[] or array<int, Shipment> is required. It also turns on checkGenericClassInNonGenericObjectType, which is why every one of our 63 Doctrine repositories reported this:

Class AppRepositoryShipmentRepository extends generic class
DoctrineORMEntityRepository but does not specify its types: T

Method AppRepositoryShipmentRepository::findPendingForCourier() return type
has no value type specified in iterable type array.

Both keys are documented in the config reference and both can be turned off while keeping the rest of level 6. I nearly did that. I am glad I did not, because writing those annotations is what produced the 22 extra bugs in the last row of the table: once PHPStan knows a method returns array<int, Shipment>, every caller that treated it as something else becomes a level 5 error.

The bug the tests never hit

One error justified the whole exercise. Level 5, in the invoicing code, on a line that has been in production since 2017:

Parameter #1 $date of method AppInvoicePeriodFormatter::format()
expects DateTimeImmutable, DateTimeImmutable|null given.

The code:

/**
 * @return DateTimeImmutable|null
 */
public function latestSettlementDate(int $accountId): ?DateTimeImmutable
{
    $row = $this->connection->fetchAssoc(
        'SELECT settled_at FROM account_settlement WHERE account_id = ? ORDER BY settled_at DESC LIMIT 1',
        [$accountId]
    );

    return $row ? new DateTimeImmutable($row['settled_at']) : null;
}

// ... in the statement builder, 400 lines away:
$header = $this->formatter->format($this->settlements->latestSettlementDate($account->getId()));

The method is honestly typed. The caller ignores it. PeriodFormatter::format() calls $date->format('F Y') on the first line, so an account with no settlement row produces a fatal error on the statement page. Every one of our invoicing tests built an account through a fixture that inserts a settlement row, because of course it does, so 611 passing tests never saw it. The accounts that hit it in production are the ones that only ever received a credit note: eleven of them, and the last support ticket about “statement page is blank” was in March.

That is the argument for static analysis in one error message. Tests check the paths you thought of. This checks the paths the types allow.

The baseline is a debt ledger

Nobody is fixing 8,996 errors before the next sprint. The answer is the baseline: run with --generate-baseline and PHPStan writes every current error into phpstan-baseline.neon as a message pattern, a count and a path, then includes that file and skips them next time.

includes:
    - phpstan-baseline.neon
    - vendor/phpstan/phpstan-doctrine/extension.neon
    - vendor/phpstan/phpstan-doctrine/rules.neon

parameters:
    level: 6
    paths:
        - src
        - tests
    doctrine:
        objectManagerLoader: tests/object-manager.php
    reportUnmatchedIgnoredErrors: true

Our baseline is 8,974 entries long and I read the documentation’s own warning about it more than once: the baseline works best for a few dozen to a few hundred errors, and if you have fifteen thousand you should be configuring PHPStan differently or running a lower level. Nine thousand is uncomfortably close to that line.

So we made one rule about what may enter it, and it is the only part of this setup I would defend in an argument. Missing annotations go in the baseline. Type mismatches do not. Anything PHPStan reports as a wrong type, a wrong argument, a null where a value is required, gets fixed in the pull request that surfaced it, even if that means a detour. The baseline is allowed to contain work we have not done. It is not allowed to contain bugs.

The second rule: the baseline may shrink, never grow. It is a file in the diff, so a PR that adds lines to it is visible in review, and reportUnmatchedIgnoredErrors stays on so that fixing the underlying code forces you to delete the entry rather than leaving a stale pattern behind. Six weeks in, we are down to 8,310 entries. At that rate it clears in about two years, which is roughly honest about what six years of untyped arrays cost.

Ignoring errors, and why it is blunter than you want

Outside the baseline, ignoring errors in 0.12.90 means regular expressions. You can add a bare pattern to ignoreErrors, which applies project-wide, or an entry with message plus path (with an optional count) or paths with fnmatch wildcards. In code there are the @phpstan-ignore-line and @phpstan-ignore-next-line PHPDoc tags.

That is all regex on the message text, and the failure mode is easy to reach. A project-wide '#Call to an undefined method#' silences the one legacy call you meant and also the typo someone writes next week. We have exactly four entries in ignoreErrors, every one of them carrying a path and a count, and each with a comment saying which library’s PHPDoc is wrong and what would let us remove it. When a message pattern needs a wildcard for a class name, it gets the narrow one, not .*.

What the Doctrine extension changed

Before writing a single annotation, installing phpstan/phpstan-doctrine removed 611 errors. It teaches PHPStan that EntityManager::getRepository(Shipment::class) returns a repository of shipments, recognises the magic findBy*, findOneBy* and countBy* methods our older code is full of, and reads EntityRepository<Shipment> in a PHPDoc so calls on the repository resolve properly.

It also added errors, which is the point. Including its rules.neon and pointing objectManagerLoader at a script that boots our container turns on DQL and QueryBuilder validation: parse errors, unknown entity classes, unknown persistent fields. Seven field names in DQL strings were wrong. Six were in branches guarded by a feature flag that has been off since 2019, but the seventh was in a reporting query that silently returned nothing rather than failing, which is the worst kind of wrong. The extension also compares entity column types against property types, which is how we learned that one decimal column has been mapped to a float property since 2015.

Annotating is the actual work

Level 6 does not want prose, it wants types, and PHPStan’s PHPDoc vocabulary is much wider than PHP 7.4’s. The PHPDoc types page covers what we used most: array<int, Shipment> and iterable<Shipment> for the shape of collections, array shapes like array{code: string, weight: float, zone: int} for the configuration arrays this codebase passes everywhere instead of objects, and class-string for the several places we take a class name as a string.

Generics were the surprise. I expected to consume them and never write them, and then hit our own container-ish helper, which took a class name and returned an object. With @template it describes itself properly:

/**
 * @template T of object
 *
 * @param class-string<T> $className
 *
 * @return T
 *
 * @throws EntityNotFound
 */
public function getOrFail(string $className, int $id): object
{
    $entity = $this->entityManager->find($className, $id);

    if ($entity === null) {
        throw new EntityNotFound($className, $id);
    }

    return $entity;
}

Nineteen call sites had been suppressing errors or re-asserting the type with an instanceof right after the call. Those all went away, and three of them turned out to be checking for the wrong class.

What we actually hold

CI runs level 6 with the baseline on every pull request. The first full run was eleven minutes; with the result cache warm and parallel processing across four cores it is 48 seconds, and --memory-limit 1G because the default is not enough for a project this size. If the analysis fails, the build fails. That last part is the only thing that makes any of it real.

Which is the honest limit of this whole exercise. A baseline is a debt ledger, and a level you cannot hold in CI is decoration. I could generate a baseline at level 8 tomorrow and put a badge on the README claiming it. Level 8 reports calls on nullable types, which is exactly the class of bug that brought me here, and I want it. But the baseline would be somewhere past 14,000 entries, nobody would read the diff when it grew, and the number would mean nothing. Level 6, held, with a shrinking ledger and a rule that type errors never enter it, is worth more than level 8 on paper.

Next is checkUninitializedProperties, and then level 7 on src/Invoice only, as a second configuration file with its own paths. If that holds for a month, the rest of src follows.

Sources

  • Rule levels: nine levels, 0 to 8, cumulative, and what each one adds. The basis for the ladder described above.
  • Configuration reference: checkMissingIterableValueType and checkGenericClassInNonGenericObjectType as the two level 6 switches, plus parallel processing and checkUninitializedProperties.
  • The baseline: --generate-baseline, the shape of phpstan-baseline.neon, including it from the config, reportUnmatchedIgnoredErrors, and the warning about baselines that are too large to be useful.
  • Ignoring errors: regular expressions with message, path, paths and count, and the @phpstan-ignore-line tags.
  • PHPDoc types: array<int, Type>, iterable<Type>, array shapes and class-string.
  • Generics in PHP using PHPDocs: @template, type variable bounds with of, and class-string<T> as used in the getOrFail() helper.
  • phpstan/phpstan-doctrine: repository return types, magic findBy* methods, DQL and QueryBuilder validation via objectManagerLoader, and column type comparison.
  • PHPStan 0.12.90: the release this was written against, tagged 18 June 2021.