Finding what Stinks, and Cleaning up the Mess

Contents

The Importance of Consistency

When writing any sort of code it is important that things be written properly; while what exactly is 'proper' is open for discussion, what is undeniable is that having a standard and convention on how you write your code will save you hours of stress and frustration later. This is most obvious when you try and track down the scattered instances where someone is using $username, and someone else is using $user_name, or if you're trying to remember if the function you need to call is resetPassword or reset_password. A code base that suffers from these kind of problems gives off a very particular odor that follows it where ever it goes.

Standards are useful in more ways than just naming conventions though: you can use standards to enforce spacing/indentation patterns, quickly inform you if you've got unused methods/variables, disallow bad coding practices, and many other things.

These consistent standards not only make your life easier, but makes your team's lives easier also, as when it comes time for them to look at your code (or for you to look at theirs) you have a common way of writing, which makes reading easier. As Joel Spolsky notes It's harder to read code than to write it. You want to make understanding the code base easier for those that follow after you - and for yourself, 6 months down the line when you have to go back and modify it. Code standards will help with this.

Implementing Standards

One of the problems with deciding on standards is that without enforcement they are quickly ignored and slowly the code will descend into to entropy and mess. If it is left to the individual developer to apply the standards there's a good likelihood of edge cases appearing where it's "not really necessary in this case". Additionally, no one wants to have to constantly fix other peoples code to match the standard. We can help prevent that through the use of scripts that apply predetermined rules to the code base to verify that they match the agreed upon standards, and throw obvious notices if they don't.

But even that's too much work - no one has the time to manually run a script whenever they want to verify their code. So, let's automate it!

Automating Standard Verification with Vim

When you are comfortable in Vim, it becomes a hammer and everything else a nail.

Getting The Necessaries

We're going to be taking advantage of a Vim plugin and some open source packages to handle all of our standards and validation checks. Lets install the packages we'll be needing - Mess Detector and Code Sniffer:

yum install php-pear-PHP-CodeSniffer php-phpmd-PHP-PMD perl-XML-XPath

This provides the two PHP packages that will be doing our checking, and installs an additional dependency that we'll be needing later down the line for the plugin. PHP Code Sniffer is used to enforce the actual standards for naming, indentation, and more. PHP Mess Detector works to detect suboptimal code, over complicated sections, unused portions of code, and other flaws.

Declare the Rules

With the applications installed it's time to build our rules. These rules will describe the standards that will be enforced by our checkers.

Code Sniffer

Code Sniffer comes with some default rules already, such as the Zend and PEAR rules. I found that I like to combine some portions of multiple rule sets for my own code. Luckily it's very easy to create your own standard. Here's how to go about it:

cd /usr/share/pear/PHP/CodeSniffer/Standards mkdir <StandardName> vim <StandardName>/ruleset.xml

This XML file will contain all the rules that you wish to apply to your standard. It'll look something like this:

<ruleset name="{StandardName}"> <description>This is My Ruleset</description> <!-- Include some sniffs from all around the place --> <rule ref="{RuleSet}.{SniffSubset}.{SniffName}"/> <rule ref="{RuleSet}.{SniffSubset}.{SniffName}"/> ... </ruleset>

Where {RuleSet} is the name of the Base rule set (eg. Generic, Zend, PEAR - contained in /usr/share/pear/PHP/CodeSniffer/Standards). {SniffSubset} represents one of the folders inside of the Sniffs folder of {RuleSet}. {SniffName} is the name of one of the PHP files inside the {SniffSubset} folder, but without the 'Sniff.php' at the end. So after all that, an example of a valid rule would be:

<rule ref="Generic.WhiteSpace.DisallowTabIndent"/>

You can see a listing of all the available rules by running the following command:

find /usr/share/pear/PHP/CodeSniffer/Standards -name "*Sniff.php" -and -not -name "Abstract*" | less

Additionally, some rules can recieve additional parameters. For example, if you're using the LineLength rule you can set when you want the rule to be triggered:

<!-- Lines can be 80 chars long, show errors at 120 chars --> <rule ref="Generic.Files.LineLength"> <properties> <property name="lineLimit" value="80"/> <property name="absoluteLineLimit" value="120"/> </properties> </rule>

That's all it takes to set up Code Sniffer. To run it issue the following command:

phpcs --standard={StandardName} <path-to-PHP-file>

You should see a nice little summary of any errors or warning detected in that file. Next up:

Mess Detector

Just like for Code Sniffer we need to have a rule set to apply to our code in order to detect any messes worth reporting. This file can be placed anywhere you like and looks something like this:

<?xml version="1.0"?> <ruleset name="{RuleSetName}" xmlns="http://pmd.sf.net/ruleset/1.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0 http://pmd.sf.net/ruleset_xml_schema.xsd" xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd"> <description>What this ruleset does</description> <rule ref="rulesets/{RuleSection}/{RuleName}" /> ... </ruleset>

You can see a listing of all the included rules at the Mess Detector Ruleset page. An example of a valid Mess Detector rule would look like this:

<rule ref="rulesets/design.xml/GotoStatement" />

Where 'design.xml' represents a rule section listed on the rules page, and the 'GotoStatement' is a specific rule included inside of the Design section (in this case, prohibiting usage of the GoTo statement). You can run the Mess Detector like this:

phpmd <PHP-source-file> <output:xml|text|html> <path-to-ruleset.xml>

That takes care of that - but we're still having to call these by hand, lets fix that next.

Attaching To Vim

I'm using Vundle to manage my Vim plugins, I highly recommend doing something of the sort yourself - it makes things incredibly simple. The plugin we'll be using is called phpqa. The documentation provided on the GitHub page will tell you everything you want to know, but here's the two settings that I use in my .vimrc file:

let g:phpqa_messdetector_ruleset = "<path-to-ruleset.xml>" let g:phpqa_codesniffer_args = "--standard=<StandardName>"

The next time you edit and save a PHP file you should get a quickfix window that will list any and all errors detected in the file. You can jump to the errors by hitting enter on the highlighted error/warning in the quickfix window.

Bonus

As an additional bonus, you'll discover that the syntax of the file is also checked, and should you have any syntax errors phpqa will throw a brilliant red line across the screen to mark its location. Quite helpful!

Closing Thoughts

The hardest part about picking standards is actually picking them. Everyone already has patterns in their head that they like to code to. Getting the whole team to agree to a set of standards may be difficult, but will pay out in the long run.

Something like Mess Detector is good at finding simple design flaws, but it is by no means a silver bullet. Don't expect it to solve all of your problems.

One solid rule you should always follow: if there is an existing convention in place in code that you're working on then continue to follow it! If you're trying to contribute to an Open Source project and you don't follow that projects conventions, your patches will never be accepted.

Have Something to Say?

Questions? Comments? Concerns? Let me know what you’re thinking.