Static code analysis has been around for a while and in the PHP world this is most often synonymous with using PHP_CodeSniffer. It is mostly used to enforce (subtly suggest) using a certain coding style. However coding styles are just that, a style – even if your code is unfashionable, it can still create a correct end result.
Wouldn’t it be great if we could also statically check if a certain piece of code is demonstrably wrong to avoid bugs in the first place?
One of the not immediately visible improvements in PHP 7.0 is that PHP got an abstract syntax tree under the hood. This in turn brought about a couple of new static code analysis tools that allow you to catch bugs earlier. I have recently come across phan and phpstan and I am going to trial both and see what we get when we apply this to a Magento 2 extension.
From their respective readmes here are only some of the checks each tool offers:
- Check that all methods, functions, classes, traits, interfaces, constants, properties and variables are defined and accessible.
- Check for type safety and arity issues on method/function/closure calls.
- Check for PHP7/PHP5 backward compatibility.
- Check for sanity with array accesses.
- Check for type safety on binary operations.
- Check for valid and type safe return values on methods, functions, and closures.
- Check for No-Ops on arrays, closures, constants, properties, variables.
- Check for unused/dead/unreachable code.
- Check for classes, functions and methods being redefined.
- Check for sanity with class inheritance (e.g. checks method signature compatibility). Phan also checks for final classes/methods being overridden, and that the implemented interface is really a interface (and so on).
- …
and phpstan.
- Existence of classes and interfaces in
instanceof,catch, typehints, other language constructs and even annotations. PHP does not do this and just stays silent instead. - Existence of variables while respecting scopes of branches and loops.
- Existence and visibility of called methods and functions.
- Existence and visibility of accessed properties and constants.
- Correct types assigned to properties.
- Correct number and types of parameters passed to constructors, methods and functions.
- Correct types returned from methods and functions.
- Correct number of parameters passed to
sprintf/printfcalls based on format strings. - …
One thing to note, my project structure for working on a Magento 2 extensions looks like this:
project ├── src ├── tests └── vendor
and I keep under vendor only the modules that are required by this extension.
Before we start taking advantage of all this goodness we need to make sure that we have the php-ast extension installed as both tools need it. So grab your favorite package manager and run
__your-package-manager__ install php-ast
(adjust as needed, for example it could be php70-ast).
Phan
I had some trouble getting the phar option to work so I used
composer require --dev phan/phan:0.8*
(double check the correct branch to use – the above is for php7.0).
Next we’ll need to create a configuration file for phan to know what we want to analyse. This file is under .phan/config.php and I started out with something like this
'directory_list' => [
'src/',
],
"exclude_analysis_directory_list" => [
'vendor/'
],
This, not unsurprisingly, brings up a lot of error messages in relation to missing classes and methods. We need to let phan know that some of the folders under vendor should be considered part of the inspection. Changing .phan/config.php to include all the direct requirements mentioned in my composer.json file like this:
'directory_list' => [
'src/',
'vendor/fooman/',
'vendor/magento/framework/',
'vendor/magento/module-backend/',
'vendor/magento/module-sales/',
'vendor/magento/module-quote/',
],
"exclude_analysis_directory_list" => [
'vendor/'
],
yielded the below results:
Before going into these let’s switch to phpstan to see what we get there.
Phpstan
I have this installed as a phar which is accessible from anywhere. It doesn’t require a configuration file and we can run with the defaults by executing
phpstan analyze src/
which would produce the below:
Comparing the two results – both tools struggle with the missing translation function __() as well as unable to work with Magento’s autogenerated Factories. The automatic code generation is a feature of Magento’s autoloading mechanism and the __() is part of the base package (never understood why).
So stopping short of requiring a full install of Magento 2 I use a minimal Magento 2 bootstrap process.
Rerunning phan we can see that we fixed the __() issue however the Factories are still not found/generated. However rerunning phpstan we now get a green result:
Why do we have this difference? Looking deeper into the Readmes of both projects phpstan does process autoload information whereas phan doesn’t.
Taking it to level 7 with Phpstan
Both phpstan and phan are designed in a way to be able to tighten the screws on its checks, which is a great feature to be able to introduce these checks on existing codebases. Phpstan is the user friendlier option for it though as we can simply pass in a different level directly:
phpstan analyze -l 7 src/
The eagle eyed reader will notice that some of the mentioned issues we now get are related to Magento’s magic getters and setters. As Phpstan is extensible we can tell it that we don’t care about these in our analysis via a plugin.
In the end we are left with error messages that we should fix – our phpdoc return type says we use an interface but we returned the actual implementation instead which is quickly fixed. Crisis averted!
In summary
Abstract Syntax Trees are awesome and have given us more tools to write better code. Try them and see how it works for you and your workflow. I have found Phpstan to be easier to work with, however as each tool has found issues which the other one didn’t I’ll keep experimenting with both and let you know how I get on over on my Magento 2 blog. What I particularly like about both is that the amount of required effort is low and showing immediate benefits aka an easy win.