WonderStyle: How We Keep Our Code Pretty

In Part 1, I talked about why having a consistent coding style matters. In a nutshell:

• Programmers spend most of our time reading code, not writing it, and
• Programmers are trained to notice inconsistency, therefore
• Programmers don't have to think as hard when the code we read is consistent.
• I have a twitching problem

At WonderProxy, most of our code is written in PHP. We use PHP_CodeSniffer to define and maintain a consistent coding style.

PHP_CodeSniffer

PHP_CodeSniffer, or phpcs, is a flexible tool that evaluates PHP code based on a set of styling rules bundled together into a coding standard. It supports several popular standards out of the box, like PEAR, PSR-1/PSR-2, and Zend Framework. phpcs can use an existing standard wholesale, mix and match individual rules as a custom standard, and even create an entirely new set of rules from scratch. At WonderProxy, we do a combination of all three.

You can add phpcs to your own project with Composer:

$ composer require --dev squizlabs/php_codesniffer

Once phpcs is installed, you'll have an executable at ./vendor/bin/phpcs. You can immediately run any of the predefined standards on your code:

$ ./vendor/bin/phpcs --standard=Zend ./src

phpcs will evaluate all the PHP code in the ./src directory using the Zend Framework coding standard, and print a list of rule violations (if they exist).

Choosing a Standard

It's easy to objectively measure coding style consistency. However, deciding which coding style to use in the first place is a subjective question, and one that you and your team will need to answer together before you can even contemplate consistency. (If your team is small and you spend more time in the backend than anyone else, "answering together" might mean "answering yourself and convincing everyone else to go along with you". Not that I speak from personal experience, of course.)

When we were defining our standard at WonderProxy, we settled on PSR-2 as a base. I don't agree with every styling choice in the PSR, but I agree with many of them, so it was a good place to start.

If we had wanted to adopt PSR-2 without any modifications, we would have been finished at this point. But no, I have opinions.

Customizing the Standard

PSR-2 requires all class names to start with a capital letter. We've got a lot of code at WonderProxy that uses many classes that have all lowercase names, so that won't work for us without significant code changes.

PSR-2 also has no opinions about array syntax. WonderProxy is a PHP 5.6 shop now, and I enormously prefer the short array syntax (i.e. [] instead of array()) that arrived with PHP 5.4. I'd like to use phpcs to make that preference consistent.

Excluding an unwanted rule

phpcs reads its coding standards from an XML file that looks like this:

<?xml version="1.0"?>
<ruleset name="The name of your custom standard">
  <description>An optional description for your standard</description>

  <!-- Include the whole PSR-2 standard: http://www.php-fig.org/psr/psr-2/ -->
  <rule ref="PSR2" />

</ruleset>

That ruleset applies the PSR-2 standard without any modifications. We want to exclude the rule that requires Pascal case for class names, so first we need to find the rule.

The built-in standards for phpcs are defined by their own XML rulesets in ./vendor/squizlabs/php_codesniffer/CodeSniffer/Standards. The PSR-2 standard lives in PSR2/ruleset.xml, and it looks a bit like this:

<?xml version="1.0"?>
<ruleset name="PSR2">
 <description>The PSR-2 coding standard.</description>

 <!-- Include the whole PSR-1 standard -->
 <rule ref="PSR1"/>
</ruleset>

Turns out PSR-2 is an extension of the PSR-1 standard! PSR-1 lives in PSR1/ruleset.xml, and that's where we find the requirement for Pascal-case class names:

<rule ref="Squiz.Classes.ValidClassName"/>

In this case, the <rule /> pulls in a single rule instead of a whole standard. It tells phpcs to apply the rule defined in the Squiz standard (Squiz/Sniffs) under Classes at ValidClassNameSniff.php.

We don't want the ValidClassName rule in our custom standard, so we can exclude it with an <exclude /> tag:

<?xml version="1.0"?>
<ruleset name="The name of your custom standard">
  <description>An optional description for your standard</description>

  <!-- Include the whole PSR-2 standard: http://www.php-fig.org/psr/psr-2/ -->
  <rule ref="PSR2">

    <!-- Requires class names start with capital letters. Ours don't and we
         don't care. -->
    <exclude name="Squiz.Classes.ValidClassName" />

  </rule>

</ruleset>

Including an extra built-in rule

phpcs has a built-in rule for requiring a short or long array syntax in their Generic standard. I want to require the short syntax, so I need the rule at Generic/Sniffs/Arrays/DisallowLongArraySyntaxSniff.php. The convention for the XML rule reference is <standard>.<category>.<name minus 'Sniff'>, so I need to use Generic.Arrays.DisallowLongArraySyntax as my reference.

