I recently heard this interesting question: if your project uses a static analysis tool like PHPStan or Psalm (as it should), should the tests by analysed too?
The first thing to consider: what are potential reasons for not analysing your test code?
Why not?
1. Tests are messy in terms of types
Tests may use mocks, which can be confusing for the static analyser:
$object = $this->createMock(MockedInterface::class);
The actual type of $object
is an intersection type, i.e. $object
is both a MockObject
and a MockedInterface
, but the analyser only recognizes MockObject
. You may not like all those warnings about "unknown method" calls on MockObject $object
so you exclude test code from the analysis.
2. Static analysis is slow
Static analysis is slow, so you want to reduce the amount of code the analyser has to process on each run. One way to do this is to ignore all the test code.
3. Production code is more important because it gets deployed
Besides performance, another justification for excluding the test code may be that production code is more important. It has to be correct, because it will be deployed. A bug in production code is worse than a bug in test code.
Why yes?
I think we've covered the three major objections against analysing test code. If you have other suggestions, please let them know in the comments! Anyway, let's tackle the objections now, because (as you may have guessed): I think we should have our test code analysed too.
1. Mock types can easily be improved
Static analysers support intersection types so a mocked object can be annotated as MockObject & MockedInterface
. They also have plugins or extensions that can derive the resulting type for you:
$object = $this->createMock(MockedInterface::class);
// Derived type of $object: MockObject & MockedInterface
2. Static analysers have a cache
Both PHPStan and Psalm use a cache so they don't have to analyse the entire code base over and over again. You won't notice any difference if you analyse all your code or just production code (quick tip: if you run the analyser in a Docker container, make sure that the cache directory is not lost after each run; configure it to be within one of the bind-mounted volumes).
3. Test code is just as important as production code
Of course, it's the production code that gets deployed so its quality needs to be guarded. However, tests play another important role in quality assurance. To care less about your tests means you'll have trouble maintaining both production code and test code. Test code also deserves to be refactored, and its design needs to evolve over time. When doing so, it will be very important to get feedback from static analysers.
Additional benefits
Separately testing behavior
Statically analysing all the code before running the tests is a great way to ensure that the tests themselves don't throw any basic errors, like wrong number of method arguments, type mismatches, etc. This allows for a more clear definition of the role of tests versus static analysis: static analysis can tell you that your code will (most likely) run, and the tests can tell you if the code actually implements the behavior you expect from it.
Probing
Running static analysis on your entire code base allows for different refactoring workflows too. Consider a common situation where a method needs an extra required argument. A traditional workflow is:
- Add an optional argument to this method.
- Find all usages of this method with your IDE.
- On each call site, pass the new argument.
- Finally make the argument required.
At every stage, the code and tests keep working and every step can be committed.
Another approach could be:
- Add that extra required argument.
- Run the static analyser and find out what no longer works.
By doing so you might not be prepared to stop at any time. However, it does follow the Mikado style; the first step of which is to Just Do It and find out all the prerequisites of the change. So it's not a better workflow per se, but I think it's interesting to have a different option when you're refactoring. You could call this option "probing".
If you want all these advantages, you have to statically analyse all the code of your project anyway. You need to know about every part that would potentially lead to problems if you make a change. But before you can trust your static analyser to support you, you have to fix its "shady-typed" areas. This is a good idea anyway, because "bugs creep in when types can't be inferred".
Of course, that's not always an option, in which case there's always the "baseline" that you can generate. Only new problems will appear, and even that is a very useful help in refactoring projects.
Another reason for yes: Test code should serve as an example of how to use the code that it's testing. If you allow yourself to write test code that your static analyzer would ban in production then you won't be able to use it as an example, and you've lost an opportunity to improve the code under test to make it easier to use.