Don't mute your static analyzer
08 november 2023
3 minutes
Static analyzers like psalm and phpstan are a very powerful tool that help you prevent runtime errors on production. Chances are, that you feel like they are nitpicking all the time. Like it or not, your static analyzer is (almost) always right. This is because types don't lie - unless they are wrong: If your function expects type A, but your function returns type A|B|null - your code needs to deal with case B|null as well! Unless you like fixing urgent bugs on production.
We all know there are some strange quirks in PHP. Your static analyzer knows this as well and will tell you what is going on. So if a tool like psalm is complaining a lot, it is because you wrote error-prone code. Instead of complaining about the tool, you better take a second look at your code.
Stop suppressing!
One thing we frequently see in code reviews is that your static analyzer was complaining about a specific case. Instead of trying to understand and solve the issue, you went YOLO and added @psalm-suppress to mute the error....
By adding a suppress statement, you are muting your static analyzer tool. At this point, YOU might know what is going wrong. The other people on your team will most likely not know this. However, YOU muted the error FOREVER. This means that someone else from the team (or future you), might write code that psalm won't report anymore. This might still result in runtime errors - which you surely don't want ?!
Our advice: Don't ever add a suppress statement, unless it is really an internal typing issue! In this case, you might use a stub file as an alternative or provide a PR to your static analyzer. If you really have to add a suppress, document the WHY - preferably linked to a github issue.
@var is a lie!
Another common static analyzer "mute" approach is the usage of internal @var declarations. For example:
/** @var Foo $foo */
$foo = createFoo();This is usually done if the symbol on the right side of the = operator has a wrong type, or isn’t specific enough. Usage of inline @var is problematic for a couple of reasons:
Because it might be used to correct wrong type information of the called symbol. Your static analyzer will always trusts it. But if there’s a mistake in this annotation, the analysis of the code below might be wrong.
If the return type changes, your static analyzer will not know about this, which might result in runtime errors.
The inline
@varneeds to be repeated above all usages of the symbol which leads to repetition in the codebase.
Instead, the type should be fixed at its source. If the called symbol comes from 3rd party code, you can correct it using a stub file. If the return type differs in each call based on the passed arguments, you can use generics, or write a dynamic return type extension instead. If you really need to use an inline @var, consider an alternative - an assert() call, which can throw an exception at runtime, so the code execution doesn’t continue if the requirements aren’t met.
These invalid var declarations are often added when you work with unstructured data like raw nested JSON input objects. Instead of adding various type annotations, you are better of parsing the input and trying to coerce the input type into the type that your application expects. In general: Parse ... Don't validate !
Note: phpstan recently launched a lie detector which inspects @var declarations to determine if they are valid.
Conclusion
By muting your static analyzer, you are hiding future production bugs. Everytime you add a @var declaration to your codebase, think to yourself: there most likely is a better way of fixing the error I'm trying to solve! The same rules apply if you are working with typescript: Don't use any or @ts-ignore!
