Static analysis (Part 2): Code quality tools

In part 1 of the series we started out with code style tools. Checking things like indention and whitespace, that is the entry level to static code analysis. This time we go one step further. This time we are trying to gain deeper insights into our code.

There are a million and one code quality tools out there (take for example this list) which are trying to analyze the code for different aspects. They all share a common goal though: to improve the code quality.

We’ll be coming back to the SPA tutorial one more time. This time we are focusing on the backend, because this is were all the logic of the application is.

The big five

These are the five code quality tools currently supported by PhpStorm:

I already covered PHP-CS-Fixer in the last post, so we’ll skip it today.

PHP_CodeSniffer

PHP_CodeSniffer is a set of two PHP scripts; the main phpcs script that tokenizes PHP, JavaScript and CSS files to detect violations of a defined coding standard, and a second phpcbf script to automatically correct coding standard violations. PHP_CodeSniffer is an essential development tool that ensures your code remains clean and consistent.

https://github.com/squizlabs/PHP_CodeSniffer

Basically this is another code style tool which can also fix code style violations. It is pretty similar to PHP-CS-Fixer. I wouldn’t say PHP-CS-Fixer is the better tool, but it is definitely easier to use. It is extensively documented. It has several built-in rule sets which are easily customizable using a simple config file format. And it is already set up in our project.

PHPMD

What PHPMD does is: It takes a given PHP source code base and look for several potential problems within that source.

https://phpmd.org/

PHPMD analyzes the code in order to find things like:

  • Possible bugs
  • Suboptimal code
  • Overcomplicated expressions
  • Unused parameters, methods, properties

It uses different approaches for this. One is to look for code smells, which are patterns in the code which hint at errors or at least suboptimal code. This could be things like unused properties or methods, assignments in if clauses but also more controversial things like using else expressions. Yes, PHPMD considers an else expression a code smell.

Another approach PHPMD takes is to gather certain metrics about the code. It checks the length (LOC) of methods and classes or the number of properties in a class. It also calculates complexity measurements like the cyclomatic complexity or the NPath complexity. The cyclomatic complexity basically counts the number of “decision points” (e.g. if, while or for expressions) within a method. PHPMD rates everything above 11 as “very high complexity” and advises against it. The NPath complexity is pretty similar. It is the number of acyclic execution paths through that method. Usually, if one of these measurements is above the threshold the other is as well.

My take so far: I like the complexity measurements as they are hard to argue with. They provide the mathematical prove when code is too complex. I’m a bit more hesitant when it comes to the code smells. Completely ruling out else expressions?

Setup and Usage

We’ll set up PHPMD using a PHAR file (see part 1 of the series for a discussion about the different ways to set up the tools):

 wget https://phpmd.org/static/latest/phpmd.phar

This allows us to use PHPMD from the command line. The call requires three arguments:

  • the source code file or directory to analyze
  • a report format (one of ansi, html, json, text, xml)
  • a rule set, either as on or multiple XML files or as list of rule set names

To run PHPMD on our src/ directory with all the rules and report all violations in html format you’d call (the --reportfile option redirects the output to the given file):

php phpmd.phar src/ html cleancode,codesize,controversial,design,naming,unusedcode --reportfile phpmd.html

For our SPA project the report is pretty small and rather boring. PHPMD only complains about too short variables:

PHPMD report

The report summarizes all violations ordered by different criteria. At the bottom of the page it lists every single violation, including a description and the violating code.

This is a really comprehensive report and it looks much nicer than in previous versions. One issue: the detailed listing does not contain the name of the rule that was violated. Which means you can’t search for a rule. That’s no big deal on a tiny dummy project. But it is on real life projects where reports can easily contain thousands of violations.

PHPMD can be customized to your liking. You can choose from the 6 available rule sets. You could also take only certain rules or take a rule set and exclude certain rules from it. You can even configure the rules themselves. You can get all the details in PHPMD documentation.

Summary

To wrap this up: PHPMD is a really useful tool to gain insights on your code. It is easy to set up and highly customizable. Some of the rules are pretty controversial. But that’s not necessarily a bad thing, as it makes you think about things you have always done in a certain way.

PHPStan

PHPStan: Find Bugs In Your Code Without Writing Tests!

https://phpstan.org

PHPStan classifies it’s code quality checks by levels. There are currently 9 levels, 0 being the loosest, 8 the strictest. Level 0 contains checks for unknown classes or functions or passing the wrong number of arguments to a method or function. The next levels check for things like dead code, missing typehints or calling methods on nullable types.

The idea behind the levels is to work your way up in terms of code quality. You start with the basic checks. Once you have fixed everything on that level (and get time for a refactoring) you can move up to the next one.

