PHPunit can check your code against PSR-2 and other coding standards

I've written at length here how it's really important to follow coding standards. Linters and auto-fixers exist for lots of different standards: Drupal's coder.module has a coder-format Drush command; and more general standards like the cross-industry PSRs are supported by Sensiolabs' own linter.

There's a problem, though, that most teams accumulate nonstandard code, much as they accumulate technical debt: in that, you have to actively think about how to reduce it for it not to increase over time. It always takes human intervention to follow a coding standard: in that, even if you have automation through your IDE or a githook, you have to get your entire team to implement that consistently, and not somehow acidentally work around it (maybe with—gasp!—a hotfix on production or a Jenkins-built codebase, where the githook doesn't exist!)

Why don't we just have the build break when nonstandard code is checked in, much as if bugs were checked in? After all, we already have a testing paradigm for automatically checking problems in our code: unit and other test suites. And while these usually focus on the functionality of the code as an API, or of the website as a UI, then they can also be used to run command-line options.

Here's a test (not really a "unit" test, I suppose) that automatically detects for violations of PSR-2, but—being a test—is necessarily idempotent and so doesn't change anything:

<?php
/**
 * @class
 * Test properties of our codebase rather than our actual code.
 */
class MetaTest extends \PHPUnit_Framework_TestCase
{
    // Possible names for php-cs-fixer, and found one.
    private $commands = array('php-cs-fixer.phar', 'php-cs-fixer');
    private $command = null;
 
    /**
     * Set up.
     */
    public function setUp()
    {
        // Attempt to look for the PSR-2 linter on the command path.
        // Downloadable from http://get.sensiolabs.org/php-cs-fixer.phar.
        foreach ($this->commands as $command) {
            $this->command = trim(shell_exec("which $command"));
            if ($this->command) {
                return;
            }
        }
    }
 
    /**
     * Test for PSR-2.
     */
    public function testPSR2()
    {
        // If we can't find the command-line tool, we mark the test as skipped
        // so it shows as a warning to the developer rather than passing silently.
        if (!$this->command) {
            $this->markTestSkipped(
                'Needs linter to check PSR-2 compliance'
            );
        }
 
        // Let's check PSR-2 compliance for our code, our tests and our index.php
        // Add any other pass you want to test to this array.
        foreach (array("src/", "tests/", "web/index.php") as $path) {
            // Run linter in dry-run mode so it changes nothing.
            exec(
                "$this->command fix --level=psr2 --dry-run "
                        . $_SERVER['PWD'] . "/$path",
                $output,
                $return_var
            );
 
            // If we've got output, pop its first item ("Fixed all files...")
            // and trim whitespace from the rest so the below makes sense.
            if ($output) {
                array_pop($output);
                $output = array_map("trim", $output);
            }
 
            // Check shell return code: if nonzero, report the output as a failure.
            $this->assertEquals(
                0,
                $return_var,
                "PSR-2 linter reported errors in $path/: " . join("; ", $output)
            );
        }
    }
}

This test, dropped into your test suite early on in development, will sit there forever more, watching your adherence to code standards like a pedantic sentinel. And if your junior developers haven't yet (managed to) implement automatic fixing in their IDE—and, let's face it: lots of IDEs are recalcitrant on this sort of thing—then your automated build process, or your pull-request approval process, will still pick up on the nonstandard code when all tests are run.

And a quick run of the linter, not in dry-run mode, will fix your codebase right now, so that you've got a good starting point for future development. Just make sure no branches are in flight, as the whitespace changes might otherwise really confuse your team when they try to merge themselves later!

Comments

Great code snippet! It helps a lot! Thank you very much!

Add new comment