Static Analysis with Psalm PHP
Time to read: 5 minutes
October 16, 2019
Written by
PHP is great, but its loosely-typed goodness is a double-edged sword. On the one hand, it gives PHP the legendary low barrier to entry, but on the other, magical type juggling inevitably means bugs.
Because PHP is an interpreted rather than compiled language, catching these type problems is tricky, and often they'll go unnoticed for a long time before they catch up with us somewhere down the road. Problems sit and fester in our code base waiting to strike. Unused code can clutter our application, and there is a myriad of other ways in which we write PHP that won't produce errors but are bugs.
There are some excellent tools to help us catch errors and find bugs before we even run our code. IDEs like PhpStorm can inspect our code and give us information on potential problems before a script is ever executed.
Catching problems with our code before we run it is delightful. Bugs not making it out of our editor is a productive state to be in, but what if we're not using an expensive IDE, or we ignore the warnings and commit the code regardless? In an ideal world, we want to add the code inspection step into our continuous integration system so that we can reject buggy code before it makes it to production.
On this week's stream, we looked at Psalm, a PHP static analysis tool from the fine video hosting company Vimeo, that can find bugs in your code from the command line. It's handy in a couple of situations, bringing an unwieldy code base into line, and keeping it clean.
Finding Problems
We're going to look at the open-source smoke testing tool Cigar. It's a cool project to investigate as it's not too large, which makes dipping our toes into static analysis much more manageable. Once we've cloned the project from GitHub and installed the dependencies with Composer, we can run the test suite to make sure everything is passing as a baseline. Cigar uses the atypical Kahlan test runner, so to run the tests we call:
I ran this on a Mac development environment, but everything should work just fine on Windows or Linux
The tests pass, so we can assume that this code is bug-free, right?
Let's install Psalm and see if it has any opinions on the quality of this code. We install Psalm using Composer and declare it as a development dependency.
We can tell Psalm to generate a default config file for us, but first, we need to talk about strictness levels. Psalm can work at a variety of levels to let you solve the most critical problems first, working your way up to the most strict level. Currently, 8 is the loosest level, and 1 is the strictest. Level 1 expects everything to be type-defined and can be a struggle for even a medium-size codebase, so as a start, let's get Psalm to create a psalm.xml config file at level 8 and see what it has to offer us.
The first parameter tells Psalm where to find our source code, and the second is the strictness level.
Now, we can run Psalm, cross our fingers, and hope it's not too bad.
Interestingly, Psalm found no errors, which is a very encouraging start. It did find 22 other issues, reported as INFO lines in the output. Anything reported as INFO at a lower strictness level gets reported as an error at higher strictness levels, because we plan on working up through the strictness its safe for us to ignore these INFO issues.
Hooray, Psalm says we have no errors at the lowest strictness level. Lets ramp it up a little and see how Psalm fairs at strictness level 3 which is the default. You can manually edit the generated psalm.xml file, but I find it easier to delete the file, and re-initialise with the new level.
Now we have 4 errors to get our teeth into. All four of these errors are to do with nullable types. Let's take a look at the code around the first error.
Psalm has noticed that while we are doing some standard dependency injection property setting, our types don't match. The second parameter of the constructor tells us to expect a string as the second $authorizationHeader with a default value of null, yet the docblock annotation for the $authorizationHeader property of our class states that the property will always be a string. Which is correct? If we think the string should be able to be null, we can fix it by updating the docblock to make it a string or null.
But should this string ever be null? Fixing these problems makes us consider how types are working across our code. It might be more reasonable to never expect that string to be null and to set it to an empty string by default. We need to consider how these values are used across the code and consider reasonable types and values.
Running Psalm again shows us only 3 errors, so we've fixed one problem.
It may seem petty to flag an incorrect type annotation as a problem, but these things are essential in a project, and become more critical the more extensive a project or team becomes. We rely on type annotations when upgrading applications to the typed properties that ship in PHP 7.4. Annotations that are currently informational will become code that’s enforced. Let's get things right now, ready for the exciting new shiny tools of the future.
Let's fix one more issue, the next one in the list.
It seems like we're passing in a null value into curl_multi_exec when it's expecting an int.
We're setting $running to null, and as curl_multi_exec executes all the HTTP requests we've asked it to, it updates the value of $running to 1. Once it's finished, it sets $running to 0. It’s a wonderful example of PHP's Fractal of Bad Design, but it is also a genuine bug in our code. We're expecting $running to be null if the code isn't running, but the function we're using is expecting it to be 0. It is a prime example of where PHP's magical type coercion is harming us hugely. In an ideal world Curl would throw an error here telling us exactly what Psalm is telling us because this type mismatch will hurt us somewhere down the line.
To fix the problem, we change the initial value of $running from null to 0.
When we rerun Psalm, we've fixed this problem. If you're interested in how we fixed the problems up to maximum strictness, take a look at pull requests #41 and #42 that fix all the Psalm issues.
Running as part of the Build
Now that we have our code ship-shape and Bristol fashion it must stay that way. We want our continuous integration system to fail the build if we introduce any Psalm detected problems. If we commit our new composer.json with Psalm as a dev-dependency, it's as straightforward as adding a new step to the script: block if we're using Travis.
Psalm fails with an exit code of 1 if it finds errors, just like our test runners and code quality tools. We can run it as part of the build, and the build fails if Psalm finds any problems.
Quality is hugely important in modern software development, and static analysis tools like Pslam are essential in producing quality maintainable applications. Let me know on Twitter or in the comments how you find Psalm, and what tools or libraries you'd like me to investigate in the future. I can't wait to see what you build.
Related Resources
Twilio Docs
From APIs to SDKs to sample apps
API reference documentation, SDKs, helper libraries, quickstarts, and tutorials for your language and platform.
Resource Center
The latest ebooks, industry reports, and webinars
Learn from customer engagement experts to improve your own communication.
Ahoy
Twilio's developer community hub
Best practices, code samples, and inspiration to build communications and digital engagement experiences.