PHPStan reports an error, you fix it. But paradoxically, that fix actually makes your code worse. How is that possible?
I remember going through PHPStan output once, systematically fixing one warning after another. I felt productive. Code is cleaner, types are correct, green everywhere. A month later I was scratching my head over why a function was returning an empty string when it shouldn't. An hour-long detective story, yet the culprit was obvious — me and my “fix.”
One thing first, though: PHPStan does
exactly what it should. It warns you that a function may return
null or false, and forces you to think about it.
That's great. The problem is your reaction.
An Innocent Example
Say we have a function that removes extra whitespace from text:
function normalizeSpaces(string $s): string
{
return preg_replace('#\s+#', ' ', $s);
}
PHPStan reports: Function preg_replace returns string|null but function
should return string. Sure enough, preg_replace can return
null if a regex error occurs. So let's fix it, right?
function normalizeSpaces(string $s): string
{
return (string) preg_replace('#\s+#', ' ', $s);
}
PHPStan is happy. Commit, push, done.
Except.
What You Actually Did
The original code was actually better. If preg_replace
ever returned null — say, because of… (no, nothing comes
to mind) — PHP would throw a TypeError. A fatal error. Tracy would
light up, it would show up in the log, you'd simply know about it. (Oh
right — that's why it can return null!)
After your “fix”, null silently gets cast to an empty
string. The function returns "", the application keeps running, and
you have no idea something went wrong. Data gets corrupted without any
warning.
Congratulations, you just made your code worse 🙂
And preg_replace isn't an isolated case. Plenty of PHP functions
return false or null for situations that practically
never occur in normal usage — json_encode,
ob_get_contents, getcwd, gzcompress, a
whole range of Intl functions. Every time you reach for a type cast, stop and
ask yourself: am I throwing away information about an (unlikely) error?
The Right Approach
If you want to satisfy PHPStan while preserving the original behavior, use a
throw expression:
function normalizeSpaces(string $s): string
{
return preg_replace('#\s+#', ' ', $s)
?? throw new \LogicException('preg_replace failed');
}
This says: “I know an error can theoretically occur. If it does, I want to know.” You've essentially written out explicitly what was there implicitly before — a fatal on failure. PHPStan happy, code unharmed.
But hold on. There's an even simpler way. You can just ignore these trivial
errors in PHPStan — add them to ignoreErrors in
phpstan.neon or use the @phpstan-ignore annotation.
And that's perfectly legitimate. After all, the original behavior where PHP
throws a TypeError is basically what you want. Why change the code
for that?
Even better is not having the problem at all. That's why wrappers exist —
for instance, Nette\Utils\Strings::replace() wraps
preg_replace and throws an exception on error. Similarly
Nette\Utils\Json::encode() instead of json_encode. Use
one function and the problem disappears — no null, no
false, nothing to deal with.
Another option is to solve it at the PHPStan level with an extension that
removes false or null from return types of selected
functions. For example, nette/phpstan-rules does this
for dozens
of PHP functions. For regex functions, it even checks whether the pattern is
a constant string — if so, it strips null from the type, since a
regex error can't occur.
This is an opinionated approach, of course. And that's exactly what I like about PHPStan — it's strict, and I can customize it with extensions to suit my needs.
A silent error is worse than a loud one. And (string) is the
quietest way to produce it.
