← All resources

/// Article — Mar 2023

You've Just Run PHPStan on Your Legacy Codebase. Don't Fix the Errors.

/// Based on the talk

Introducing static analysis tools to legacy projects

PHP-SW, March 2023. The video goes through the same ideas in more detail.

Watch the talk →

Run PHPStan at max level on a codebase that's never used it before and you'll get thousands of errors.

Not dozens. Thousands.

The instinct is to either fix them all (you won't, you have features to ship) or give up and turn the level down until the number looks less scary (you'll lose most of the value).

There's a better option. Draw a line in the sand. Accept every existing issue, and only fail the build on new ones.

That's a baseline. Used well, it lets you get the benefits of static analysis from day one on any codebase, however old. And it gives you a way to steadily pay down the backlog without ever having to stop feature work.

What "legacy" means here

When I say legacy, I don't mean old, or badly written.

In this context, a legacy project is any project that hasn't used the static analysis tool you want to add. A six month old codebase written by a great team is "legacy" as far as PHPStan is concerned, if PHPStan has never been run on it.

And every one of those codebases will produce a pile of issues the first time you run a tool at its strictest level.

Why not just fix them?

Because it's not a good use of your time.

Your legacy codebase is, for the most part, working. It's in production. Customers are using it. Many of the issues the analyser reports will be things like missing type information, or code that could theoretically receive null but in practice never does.

Some issues will be real bugs, and you should fix those. But spending weeks fixing thousands of issues before you get any benefit from the tool is a hard sell to your team, and an even harder sell to whoever pays for your time.

The alternative is to start getting value immediately. Every new line of code gets checked properly. The old code gets improved gradually.

Three ways to baseline

There are several ways to build a baseline, and they have quite different trade-offs.

Technique 1: count the issues

The simplest possible baseline is a single number. You have 2,378 issues today. The build passes as long as there are 2,378 or fewer.

Please don't do this.

Imagine a developer fixes ten trivial issues and, in the same pull request, introduces one genuinely dangerous bug. The count has gone down. The build passes. The bug ships.

A count tells you almost nothing about which issues you have.

Technique 2: count issues per file (the PHPStan and Psalm approach)

PHPStan and Psalm both have baselining built in, and they work in a similar way. Instead of one number, the baseline records, for each file, how many times each issue occurs.

With PHPStan you create it like this:

vendor/bin/phpstan analyse --level=max --generate-baseline

That writes a phpstan-baseline.neon file, which you include from your main config:

includes:
    - phpstan-baseline.neon

The entries look roughly like this:

parameters:
    ignoreErrors:
        -
            message: '#^Cannot call method getName\(\) on App\\Entity\\Person\|null\.$#'
            identifier: method.nonObject
            count: 2
            path: src/Service/Welcomer.php

Now suppose that file later has three instances of that error. The build fails, because someone introduced a new one. And since the failure is tied to a specific file, it's usually easy to spot the culprit in the diff.

This approach is simple, it's built in, and for most projects it's the right place to start.

It does have a couple of weaknesses.

Renames and moves. Rename or move a file and every issue in it no longer matches its baseline entry, so they all look new.

Cancelling out. If a developer fixes one issue and introduces a different instance of the same issue in the same file, the count stays the same. The build passes, and the new issue slips through.

Maintenance. When you fix issues, the baseline needs updating. Modern PHPStan helps here. If an entry expects an error twice and it now only occurs once, PHPStan reports it, so you're nudged to regenerate the baseline and the count ratchets down. Do that every time. Otherwise the gap you've left is space for someone to reintroduce the problem.

Technique 3: track issues by line, using git (the SARB approach)

The third technique records each issue's type, file and line number, along with the git commit the baseline was created at.

That's what SARB (Static Analysis Results Baseliner) does. Full disclosure: I wrote it. I started SARB in mid 2018, before either PHPStan or Psalm had a baseline feature. To be fair, if I'd seen their approach first, I probably wouldn't have bothered.

When you run the analyser later, SARB takes each reported issue and uses git's history to work out where that line of code was at the baseline commit.

Here's an example. The baseline, created at commit 06b982c, contains:

InvalidNullableReturnType               src/Entity/Person.php:93

Since then, someone has renamed Person to Employee and deleted 20 lines above the problem. The analyser now reports:

