How useful is PHP CodeSniffer? Code Standards Enforcement in General?

Question

I'm dabbling with the idea of setting up PHP CodeSniffer on our continuous integration server in an effort to improve the quality of our code-base. After reading the documentation I'm very excited about the idea of normalizing and enforcing our coding standards. However, I'm left wondering about the actual improvement to our product. I'm well aware that the sniffer only detects violations to a defined coding-standard but what type of benefits does a clean, consistent, code-base provide? Is it worth the extra work to refactor a project with 100k+ lines of code to conform to the PEAR standard?

For those who are not familiar with PHP CodeSniffer or code-smell in general, here is an example output:

FILE: /path/to/code/myfile.php
FOUND 5 ERROR(S) AFFECTING 2 LINE(S)
--
2 | ERROR | Missing file doc comment
20 | ERROR | PHP keywords must be lowercase; expected "false" but found "FALSE"
47 | ERROR | Line not indented correctly; expected 4 spaces but found 1
51 | ERROR | Missing function doc comment
88 | ERROR | Line not indented correctly; expected 9 spaces but found 6

Strictly speaking, the user/client would not notice any difference in a product that was refactored to be standards-compliant but I'm wondering if there are other hidden benefits

Right now our code is by no means sloppy and we try to follow our own personal standards which, for the most part, are derived from Pear's Coding Standards but a trained eye can spot the differences.

So my question is how much do they improve the quality of a product. What kind of latent benefits resulted from it?

Am I just being obsessive-compulsive with my desire to move our product closer to a set of standards? Would it be worth it? If so, What kind of strategy did you use to implement the code-sniffer and correct the subsequent violations that were detected?

Answer 1

Having coding style conventions is a good idea, because it helps developers not get distracted by code written in a different style when working on code they did not write. It will make your code base superficially cleaner. It's great if you can automate it, but there is usually no need to go through great lengths to comply (unless the current style is terrible). If you already have a good-enough standard, stick to it.

Code smell is something different though: it is (a set of) symptoms that may indicate a deeper problem with the code. Examples are cyclomatic complexity, long method names, large classes, undescriptive names, duplicate code, etc. This is usually much more problematic, as it may thoroughly hurt the maintainability of your code. You should definitely solve these problems.

PHP CodeSniffer appears to be mainly developed for checking style conventions, not code smell. If you can use it to help enforce style conventions, great. But beware that it will not make your code base substantially better. You will want to do manual reviews to accomplish that.

If you want to use it to check if you comply to your current standard, that appears to be possible, see the answer to the question "I don't agree with your coding standards! Can I make PHP_CodeSniffer enforce my own?" in their FAQ.

Answer 2

Are you providing PEAR packages for public distribution thru PEAR/PECL? If so then you probably want to stick to PEAR conventions.

Otherwise, I can't see it being worth the big refactor. The biggest thing is agreeing upon a coding standard for your team... doesn't have to be PEAR's standard... just make sure there is some standard convention enforced.

For instance, I'm a fan of the

function foo () {

format vs the PEAR standard..

function foo ()
{

Bottom line, don't worry too much about conforming to their standard if its going to be a ton of work, especially if you aren't putting the packages on PECL.

Answer 3

There are lots of cases that require human judgement, and CodeSniffer doesn't have one.

Consistent brackets, indentation improve the code. Lack of space after commas in function call? Probably can be forgiven, but that's ERROR according to CodeSniffer.

IMHO there are way too many errors reported by CS. Even projects that appear to have neat code can easily run into thousands of CS issues. It quickly becomes tiring and nearly impossible to resolve all those issues, especially when it's a mix of real problems and obsesive-compulsive nonsense — both equally often marked as ERRORS.

You may be better off ignoring CS and spending time on actual improvements to the code (in terms of design, algorithms) rather than just completely superficial whitespace and comments changes (does 1-line isAlpha function really need 8 lines of comments? Yes, if you ask CS).

CS can too easily become turd-polishing tool.

Answer 4

This is definitely a good thing. We run ours from an SVN hook so that all code has to pass the house standard (a modification from PEAR) before it can be committed (this was one of the best decisions I ever made).

Of course, this works best for a new project, where there isn't loads of legacy code to convert to the new standard. One way around this is modify your SVN pre-commit hook to only run new additions to the codesniffer and to ignore modifications. You can do this by adding the line:

$SVNLOOK changed "$REPOS" -t "$TXN" | grep "^A.*\.php " > /dev/null || exit 0

This will exit the hook script if there isn't a new PHP code to parse. Hence all new files will need to obey the standard and you can bring the legacy code up to the standard in your own time.

Answer 5

Note that if you are using Eclipse or Zend IDE you can benefit from automatic tools that makes the respect of a standard less costly. You can also use a continuous integration tool like Hudson or PHPUndercontrol.

• PDT is a cool editor for PHP
• PDT-Tools are some plugins for PDT with an automatic formatting tool
• DTLK (Dynamic Toolkit Library) can be used to launch some external scripts to check your files.

You can also have a look at PHP Checkstyle which I think is easier to configure (disclaimer : I've worked on it)

Some other tools are listed on the "documentation" page of the web site.

Answer 6

CodeSniffer is a great thing to implement, but you have to know how to use it. Unless you have to comply with a given coding standard because you are submitting your work to some external project, you are free to completely define your own coding standards.

PHP CodeSniffer should make this very easy for you, because there are already many single code sniffs that you can include in your own coding standard definition and need not write them from scratch. While exploring the possibilities of the existing Codesniffers, you might end up writing an extension to an existing sniff or one sniff on your own, if you feel the need.

If you want to start with CodeSniffer, first step would be to grab a set of sniffs that completely resemble your current coding standards, and check the resulting errors and warnings. Don't apply one of the predefined standards, as this will most likely result in too many errors with too few benefit if fixed. For example, if you do not use PHPDoc to generate a documentation, it would be no use to fulfill all the codesniffer errors about missing PHPDoc tags and comments.