Static Analysis Has Come To Vanilla
Hi devs! I just wanted to inform you that the static analyzer known as Psalm has come to Vanilla!
What is static analysis?
Static analysis is an automated tool that is run on code that analyzes it for potential errors. It follows code paths to see if there are any easily identifiable errors in our code. What kinds of errors you may ask? There are too many to list, but here are some good ones.
- Calling functions with the wrong number of parameters.
- Using undeclared variables.
- Improper subclassing.
- Type mismatches
From detecting typos, to bugs that may take down a site, static analysis can help find things that humans often miss.
Why static analysis?
Static analysis is just another tool in our toolbox of code quality. It detects probable bugs. It helps keep our code clean and consistent. And the best part: we get this all essentially for free. We don't have to write specific tests for static analysis to work. It just runs in our code and tells us about problems.
What's the catch?
There isn't really much of a catch. However, there are a few strings attached with static analysis.
- Static analysis can report some false positives. This means it will tell us there is something wrong when we know there isn't. In these cases there are ways to address this (hint: read the docs). That being said: you may find addressing these issues a little tedious.
- We are not quite at the point where we have full code coverage with psalm. Turning on a tool like this on a mature code base always reports a tonne of errors. In our case this amounts to hundreds of errors. Not to worry though: I've dialed down the strictness of the tool to start in order to give us time to address some issues at least. I'll dial it up slowly over time as I am able to fix things. If you want to help with this then come talk to me.
What should I do to be ready?
In order to not get your knuckles wrapped by Psalm it’s best to code cleanly. Luckily code sniffer already detects a lot of what psalm needs to do its thing. Other than that I suggest the following:
- Read Psalm's docs! They explain the tool better than I can and are not too long of a read.
- Make sure you type hint as much as possible and make sure they are correct. Type hints allow for code analysis more than anything else. You can use
@paramhints or real type hints. Psalm will use both. - For new functions avoid multiple allowed types for parameters. This is manifested a lot in our codebase where we take an array parameter or result, but default it to
false. This is a bad habit that we should avoid going forward. - Pay attention to PHPStorm's inspections. PHPStorm already does a measure of static analysis. If you fix its errors then you are one step closer to a clean bill of health from Psalm.
Anyway, you will start seeing Psalm as part of our build process. You can also run it locally from your vanilla directory with:
./vendor/bin/psalm
Happy coding!
Comments
-
That's awesome - lots of benefits, including security, from Static Analysis. Will be interesting to see how we use it down the road!
0 -
@Paul Johnston, I haven't enabled the security analysis yet, but one of your questionnaires was definitely one of the reasons why I decided to get moving on this again. I saw that Psalm announced taint analysis shortly after you asked some security questions.
I think the plan is to see what enabling the taint analysis on our PHP views looks like first. That will give us some automated coverage for XSS attacks which is one of our most common Hacker One reports.
We are generally pretty good on SQL injections because our database layer offers a base level of protection, but the extra layer will still be nice to see.
1 -
Glad to hear it! A progressive roll out sounds good and make sense.
0 -
Here's a tip: If you install the PHPStorm plugin Awesome Console then you can click on file errors that psalm outputs to go right to the line.
0