<?xml version="1.0"?>
<ruleset name="The name of your custom standard">
  <description>An optional description for your standard</description>

  <!-- Include the whole PSR-2 standard: http://www.php-fig.org/psr/psr-2/ -->
  <rule ref="PSR2">

    <!-- Requires class names start with capital letters. Ours don't and we
         don't care. -->
    <exclude name="Squiz.Classes.ValidClassName" />

  </rule>

  <!-- Require short array syntax -->
  <rule ref="Generic.Arrays.DisallowLongArraySyntax"/>

</ruleset>

Now that we've got a custom standard, we hand the XML file to phpcs on the command line:

$ ./vendor/bin/phpcs --standard=my-ruleset.xml ./src

If you'd like to see our complete, annoted ruleset, it's on Github.

Defining a Custom Rule

phpcs defines many rules, and it's likely that you'll be able to tune a custom standard exactly to your preferences without writing any extra code. (I found this phpcs standard generator helpful, both for locating interesting rules and for building the XML file.)

At WonderProxy, we had a minor edge case with multi-line conditional statements. PSR-2 requires conditionals with multiple lines to be indented, and to start with the boolean:

if ($aVeryLongConditional
    && $anotherLongConditional
    && $yetAnotherLongConditional
) {

That's all fine and dandy, but if you have nested conditionals, the indenting gets a little ambiguous:

if ($aVeryLongConditional
    && ($anotherLongConditional
    || $yetAnotherLongConditional)
) {

The WonderProxy team prefers syntax like this:

if ($aVeryLongConditional
    && ($anotherLongConditional
        || $yetAnotherLongConditional
    )
) {

There is no phpcs rule that defines the kind of behavior we wanted, so I wrote one!

Setting Up a Dedicated Standard

Up to this point, the WonderProxy projects that used our customized phpcs standard all included the copy/pasted ruleset file (containing our rule exclusions and inclusions). Writing a custom rule means writing some real PHP code, so we would have had to start copy/pasting both the ruleset file AND the code for the custom rule. Instead, I set up a dedicated WonderSniffer repository.

In the WonderSniffer project, I added phpcs as a Composer dependency (not a --dev dependency):

$ composer require squizlabs/php_codesniffer

I created a new standard at WonderNetwork/ and added our customized ruleset file at WonderNetwork/ruleset.xml. With the scaffolding in place, I modified our WonderProxy projects to require WonderSniffer, instead of phpcs itself...

$ composer remove --dev squizlabs/php_codesniffer
$ composer require --dev wondernetwork/wondersniffer

...and I trimmed their ruleset files down:

<?xml version="1.0"?>
<ruleset>
    <rule ref="vendor/wondernetwork/wondersniffer/WonderNetwork" />
</ruleset>

Now the projects are a composer update away from any changes to our phpcs standard. It's time to start coding!

Writing a Custom Rule

phpcs convention places individual rules in a Sniffs/<rule category>/<rule name>Sniff.php file within each standard. My custom rule deals with control structures, so I created Sniffs/ControlStructures/MultiLineConditionSniff.php in the WonderNetwork/ standard. (Rules defined in a standard's Sniffs/ directory are automatically included included in the standard, so I didn't need to add anything to the ruleset XML file to include my new rule.)

The details of the custom rule implementation are a little gnarly for a blog post, but the phpcs project maintains a solid tutorial. To start, I recommend finding the rule that most closely matches the behavior you want (in my case, that was Pear.ControlStructures.MultiLineCondition), and using it as a template. I also found the testing setup from MediaWiki and CakePHP useful, so check those out if you find yourself writing a custom rule.

Maintaining the Standard

Once we had a standard we liked, we wanted to make sure we kept using it. We could have created some Git commit hooks to run phpcs before the commit and throw an error if it failed, but requiring the whole team to set up hooks in all their repositories seemed unnecessarily burdensome (and easy to bypass). Instead, I added phpcs to our continuous integration pipeline:

#!/bin/bash

set -e # fail if any commands fail

./vendor/bin/phpcs --standard=ruleset.xml ./src

If phpcs fails, the build fails, and we get notified in our team chat. Breaking the build over a missing space between function and () is annoying enough that we're all incentivized to follow the standard and run phpcs ourselves. That kicks off a virtuous cycle of coding style consistency, which lets us focus on building things instead of arguing about curly braces!

Code Forth, to Victory!

It took us a few days to get phpcs set up the way we wanted it, but that up-front investment has already paid off in code quality improvements, increased readability, and a significant decline in twitching on my part. As WonderProxy embarks on new projects, the sound foundation we built with phpcs will help keep us honest, increasing future happiness and productivity!

If you'd like to try phpcs yourself and you have questions, send us a tweet or check out our WonderSniffer repo on Github.