Another nice feature is the so-called baseline. Imagine you want to introduce PHPStan to a rather large codebase. You might end up with lots of errors in the existing code and probably no time to fix them all. The baseline allows you to apply the strict checks only on new code. PHPStan basically takes a snapshot of the current state of the project. After that it will only report errors in new and changed code.

Setup and Usage

We’ll set PHPStan up using a PHAR file:

wget https://github.com/phpstan/phpstan/releases/download/0.12.80/phpstan.
phar

To run PHPStan from the command line we simply point it to our source code by using the analyze command like this:

php phpstan.phar analyze src/

 7/7 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%



 [OK] No errors


� Tip of the Day:
PHPStan is performing only the most basic checks.
You can pass a higher rule level through the --level option
(the default and current level is 0) to analyse code more thoroughly.

It reports zero errors which means our code is great 😉 Well, not quite. As you can see in the command output, PHPStan runs on level 0 by default, which contains only the most basic checks. So let’s crank this up to 11, I mean, 8.

php phpstan.phar analyze src/ --level=8
 7/7 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------------------------------------------------------
  Line   Controller/MovieController.php
 ------ ------------------------------------------------------------------------------------------
  44     Cannot access offset 'description' on array|object|null.
  44     Cannot access offset 'title' on array|object|null.
  61     Parameter #1 $string of method Psr\Http\Message\StreamInterface::write() expects string,
         string|false given.
 ------ ------------------------------------------------------------------------------------------

 ------ -----------------------------------------------------------------------------------------------
  Line   Core/Infrastructure/Repository/MovieRepository.php
 ------ -----------------------------------------------------------------------------------------------
  39     Parameter #1 $id of class SPA\Core\Domain\Entity\Movie constructor expects string, int given.
 ------ -----------------------------------------------------------------------------------------------


 [ERROR] Found 4 errors

PHPStan reports completely different things than PHPMD before. Let’s look at these violations in detail.

The first three reported issues are in MovieController::addMovie which looks like this:

public function addMovie(Request $request, Response $response): Response
{
    $body = $request->getParsedBody();
    $movie = $this->addMovieCommand->execute($body['title'], $body['description']);

    return $response->withStatus(201, sprintf('Movie id %s created', $movie->getId()));
}

We start by getting the parsed body from the request. I treated the result as an array, but PHPStan tells us that this is not necessarily correct. The result could also be an object or null. So let’s add a type check there and see if PHPStan will recognize this.

public function addMovie(Request $request, Response $response): Response
{
    $body = $request->getParsedBody();

    if (!is_array($body)) {
        throw new \Exception('Failed to parse body');
    }

    $movie = $this->addMovieCommand->execute($body['title'], $body['description']);

    return $response->withStatus(201, sprintf('Movie id %s created', $movie->getId()));
    }

Running the tool again:

php phpstan.phar analyze src/ --level=8
 7/7 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------------------------------------------------------
  Line   Controller/MovieController.php
 ------ ------------------------------------------------------------------------------------------
  66     Parameter #1 $string of method Psr\Http\Message\StreamInterface::write() expects string,
         string|false given.
 ------ ------------------------------------------------------------------------------------------

 ------ -----------------------------------------------------------------------------------------------
  Line   Core/Infrastructure/Repository/MovieRepository.php
 ------ -----------------------------------------------------------------------------------------------
  39     Parameter #1 $id of class SPA\Core\Domain\Entity\Movie constructor expects string, int given.
 ------ -----------------------------------------------------------------------------------------------


 [ERROR] Found 2 errors

Great! We are down to 2 errors. The next issue is in a different method of the same class:

public function listMovies(Response $response): Response
{
    $movies = $this->findAllMoviesQuery->execute();
    $movieData = array_map(function (Movie $movie) {
        return $movie->toArray();
    }, $movies);
    $payload = json_encode($movieData);
    $response->getBody()->write($payload);

    return $response->withHeader('Content-Type', 'application/json');
}

It’s pretty similar to the first issue. I haven’t checked the result of the json_encode call, I just assumed that it would be a string. So let’s add another type check there:

$payload = json_encode($movieData);
        
if (!is_string($payload)) {
    throw new \Exception('Json encoding failed');
}
        
$response->getBody()->write($payload);

PHPStan likes this as well. The last error is reported at MovieRepository::addMovie:

public function addMovie(string $title, string $description): Movie
{
    $id = count($this->movies) + 1;
    $this->movies[$id] = new Movie($id, $title, $description);
    $this->writeFile();

    return $this->movies[$id];
}

As we are not using a database which would auto-generate IDs for new entities, I simply counted the number of movies and increased that by 1. That means the ID is an integer but the constructor of the Movie class expects a string value. So let’s cast the $id variable to a string and see if PHPStan is happy now.

