/// Article — Feb 2022
Your Tests Pass. You Have 100% Coverage. Your Code Is Still Broken.
/// Based on the talk
Practical Static Analysis
PHP UK 2022. The video goes through the same ideas in more detail.
Watch the talk →Here's a function:
function cost(string $type): int
{
if ($type === 'CHILD') {
$price = 10;
}
if ($type === 'ADULT') {
$price = 20;
}
return $price;
}
Here are the tests:
public function testChildCost(): void
{
$this->assertSame(10, cost('CHILD'));
}
public function testAdultCost(): void
{
$this->assertSame(20, cost('ADULT'));
}
Both pass. Line coverage: 100%. Branch coverage: 100%. By every metric we usually look at, this code is fully tested.
It's also broken. Call cost('STUDENT') and $price is undefined.
Your tests didn't find that bug. PHPStan finds it in about a second, without you writing any extra code:
Variable $price might not be defined.
That's the case for static analysis in a nutshell.
Tests and static analysis do different jobs
That example isn't an argument against tests. It's an argument for understanding what each tool tells you.
Tests show your code is correct, but only for the scenarios you tested. Two tests, two scenarios. Everything else is unknown.
Static analysis shows where your code is incorrect. It reasons about every possible path through the code. But it can never tell you the code does what the business actually wants.
You need both. Neither replaces the other.
And there's a nice interaction between them. The better your types, the smaller the space of inputs your tests have to cover. If cost() took an enum instead of a string, there would be exactly two possible inputs, and our two tests really would cover everything. More on that later.
Why bother? Because bugs get more expensive
The later a bug is found, the more it costs.
A bug found in production might mean an incident, unhappy customers, an emergency fix, and time spent working out what went wrong. A bug found by a failing test in CI costs a few minutes of context switching. A bug found by static analysis, before the code even runs, costs almost nothing.
Static analysis moves bug detection as far left as it can go. And unlike tests, you don't have to write anything to get the benefit. Point the tool at your code and it starts working.
Three kinds of problem the analyser finds
When you first run PHPStan or Psalm on a codebase, you'll get a long list. It helps to sort what you find into categories.
Real bugs
Code that will fail when it runs.
interface UserRepository
{
public function findUser(string $name): ?User;
}
$user = $userRepository->findUser('bob');
$emailer->message($user, 'Hi Bob'); // Fails when Bob doesn't exist
findUser() can return null. message() expects a User. When Bob isn't found, this blows up.
Some of these may already be failing in production. If you find one, it's worth checking your error logs.
Deferred bugs
Code that works today, for every current use, but will break when something changes.
The cost() function is a good example. Maybe it's only ever called with 'CHILD' or 'ADULT', today. It works. Then someone adds a student ticket.
These cause arguments. "That's not a real bug, we never call it with anything else!" My advice: don't have the argument. The time spent proving the code is safe is usually longer than the time it takes to fix it. And the proof goes out of date the moment someone changes the calling code.
Evolvability defects
Code that isn't wrong as such, but makes the codebase harder to understand, modify or extend. Technical debt, in other words.
/**
* @return int
*/
function getReference($order)
{
return $order->ref; // Actually a string
}
An untyped parameter. A docblock that lies about the return type. The code works, but the next person to touch it will make wrong assumptions.
These are the easiest to dismiss and the most important to fix over time. Every one of them makes the next change a little slower and a little riskier.
Adding a tool is like adding a team member
When you add PHPStan or Psalm to a project, the team goes through something very like the classic stages of team formation.
Forming. Excitement. You've heard good things. You install it and run it.
Storming. Thousands of errors. Half of them look ridiculous. "That's clearly not a bug!" There's frustration, and someone suggests turning it off.
Norming. The team starts to understand what the tool needs. Types, docblocks, extensions, a baseline. The errors start to make sense.
Performing. Fewer bugs reach production. Refactoring gets less scary. You start writing custom rules to enforce your own standards.
Knowing the storming phase is coming helps a lot. It's normal, and it passes. In my experience, it takes about three rounds of "write code, run the analyser, fix the issues" before it becomes second nature.
Help the tool understand your code
The analyser is only as good as the information it has. There are four places it gets that information from, and your job is to make each of them as rich as possible.
1. Type declarations
Native PHP types. The analyser trusts these completely, because PHP enforces them at runtime.
Add them everywhere you can. Every property. Every parameter. Every return type, including void. Including the return types of closures and arrow functions, which I only started doing because PHPStan kept pointing them out.
2. Docblock types
PHP's type system can't express everything. It can't say "an array of User objects keyed by string", or "a string that's one of these three values", or "a list that's never empty".
Docblocks can:
/** @return array<string, User> */
public function getUsersByEmail(): array { /* ... */ }
/** @param non-empty-list<Order> $orders */
public function processBatch(array $orders): void { /* ... */ }
/** @param 'asc'|'desc' $direction */
public function sort(string $direction): void { /* ... */ }
And, of course, generics with @template. I've written a whole article on that: PHP Has Had Generics for Years.
One annotation I want to call out: assertions. If you've got a helper that throws when a value is null, the analyser can't see through it by default:
/**
* @phpstan-assert !null $value
*/
function assertNotNull(mixed $value, string $message): void
{
if ($value === null) {
throw new LogicException($message);
}
}
$user = $repository->findUser($name);
assertNotNull($user, "User [$name] must exist");
$user->getEmail(); // PHPStan now knows $user isn't null
3. Extensions and plugins
Frameworks do magic. Laravel facades, Doctrine repositories, Symfony's container. Out of the box, the analyser doesn't understand them, and you'll get a flood of false errors.
Extensions teach the tool about that magic. There are good ones for PHPUnit, Symfony, Doctrine, Laravel (Larastan) and many more.
With PHPStan, install phpstan/extension-installer once, and any extension you add with Composer registers itself:
composer require --dev phpstan/extension-installer
composer require --dev phpstan/phpstan-phpunit phpstan/phpstan-doctrine
You can find them by searching Packagist for the phpstan-extension type.
4. Stubs
Sometimes a library you depend on has poor or missing type information, and there's no extension for it. A stub file re-declares the library's classes with just their signatures and better type information, and the analyser uses that instead.
Stubs are powerful, but be careful. Get a stub wrong and the analyser will confidently tell you wrong things.
Often a cleaner option is a thin adapter: wrap the badly-typed library in a small class of your own with proper types, and keep any mess confined to that one place.
Make your coupling visible
This is the part of the talk I think gets overlooked most, and it's where static analysis really changes how you write code.
Go back to cost(). The underlying problem isn't the missing else. It's that there's an invisible link between the strings 'CHILD' and 'ADULT', wherever they're passed in, and the if statements inside cost(). Add a new ticket type and nothing tells you about all the places that need updating.
Make that link explicit, and the analyser can follow it:
enum PersonType
{
case Adult;
case Child;
}
function cost(PersonType $type): int
{
return match ($type) {
PersonType::Child => 10,
PersonType::Adult => 20,
};
}
Now add a new case:
enum PersonType
{
case Adult;
case Child;
case Student;
}
Run PHPStan:
Match expression does not handle remaining value: PersonType::Student
Every match on PersonType, anywhere in the codebase, gets flagged. The analyser gives you a to-do list of every place that needs to know about students.
That's the shift. You stop writing code where the connections exist only in developers' heads, and start writing code where the tools can see them.
It also brings us back to testing. cost() now has exactly two possible inputs, and two tests really do cover every case.
Pick a level, then turn it up to max
Both main tools have levels. PHPStan goes from 0 (least strict) up to max. Psalm runs the other way, from 8 (least strict) to 1 (strictest).
When I first gave this talk, I recommended starting at a low level and gradually ratcheting up. I've changed my mind.
Now I'd say: go to the strictest level straight away, and use a baseline to record the existing issues. That way, all new code is held to the highest standard from day one, and you're not re-baselining every time you move up a level.
How to do that well, especially on older codebases, is a topic in itself. I've written it up in You've Just Run PHPStan on Your Legacy Codebase. Don't Fix the Errors.
Don't forget your tests. Run static analysis on test code too. Buggy tests are worse than no tests, because they give you false confidence.
What to do when you disagree with the tool
You will disagree with it sometimes. There are two very different situations.
You disagree with a whole rule. Some checks just don't suit some teams. That's fine. Turn the check off in config, and everyone knows where they stand.
It's a one-off. The tool is wrong about this specific line, or you've got a good reason for doing something unusual. Suppress it right there in the code, with the error identifier and a reason:
// @phpstan-ignore argument.type (Legacy API returns numeric strings, see ticket 1234)
$this->processor->handle($legacyValue);
Using the identifier means you only suppress that specific error, not everything on the line. And leaving the reason means the next person doesn't have to guess.
What I'd avoid is putting one-off exceptions in the baseline. The baseline is for issues you intend to fix eventually. A deliberate exception belongs next to the code, where people can see it.
And before you suppress anything, see if you can rewrite the code so the warning goes away. Nine times out of ten, the rewritten version is clearer anyway. The tool was confused because the code was confusing.
A refactoring tip
When you refactor, run static analysis before the tests.
A failing test tells you something's wrong, and you then have to work out what and where. Static analysis tells you exactly which line has a problem and why.
Rename a method, change a signature, move a class. Run PHPStan, fix everything it reports, then run the tests. You'll spend much less time debugging.
Data from the outside world lies
One of the most common real bugs I see is trusting data that comes from outside the application.
$data = json_decode($request->getContent(), true);
$this->orders->cancel($data['orderId']);
What type is $data['orderId']? You think it's a string, or maybe an int. But it came from a client. It could be a string, an int, a boolean, null, an array, or missing altogether.
At the strictest level, PHPStan treats it as mixed and won't let you pass it to a method expecting a string without checking first. That feels annoying at first. It's actually the tool pointing at exactly the place where a malformed request will cause an error, or worse.
The fix is to validate at the boundary, and give the rest of your code proper types:
$orderId = $data['orderId'] ?? null;
if (!is_string($orderId)) {
throw new BadRequestHttpException('orderId must be a string');
}
$this->orders->cancel($orderId); // PHPStan knows this is a string
Or, better, map the request onto a typed object using your framework's validation or a library like Valinor, and never pass raw arrays around at all.
Be careful adding types in bulk
Tools like Rector can add type declarations across a whole codebase automatically. It's tempting, and it can save a lot of time.
But be careful. Adding a native type declaration changes runtime behaviour.
I've been caught by this. A parameter was documented as an int, so it got declared as int. In production, it was actually receiving numeric strings from the database, like "42".
What happens next depends on the calling code. Where the caller uses declare(strict_types=1), you get a TypeError. Where it doesn't, PHP quietly coerces the string to an int, which changes the value's type inside the method and can trip up anything relying on it being a string.
Either way, the docblock had been wrong all along. The old code only worked because PHP was being forgiving about types, and adding the declaration took that forgiveness away.
Automated fixes are great. Just review them like you'd review a colleague's pull request, and be especially wary of types being added to code that handles data from databases or requests.
Where to focus
Not all code deserves the same attention.
Your business logic is the most valuable code you have. It's what makes your application yours. Aim the strictest analysis there.
Framework glue code, like controllers and configuration, matters less. The frameworks themselves are heavily tested. By all means analyse it, but don't lose sleep over every warning.
And watch out for misdirected effort. I've seen developers lose hours chasing low-priority static analysis warnings, when that time would have been better spent writing tests. The tool is there to help you ship better software, not to be satisfied for its own sake.
One tool is a big win. Two is a small one.
Going from no static analysis to PHPStan or Psalm is a huge improvement. Adding the second one on top gives you relatively little.
So which should you choose? Honestly, it doesn't matter much. Both are excellent. Pick the one your team, or the people you can ask for help, already know.
Where to start
If you don't have static analysis yet:
composer require --dev phpstan/phpstan phpstan/extension-installer
vendor/bin/phpstan analyse src tests --level=max
Take a deep breath at the output. Install extensions for your framework and run it again. Fix anything that's obviously a real bug. Baseline the rest. Add it to CI.
Then find one stringly-typed value in your business logic, turn it into an enum, and replace the ifs or switch with a match. See how it feels when the analyser starts pointing you at every place that needs to change.
And if the tool saves you time, consider sponsoring its maintainers. Both PHPStan and Psalm are largely the work of a small number of people.
The full talk from PHP UK 2022 covers all of this with more examples.
What's the best bug static analysis has found for you?
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.