PHP
Article

Inspecting PHP Code Quality with Scrutinizer

By Bruno Skvorc

This article has been slightly revised based on the feedback from the people behind Scrutinizer. For the most part, the changes are in the “Configuration” section which is now noticeably simpler.

We’ve gone through a decent number of tutorials about code quality, inspections, auto-build systems and so on here at SitePoint:

In this article, we’ll take a look at Scrutinizer CI – a continuous integration tool that’s quite expensive and closed to private projects, but very handy for public ones.

Scrutinizer vs/+ Travis

Scrutinizer performs many analyses that a compiler does in order to help you find potential bugs, security vulnerabilities, or violations of best practices. It also allows you to combine its results with those of some open-source tools like PHP Code Sniffer that we discussed in our 4-part series on Jenkins.

Where Travis is fully customizable, with various virtual environments running your code and letting you know of your build status, it doesn’t really have built-in support for quality assurance. Scrutinizer does, but it doesn’t run tests on public projects, and to power private ones, you need a paid plan.

Scrutinizer, therefore, cannot run PHPUnit for you, nor can it provide build status or code coverage. We can, however, configure Travis to send coverage reports to Scrutinizer on every build. That way, whenever Travis makes a new build of your project, it automatically makes sure the Scrutinizer report is up to date, too.

External Code Coverage

To get started with Scrutinizer, sign up for an account there. Then, connect your Github account with it and authorize access, so it can reach your repositories and verify you’re the owner. Finally, click the Add Repository button and add the one you want checked. Scrutinizer will automatically add a webhook to your repo, so that all events happening on it automatically trigger a scan process:

Github Scrutinizer Webhook

I’ll assume you already know how to configure Travis. If not, please see this post and come back. At the end of our .travis.yml file, we need to add the following:

script:
  - phpunit --coverage-text --coverage-clover=coverage.clover

after_script:
  - wget https://scrutinizer-ci.com/ocular.phar
  - php ocular.phar code-coverage:upload --format=php-clover coverage.clover

This first instructs Travis on what to do in order to trigger a build process: run PHPUnit and produce a clover coverage report. After the build executes, Travis needs to download the ocular.phar helper from Scrutinizer, and uses this tool to send the coverage report over. This means that, if you prefer to bypass Travis altogether, you can use this tool from your own machine, too – just run the same commands locally.

Configuration

When you add your project, Scrutinizer will automatically infer a configuration based on your project structure. It recognizes many common frameworks and CMSes like Symfony, Zend, Laravel, Drupal, Magento, or WordPress, and will automatically adjust its analysis to match that particular project. Scrutinizer also allows you to change its behavior through a config file. The configuration process is a bit too versatile for comfort, so I’ll try and simplify as best I can.

Global / Repo

There are configurations that you save on the Scrutinizer website, into your account – global, and repo. Global configuration can be shared among repos (but doesn’t have to be – the word global means it’s globally available, not globally active).

Repo configuration is configuration saved in a repo’s configuration section:

Configuration screen

At first, it’ll be empty, with some default values implied. To fine-tune it, one can just put in custom values as per the docs. In the case of thephpleague/skeleton, this is already beautifully defined, so you can rip off their configuration and shove it right in there:

filter:
    excluded_paths: [tests/*]
checks:
    php:
        code_rating: true
        remove_extra_empty_lines: true
        remove_php_closing_tag: true
        remove_trailing_whitespace: true
        fix_use_statements:
            remove_unused: true
            preserve_multiple: false
            preserve_blanklines: true
            order_alphabetically: true
        fix_php_opening_tag: true
        fix_linefeed: true
        fix_line_ending: true
        fix_identation_4spaces: true
        fix_doc_comments: true
tools:
    external_code_coverage:
        timeout: 600
        runs: 3

When a build process is triggered (and this will happen automatically due to the webhook), Travis will run the PHPUnit command and send over the coverage report when it’s done. A timeout of 600 seconds means that’s the maximum amount of time Scrutinizer will tolerate waiting for the code coverage report to arrive.

runs is useful for when you have your PHPUnit tests split up into several suites, each producing its own coverage report – specifying runs as X means the last X coverage reports sent to Scrutinizer will be merged together and treated as one. This is also useful when you’re testing for several environments on Travis – e.g. PHP 5.5, 5.6, and 7.0 – specifying a runs value of 3 will merge these reports. If runs is omitted, only the first coverage report Scrutinizer receives will be taken into account for code coverage and build success status.

File Configuration

File based configuration is read from a .scrutinizer.yml file from your project’s root folder. It can contain the exact same values as all other configurations, but takes priority over them – i.e., values in the file will overwrite values in repo configuration, which will overwrite those in global configuration.

The League Skeleton has one such file, and as such is ready for Scrutinizer testing by default. Feel free to use their file as an example on how to build your own.

Local Configuration

Local configuration can be defined per-run, by clicking “Schedule Inspection”, which will allow one to punch in custom config values to be applied immediately before the run, and only on that one run:

Schedule Inspection Custom Config

Local configuration is also one which you pass in via API calls as parameters.

This configuration overwrites all others before it while merging.

For a handy reference to these overwrites and the configuration hierarchy, as well as helpful hints for when to use which configuration type, see this handy chart.

Reports

Now that configuration is out of the way, let’s inspect an actual project. For an example, I’ll use a past version of my Diffbot client which we’ve built in a previous series. After adding it to Travis, configuring everything as mentioned above (using the identical configuration settings), the Scrutinizer build triggers.

Dashboard

After completion, the project dashboard changes:

Project Dashboard

The build process was considered failed originally due to a configuration misstep on my end, hence the error.

We can see that we have a very good rating in code quality, 100% test coverage, and 4 detected issues. Let’s see what those are.

Issues

Clicking on either the “4 issues” link, or the issues icon in the left-hand side menu, we’re taken to the issues list:

Issues screen

Scrutinizer offers handy tags for issue categorization by type and severity. Oddly enough, the “Last Found” column reads “2 months ago” in spite of the project just having been added to Scrutinizer and the first inspection running just mere minutes ago. In some regard, this interface leaves something to be desired – it’d be nice, for example, if bugs were color coded per severity, and if there were an “expand” option to glance at the errors without going to a whole new screen for a minor issue.

Alright then, let’s see what the problem is. I chose to inspect the second entry: Entity.php:

Entity Inspection

Besides the fact that it would be nice to have the full file path printed at the top instead of just the file name, this interface is clean and very direct in pointing out one’s mistakes. In this particular case, it actually inspected the docblock of an interface’s method signature and warned me about not respecting it.

Indeed, once I changed this:

Fixed Code

to this:

Bugged Code

and fixed a similar Api.php bug as well, I committed, pushed, and the analysis was re-run, producing a positive output:

Success Notification

It even let me know via email:

Success Notification in Email

The remaining two issues were unfixable due to an issue with Scrutinizer’s analyzer. Due to their unfixable nature, ignoring these issues was the logical approach:

Hide Issues

This doesn’t really resolve them, but it does hide them from the issues list so they don’t stand out in future inspections.

Code

Clicking on the “Code” menu option, we’re taken to the code analysis screen which tells us about the quality of our classes.

Code Quality Screen

The Product class seems problematic. Let’s see what Scrutinizer is complaining about.

Product Class Analysis

Uh huh. So it’s the complexity that’s causing the problem? Hmm, let’s see “How to fix”.

How to Fix instructions

Now, while I absolutely love this approach of giving advice with followup links, there’s just nothing that can be done for this class. The file is just one big collection of getters, and there’s no way to extract a class, subclass, or interface out of it. Sadly, there’s no way to ignore just this one inspection like there is on issues.

There is another link on the “Code” screen, one perhaps less obvious: “Hot Spots”.

Hot spots displays the most “optimizable” areas of the code – those that Scrutinizer assumes would yield the greatest quality increase with the least amount of work:

Hot Spots Listed

In my case, those were the aformentioned (unfixable) Product class, and two methods in the Api abstract class. Interesting. Let’s have a look at the buildUrl method’s problems.

BuildURL Method's problems

Hmm. Too many conditions? The buildUrl method takes various options from the API that calls it and builds a URL from them, so it’s only natural that it has “many” control structures. One could extract the loops into methods like buildFieldString and buildOptionsString, but what for? This is the only method that needs this string building, so introducing additional unnecessary overhead to fix this is not something I’m interested in.

The other method, __construct definitely leaves something to be desired. Let’s improve it somewhat and change it from this:

public function __construct($url)
    {
        if (!is_string($url)) {
            throw new \InvalidArgumentException('URL param must be a string.');
        }
        $url = trim($url);
        if (strlen($url) < 4) {
            throw new \InvalidArgumentException('URL must be at least four characters in length');
        }
        if ($parts = parse_url($url)) {
            if (!isset($parts["scheme"])) {
                $url = "http://$url";
            }
        }
        $filtered_url = filter_var($url, FILTER_VALIDATE_URL);
        if (false === $filtered_url) {
            throw new \InvalidArgumentException('You provided an invalid URL: ' . $url);
        }
        $this->url = $filtered_url;
    }

to this:

public function __construct($url)
    {
        $url = trim((string)$url);
        if (strlen($url) < 4) {
            throw new \InvalidArgumentException(
                'URL must be a string of at least four characters in length'
            );
        }

        $url = (isset(parse_url($url)['scheme'])) ? $url : "http://$url";

        $filtered_url = filter_var($url, FILTER_VALIDATE_URL);
        if (!$filtered_url) {
            throw new \InvalidArgumentException(
                'You provided an invalid URL: ' . $url
            );
        }

        $this->url = $filtered_url;
    }

Now if we trigger a rebuild by committing and pushing, we get this:

Dashboard shows improvements

The quality has increased from a B rating to A for the __construct method – success!

Inspections and Statistics

The two remaining screens are Inspections, which lists all the inspections done so far and their outcome, and Statistics and Trends, a dashboard of graphs displaying visual cues as to how the quality of your code progresses (or regresses) over time – I’ll leave those up to you to explore.

Conclusion

Scrutinizer is a powerful tool in making sure your PHP code quality is top notch. It’s very easy to set up, and once it’s up and running requires minimal interaction to stay active and up to date. While the pricing tiers aren’t all too accessible to individuals and solo developers with private projects (starting at 50 Euro rather than 19 Euro as stated on the landing page), they’re very approachable to companies, and the free tier is more than enough for an open source project.

Have you tried Scrutinizer? Which code quality services do you use? Maybe their competition, Code Climate? Let us know.

Comments
Azaru

I'm using it since january. Thanks to it, I've improved the way I'm coding a lot.Your article only cover the most obvious things, there are tons of other useful things.

As for the 19€ pricing, that's because they recently changed the way they price the licenses. Before, you had a Solo license, costing 19€, where you had access to everything, but could only have 1 private repo. That's the plan I'm currently subscribed to.

At first, I was kinda upset too by they way they manage complexity and conditions in the analysis. But in the end, I managed to respect these analysis, and I have better code. But I had to refactor a lot.

In fact, I'm getting 10, with a lot of strict coding standards in place. Code is clean, readable, commented, without any bugs.

djsmithme

Scrutinizer is nice but far too expensive to worth considering for individuals. $50/month just for Code Smells, Best Practices, Bug Detection,Running Tests and Code Coverage, all of which isn't that hard to have set/running in an IDE/locally.

https://insight.sensiolabs.com/ is free for opensource and $6/month for lite, so I see no reason to leave.

Recommended

Learn Coding Online
Learn Web Development

Start learning web development and design for free with SitePoint Premium!

Get the latest in PHP, once a week, for free.