InvalidNullableReturnType               src/Entity/Employee.php:73

SARB asks git: where was Employee.php line 73 at commit 06b982c? Git says: Person.php line 93. That's in the baseline, so the issue is suppressed.

If any step fails (the file is new, or the line can't be traced back, or it traces back to a line that didn't have that issue), the issue is reported as new.

This gets around both the rename problem and the cancelling-out problem. The trade-offs are that it's more work to set up than the built-in baseliners, and it has a quirk of its own. If one line has two issues and you edit that line to fix one of them, git sees it as a new line, so the remaining issue gets reported as new. That's a limitation of how git tracks lines, and there's no good way around it.

Using SARB with PHPStan looks like this:

# Create the baseline (make sure you're on the commit you want to baseline)
vendor/bin/phpstan analyse --error-format=json \
    | vendor/bin/sarb create --input-format="phpstan-json" phpstan.baseline

# Later, show only issues introduced since the baseline
vendor/bin/phpstan analyse --error-format=json \
    | vendor/bin/sarb remove phpstan.baseline

If you're running it in GitHub Actions, make sure you fetch the full git history (SARB needs it) and use --output-format=github to get annotations on your pull requests.

Which should you use?

Start with the built-in baseline in PHPStan or Psalm. It's simple, and for most teams it's enough.

Reach for SARB if file renames and moves are causing you pain, if you need a stronger guarantee that every new issue is reported, or if your tool doesn't have a baseline feature at all. Don't use issue counting.

Baselining any tool, in any language

That last point is worth expanding on, because it's something people often don't realise.

SARB is written in PHP, but it doesn't care what language your code is in, or what tool analysed it. Its input format is a simple JSON array. Each issue needs a file, a line, a type (the rule that was broken) and a message.

SARB supports several tools out of the box (PHPStan, Psalm, Phan, PHP_CodeSniffer, PHPMD and others). For anything else, write a small script that converts the tool's output into the SARB format and pipe it through.

Here's one for ESLint. ESLint's JSON output groups issues under each file, so we flatten them out:

<?php

// eslint2sarb.php

$input = stream_get_contents(STDIN);
if ($input === false) {
    die("Could not read input\n");
}

$files = json_decode($input, true);
if (!is_array($files)) {
    die("Could not parse JSON\n");
}

$issues = [];
foreach ($files as $file) {
    foreach ($file['messages'] as $message) {
        $issues[] = [
            'file' => $file['filePath'],
            'line' => $message['line'],
            'type' => $message['ruleId'],
            'message' => $message['message'],
        ];
    }
}

echo json_encode($issues, JSON_PRETTY_PRINT);

Then:

npx eslint . --format json | php eslint2sarb.php | vendor/bin/sarb create eslint.baseline

Twenty-odd lines of PHP and you've got line-level baselining for your JavaScript.

Before you create the baseline

Don't just run the tool and baseline everything straight away. Spend a little time first.

Run any safe auto-fixers. Formatting fixers, for example, are pretty much always safe to run. There's no point baselining things a tool can fix for you.

Fix the critical issues. Some of what the analyser finds will be genuine bugs. Fix those.

Time-box it. I'd spend roughly half a day to a day triaging before creating the baseline. Your code is mostly working, so there are diminishing returns in fixing lots up front. The exception is if you're adding static analysis because the code crashes all the time. In that case, spend longer.

With PHPStan, the levels give you a handy way to prioritise. Start at level 0. Anything found at the low levels is likely to be a real, crash-causing bug. Fix what's genuine, working up to around level 5. Then jump straight to max level and baseline everything that's left.

Then commit the baseline to your repository, and add the analysis to your CI pipeline.

From now on the loop is: write code, run the analyser, fix anything introduced since the baseline, repeat.

Making it stick

A baseline only works if the analysis runs on every change. If it's something people run "when they remember", new issues will creep in and the baseline becomes meaningless.

So it goes in CI, and it fails the build. Here's a minimal GitHub Actions job using PHPStan's built-in baseline:

name: Static analysis

on: [push, pull_request]

jobs:
    phpstan:
        runs-on: ubuntu-latest
        steps:
            - uses: actions/checkout@v4

            - uses: shivammathur/setup-php@v2
              with:
                  php-version: '8.4'

            - run: composer install --no-progress

            - run: vendor/bin/phpstan analyse --no-progress

