29 Aug 2014

Introducing new rules for static methods and interfaces

SensioLabsInsight performs more than 100 code quality checks for your PHP projects, such as Laravel applications and Drupal modules. Based on the experience of our Audit Team, we are constantly adding new checks or rules. This week, we've just released two new rules.

Static method should not contain $this reference

Static methods of PHP classes should not use $this but self to avoid errors at runtime. Although this is an easy rule to follow, it's not uncommon to introduce the following errors when refactoring your application code:

<?php

class FileUtils
{
    private $contents = array();

    // This method used to be static
    // But to handle caching, it had to become an instance method
    public function getContents($filePath)
    {
        if (isset($this->contents[$filePath])) {
            return $this->contents[$filePath];
        }
        if (!file_exists($filePath)) {
            throw new \InvalidArgumentException(
                sprintf('Required file "%s" does not exist', $filePath)
            );
        }

        return $this->contents[$filePath] = file_get_contents($filePath);
    }

    public static function countNbLines($filePath)
    {
        // when getContents was turned to non-static, the developer updated
        // the call below. Unfortunately, they forgot to remove the static
        // keyword in the countNbLines() definition
        return substr_count($this->getContents($filePath), "\n");
    }
}

Imagine overviewing this error until it's too late and your application is deployed in production! That's exactly what SensioLabsInsight is for: we keep on adding rules preventing bugrisks in production, to make your code always more robust.

Interfaces names should end with "Interface"

One of the main goals of SensioLabsInsight is to promote the good practices used by professional PHP applications. In the past we couldn't enforce those code checks because they could affect your analysis results.

However, now that the chocolate medals are gone we'll start adding lots of these code style checks on a regular basis.

The first one of these rules checks whether your interface classes include the Interface suffix:

<?php

// Wrong interface name
interface Storage { ... }

// Wrong interface name
interface StorageInt { ... }

// Correct interface name
interface StorageInterface { ... }

Although this is not the only way to name interfaces, it's probably the easiest one to follow consistently in your codebase. Some developers prefer to use names such as Loggable and Cacheable. The problem with this approach is that sometimes is really hard to come up with a good name that follows this strategy.

In any case, as any other SensioLabsInsight rule, you can tweak its behavior. To do so, edit your project configuration and add the interface_name_pattern option to define the regular expression used to validate the name of the interfaces:

rules:
    # ...
    php.interface_has_no_interface_suffix:
        enabled: true
        interface_name_pattern: /(Interface|Exception|able)$/

You can also disable all violations reported by this rule adding the following option to your project configuration:

rules:
    # ...
    php.interface_has_no_interface_suffix:
        enabled: false

False positives, tweaks and bugfixes

As always, we keep on tuning these and the other rules for false positives. Don't hesitate to report us any false positives on the new rules.

comments powered by Disqus