$id = count($this->movies) + 1;
$id = (string) $id;
$this->movies[$id] = new Movie($id, $title, $description);
php phpstan.phar analyze src/ --level=8
 7/7 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%



 [OK] No errors

Yeah! This is fun.

All these issues might seem minor but this is were lot’s of bugs come from. Slight oversights and wrong assumptions. Which PHPStan finds for us.

Summary

I really like the level approach of PHPStan. It allows you to gradually improve your code. I also really like how PHPStan finds these small issues which often lead to bugs later on.

PSALM

Give PHP the love it deserves

https://psalm.dev/

PSALM combines the approaches of PHPMD and PHPStan in terms of configuring the tool to your needs. It has levels as PHPStan but it is also highly customizable as PHPMD. BTW: PSALM has it’s levels in the opposite order as PHPMD: level 1 refers to the most strict analysis, level 8 contains the loosest checks. It comes with an XML configuration file format (as PHPMD) and a baseline feature (as PHPStan).

So let’s jump straight in.

Setup and Usage

We’ll set PSALM up using a PHAR file:

wget https://github.com/vimeo/psalm/releases/latest/download/psalm.phar

We’ll use the PHAR to initialize the project:

php psalm.phar --init
Calculating best config level based on project files
Scanning files...
Analyzing files...

░E░░░░E

Detected level 5 as a suitable initial default
Config file created successfully. Please re-run psalm.

This performs an initial scan of the project to determine a suitable error level and generates the config file base on this result. The generated config for our project looks like this:

<?xml version="1.0"?>
<psalm
    errorLevel="5"
    resolveFromConfigFile="true"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns="https://getpsalm.org/schema/config"
    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
>
    <projectFiles>
        <directory name="src" />
        <ignoreFiles>
            <directory name="vendor" />
        </ignoreFiles>
    </projectFiles>
</psalm>

The error level was set to 5 which is rather low. The tool also found our source code directory. Finally, it decided to ignore the vendor directory. I’m slightly embarrassed by the low error level, but I really like the automatic setup.

Let’s run PSALM for the first time and see what it comes up with. I reverted all the changes made while running previous tools.

php psalm.phar
Scanning files...
Analyzing files...

░░░░░░░

------------------------------
No errors found!
------------------------------
7 other issues found.
You can display them with --show-info=true
------------------------------

Checks took 2.10 seconds and used 72.995MB of memory
Psalm was able to infer types for 90.0826% of the codebase

No errors on level 5. As suggested, let’s re-run it with the --show-info option:

php psalm.phar --show-info=true
Scanning files...
Analyzing files...