If you're using SARB instead, add fetch-depth: 0 to the checkout step so SARB has the full git history it needs, then pipe PHPStan's JSON output into sarb remove with --output-format=github.

CI is the safety net, but it shouldn't be where developers first find out about problems. Waiting ten minutes for a pipeline to tell you about a missing null check is frustrating, and that frustration is how teams end up resenting the tool.

Make it easy to run locally. A Composer script helps:

{
    "scripts": {
        "analyse": "phpstan analyse"
    }
}

Now everyone runs composer analyse before pushing, and gets feedback in seconds.

Some teams go further and run it in a pre-commit hook. I'd suggest starting with the Composer script and CI, and adding hooks if people want them. Forcing hooks on a team that isn't bought in tends to end with people using --no-verify.

One more thing that makes a big difference: talk to the team before you switch it on. Explain what a baseline is, and that nobody is expected to fix the existing issues. The most common objection I hear is "I'll get blamed for thousands of errors I didn't write". A baseline means they won't. They're only responsible for the code they change.

When should the baseline change?

In an ideal world you'd never recreate the baseline. In the real world there are three good reasons to update it.

You've made the analysis stricter. You raised the level, turned on more checks, or upgraded the tool. The new issues aren't new bugs, they were always there. You just couldn't see them. Fix them if it's reasonable. If not, re-baseline.

You've fixed issues. Removing entries from the baseline is always safe. Do it every time, so the baseline only ever shrinks.

A correct fix causes a ripple. This one needs explaining, because it's the case where the baseline might grow, and people feel bad about that.

I hit this on a real project. There was a function like this:

/**
 * @return Person
 */
function getPerson($id)
{
    // ...can actually return null
}

The docblock said it returned a Person. It could actually return null. The correct fix is obvious:

function getPerson(int $id): ?Person

But every caller had been written assuming it never returned null. As soon as I fixed the signature, the analyser flagged every one of them. Fixing those caused more issues further up the chain. It would have taken a day or two to fix it all properly.

The pragmatic move: fix the signature, so any new code calling getPerson has to handle null correctly. Then baseline the knock-on issues. The code isn't crashing in production, so in practice those callers are fine. You'll clean them up later.

The baseline grew. That's fine. The codebase is safer than it was.

Paying down the backlog

A baseline stops things getting worse. On its own, it doesn't make things better.

The good news is the baseline shrinks naturally. New code interacts with old code, and issues get fixed in passing. On one project I worked on, we had around 3,000 to 4,000 issues when we introduced static analysis. Two or three years later we were down to about 600, mostly without anyone making a special effort.

You can speed that up, though.

Five a day

SARB has a --clean-up option:

vendor/bin/phpstan analyse --error-format=json \
    | vendor/bin/sarb remove phpstan.baseline --clean-up

Instead of showing you the entire baseline, it picks five issues at random.

Here's the maths. If every developer fixes five baseline issues a day, over a working year that's roughly 1,000 issues per developer. On top of the ones that get fixed naturally.

Why random? Because when you give people a list of 2,000 issues, they look at the first few or the last few, and never the middle. And a list of 2,000 is demoralising. Five is manageable. Fix the easy ones, run it again, get a fresh five.

You don't need SARB for this. A small script that picks random entries from your PHPStan baseline does the same job. The idea is what matters.

Show the progress

SARB reports how many issues were in the original baseline compared to now. Something like "2,000 → 676". It's a small thing, but it's motivating to see the number come down.

It's also a reason not to recreate the baseline unnecessarily, because the counter resets. (There's one exception: with SARB, the analysis gets slower as the diff between the baseline commit and your current code grows. In my experience it can reach around ten seconds. Regenerate it occasionally for speed.)

A baseline is a bridge

The point of a baseline isn't to live with it forever. It's a way to get from where you are now to a codebase with zero issues, without stopping everything else to get there.

Add the tool. Baseline the existing issues. Check every new line properly. Chip away at the old ones.

Within a year or two, you might find you don't need the baseline at all.

The full talk is on YouTube, and SARB is on GitHub if you want to try it.

How big was your first baseline, and how far have you got it down?

Start a conversation

We help teams put these ideas into practice, through training, code review and working alongside you. Tell us where you are and what you're aiming for.