INFO: PossiblyNullArrayAccess - src/Controller/MovieController.php:44:50 - Cannot access array value on possibly null variable $body of type arra
y<array-key, mixed>|null|object (see https://psalm.dev/079)
        $movie = $this->addMovieCommand->execute($body['title'], $body['description']);


INFO: PossiblyInvalidArrayAccess - src/Controller/MovieController.php:44:50 - Cannot access array value on non-array variable $body of type object (see http
s://psalm.dev/109)
        $movie = $this->addMovieCommand->execute($body['title'], $body['description']);


INFO: PossiblyNullArgument - src/Controller/MovieController.php:44:50 - Argument 1 of SPA\Core\Application\Command\AddMovie::execute cannot be null, possibl
y null value provided (see https://psalm.dev/078)
        $movie = $this->addMovieCommand->execute($body['title'], $body['description']);


INFO: PossiblyNullArrayAccess - src/Controller/MovieController.php:44:66 - Cannot access array value on possibly null variable $body of type array<array-key
, mixed>|null|object (see https://psalm.dev/079)
        $movie = $this->addMovieCommand->execute($body['title'], $body['description']);


INFO: PossiblyInvalidArrayAccess - src/Controller/MovieController.php:44:66 - Cannot access array value on non-array variable $body of type object (see http
s://psalm.dev/109)
        $movie = $this->addMovieCommand->execute($body['title'], $body['description']);


INFO: PossiblyNullArgument - src/Controller/MovieController.php:44:66 - Argument 2 of SPA\Core\Application\Command\AddMovie::execute cannot be null, possibl
y null value provided (see https://psalm.dev/078)
        $movie = $this->addMovieCommand->execute($body['title'], $body['description']);


INFO: InvalidScalarArgument - src/Core/Infrastructure/Repository/MovieRepository.php:39:40 - Argument 1 of SPA\Core\Domain\Entity\Movie::__construct expects
 string, positive-int provided (see https://psalm.dev/012)
        $this->movies[$id] = new Movie($id, $title, $description);

This looks somewhat familiar, as these are basically the same things that PHPStan reported. With the same fixes as before PSALM is happy, at least on level 5.

So let’s go all-in and run PSALM with the most strict checks. To do that we change the error level in the config file:

<?xml version="1.0"?>
<psalm
    errorLevel="1"
    resolveFromConfigFile="true"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns="https://getpsalm.org/schema/config"
    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
>
    <projectFiles>
        <directory name="src" />
        <ignoreFiles>
            <directory name="vendor" />
        </ignoreFiles>
    </projectFiles>
</psalm>
php psalm.phar
Scanning files...
Analyzing files...

░E░░░░E

ERROR: MixedArgument - src/Controller/MovieController.php:49:50 - Argument 1 of SPA\Core\Application\Command\AddMovie::execute cannot be mixed, expecting st
ring (see https://psalm.dev/030)
        $movie = $this->addMovieCommand->execute($body['title'], $body['description']);


ERROR: MixedArgument - src/Controller/MovieController.php:49:66 - Argument 2 of SPA\Core\Application\Command\AddMovie::execute cannot be mixed, expecting st
ring (see https://psalm.dev/030)
        $movie = $this->addMovieCommand->execute($body['title'], $body['description']);


ERROR: MixedAssignment - src/Core/Infrastructure/Repository/MovieRepository.php:52:13 - Unable to determine the type that $data is being assigned to (see ht
tps://psalm.dev/032)
            $data = json_decode($raw, true);


ERROR: MixedAssignment - src/Core/Infrastructure/Repository/MovieRepository.php:54:31 - Unable to determine the type that $item is being assigned to (see ht
tps://psalm.dev/032)
            foreach ($data as $item) {


ERROR: MixedArrayOffset - src/Core/Infrastructure/Repository/MovieRepository.php:55:17 - Cannot access value on variable $this->movies[mixed] using mixed of
fset (see https://psalm.dev/031)
                $this->movies[$item['id']] = new Movie(


ERROR: MixedArrayAccess - src/Core/Infrastructure/Repository/MovieRepository.php:55:31 - Cannot access array value on mixed variable $item (see https://psal
m.dev/051)
                $this->movies[$item['id']] = new Movie(


ERROR: MixedArrayAccess - src/Core/Infrastructure/Repository/MovieRepository.php:56:21 - Cannot access array value on mixed variable $item (see https://psal
m.dev/051)
                    $item['id'],


ERROR: MixedArgument - src/Core/Infrastructure/Repository/MovieRepository.php:56:21 - Argument 1 of SPA\Core\Domain\Entity\Movie::__construct cannot be mixe
d, expecting string (see https://psalm.dev/030)
                    $item['id'],


ERROR: MixedArrayAccess - src/Core/Infrastructure/Repository/MovieRepository.php:57:21 - Cannot access array value on mixed variable $item (see https://psal
m.dev/051)
                    $item['title'],


ERROR: MixedArgument - src/Core/Infrastructure/Repository/MovieRepository.php:57:21 - Argument 2 of SPA\Core\Domain\Entity\Movie::__construct cannot be mixe
d, expecting string (see https://psalm.dev/030)
                    $item['title'],


ERROR: MixedArrayAccess - src/Core/Infrastructure/Repository/MovieRepository.php:58:21 - Cannot access array value on mixed variable $item (see https://psal
m.dev/051)
                    $item['description']


ERROR: MixedArgument - src/Core/Infrastructure/Repository/MovieRepository.php:58:21 - Argument 3 of SPA\Core\Domain\Entity\Movie::__construct cannot be mixe
d, expecting string (see https://psalm.dev/030)
                    $item['description']


------------------------------
12 errors found
------------------------------

The 12 errors fall into 3 categories:

But they are all down to one thing: PSALM cannot infer the type of a value. It than treats it as mixed and checks if that is valid for subsequent usage. Which apparently failed on these 12 occasions. So PSALM takes this one step further than PHPStan.

Summary

PSALM comes with a nice automatic configuration. It combines the level approach of PHPStan with the highly customizable configuration from PHPMD. From this short test run it looks like it performs the most thorough analysis.

Conclusion

PHP’s type system was greatly improved over the last couple of versions. You can now assign types to class properties, method parameters and return values. But it is still an interpreted language. Type errors which might come up are only detected at run time. Code quality tools can help to prevent these errors from making it to production.

On top of that, they can help us to write code that is easier to understand and maintain. This comes in the form of collecting metrics and limiting the complexity based on these numbers. They also try to look for common patterns which might lead to problematic code.

Some of the reported violations might seem controversial, especially if you activate the most strict checks. But they can still provide valuable insights on the code, even if you don’t agree with all of them.

You can find the current version of the code in the repository.

The series concludes with Part 3: The wrap-up