How useful is PHP CodeSniffer? Enforcement of standards in general?

Question

I am trying to create PHP CodeSniffer on our continuous integration server to improve the quality of our code base. After reading the documentation, I am very excited about the idea of ​​normalizing and enforcing our coding standards. However, I am interested in the actual improvement of our product. I well know that the analyzer detects violations of only a certain coding standard, but what are the advantages of a clean, consistent code base? Is it worth the extra work of refactoring a project with lines of code 100k + to make it comply with the PEAR standard?

For those not familiar with PHP CodeSniffer or the smell of code in general, here is an example output:

FILE: /path/to/code/myfile.php
ERROR 5 (S) INFLUENCING ON 2 LINES (S)
-
2 | ERROR | Missing comment on document file
20 | ERROR | PHP keywords must be lowercase; expected a "false", but found a "FALSE"
47 | ERROR | The line has no indentation; 4 spaces expected, but 1 found
51 | ERROR | Missing comment documentation feature
88 | ERROR | The line has no indentation; 9 spaces expected but 6 found

Strictly speaking, the user / client would not notice any difference in the product that was reorganized to meet standards, but I wonder 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 are for the most part based on Pear coding standards, but the trained eye can notice the differences.

So my question is how much they improve the quality of the product. What are the hidden benefits it provides?

Am I just obsessively compulsive with my desire to bring our product closer to a set of standards? Is it worth it? If so, what strategy did you use to implement the code sniffer and fix the detected subsequent violations?

Answer 1

Firstly, I support PHP_CodeSniffer, so I am clearly prone to this area. But I also worked on some large codebases for 10 years as a PHP developer, so I hope I can provide some specific reasons why coding standards are good. I could write a blog series on this topic, but I’ll just tell you a little about how PHP_CodeSniffer came about, so you can understand what the tool solved for me.

I worked on several major CMS projects. The first had a bunch of code behind it and a relatively small development team. We did not have any standards. But we had no real problems. The team was small and stayed together for a long time. We are used to each other.

Then we built a new CMS. We started working with several developers. Then I was part of a team of two developers. Again, coding standards did not cause us any problems. I and other developers came from the same background and already set some recommendations that we were guided by. We did not need PHPCS then.

But this team grew as a developer at a time and eventually reached 12 full-time developers, and a lot came and went. Some came from the old CMS, and some came from outside the company. Everyone had different backgrounds and a different approach to development. It was obvious who wrote what code because the styles were so different. Whenever you worked on something complicated, you first need to adapt to their style, because it was just not the way you used to see the code. It’s like reading Shakespeare for the first time. You need to get used to it before you can read at your natural pace.

For developers, this is extra time to stop and figure out a different coding style - it's just wasted time. This is a chance for the idea to slip away while you are bogged down with a step, indent, and staple. In the end, all this does not matter. But let me tell you, they are very important if they force developers to disrupt their flow. So we needed a way to get them out of the way and let developers do what they do best.

At the same time, we delved a lot more into JavaScript. The new language in which the style was generally thrown out the window. The code was copied / pasted from sample sites and pressed together. Studying the development of complex code in a new language, it made sense to find a way to make our JS look like our PHP. We can minimize it later, but we had to quickly switch between languages ​​in order to maintain the flow.

So for this PHP_CodeSniffer was born. This helps developers work with the same coding style so that formatting and other flame problems are completely eliminated. This allows you to treat your JS as your PHP. I use it to detect product-specific odors, such as untranslated strings, or developers who don’t use our correct class inclusion code. I also use it for language-specific odors to make sure that the JS comma that kills IE is not left. You can use it the way you want. It comes with heaps of sniffs that are easily combined using an XML rule file . You can also write your own. You can integrate third-party tools to make it a one-stop shop for static code analysis. You can be as serious about code standards and flavors as you like.

PHP_CodeSniffer, like any dev tool, should work for you. You are not working for this. If it causes too many errors that you don’t need, adjust the standard to remove the ones you don’t need or turn errors into warnings. But if my story sounds like something you are experiencing or may pass in the future, it's worth taking a look at PHP_CodeSniffer to see if it can help you.

I hope this helps you and others understand why coding standards are really important for some projects and developers. This is not about the details. It's about removing the coding style from the list of things that cause developers to lose focus.

Answer 2

Aligning coding styles is a good idea because it helps developers not get distracted by code written in a different style when working on code that they don’t write. This will make your code base more superficial. This is great if you can automate it, but as a rule, there is no need to go a long length to fit (unless the current style is scary). If you already have a good enough standard, stick to it.

The smell of code is something else: it is a (set) of symptoms that may indicate a deeper problem with the code. Examples are cyclic complexity, long method names, large classes, non-descriptive names, duplicate code, etc. This is usually much more problematic, as it can seriously damage the maintainability of your code. You must definitely solve these problems.

PHP CodeSniffer seems to be mainly designed to check for style conventions, and not the smell of code. If you can use it to enforce style conventions, great. But be careful that this does not make your code base substantially better. You will want to do manual reviews to do this.

If you want to use it to check if you are up to your current standard, which seems possible, see the answer to the question "I do not agree with your coding standards! Can I make PHP_CodeSniffer my own?" in your FAQ .

Answer 3

Count me among those who evangelize CodeSniffer. After many years of skepticism, I now use it in all the projects I am working on. Why?

As Grace Hopper and / or Andrew Tanenbaum said,

The great thing about standards is that you have so many to choose from.

Similarly, it is almost always a bad idea and a trade; create your own coding standard; making one comprehensive enough to cover all of your code is difficult, and more importantly, will not love the next person to support your code, which will try to “improve” your standard to match its long-term, coding style. Adopting an appropriate external standard, whether it's Zend or PEAR or Kohana or JoeBobBriggsAndHisFifthCousin, allows you to focus on content rather than formatting. Even better, tools like PHP CodeSniffer either support the fresh from tin standard, or those who left earlier almost certainly implemented support as an add-on.

Standards for mixing coding with existing code not written to this standard will give you a fit if you do not accept two simple additional rules.

Exclude files that accept the encoding standard from being checked through the --ignore command line option or the equivalent of configuring the configuration file. But when you modify any part of the source file, update the entire file to meet your chosen standard.

I just wrote a new blog post about it.

Answer 4

There are many cases that require human judgment, but CodeSniffer does not.

Matching parentheses, indents improve code. Not enough space after commas in a function call? Probably forgiven, but it's ERROR according to CodeSniffer.

IMHO there are too many errors reported by CS. Even projects that appear to have clear code can easily launch thousands of CS issues. It quickly becomes tedious and almost impossible to solve all these problems, especially when it is a combination of real problems and obsessive-compulsive stupidity - both are often referred to as ERRORS.

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

Too easy to turn into a torsion polishing tool.

Answer 5

This is definitely good. We run our version with an SVN hook so that all the code needs to pass the standard at home (a modification from PEAR) before it can be perfect (this was one of the best decisions I have ever made).

Of course, this is best suited for a new project where there is no loading of legacy code to convert to a new standard. One way to do this is to change the SVN binding before commit, in order to launch new additions to the code identifier and ignore the changes. 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 no new PHP code exists for analysis. Therefore, all new files will have to comply with the standard, and you can bring outdated code into compliance with the standard in due time.

Answer 6

Please note that if you use the Eclipse or Zend IDE, you can use automated tools that make using the standard less expensive. You can also use a continuous integration tool such as Hudson or PHPUndercontrol.

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

You can also look at PHP Checkstyle , which, it seems to me, is easier to set up (disclaimer: I worked on it)

Some other tools are listed on the documentation page of the website.

Answer 7

CodeSniffer is a great thing to implement, but you need to know how to use it. If you do not have to follow this coding standard because you are submitting your work to some external project, you can fully define your own coding standards.

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

If you want to start with CodeSniffer, the first step is to capture a set of nuances that are completely reminiscent of your current encoding standards, and check for errors and warnings. Do not apply one of the predefined standards, as this will most likely lead to too many errors with too little gain if corrected. For example, if you are not using PHPDoc to generate documentation, it would be useless to make all code-related errors regarding missing tags and PHPDoc comments.

Answer 8

Do you provide PEAR packages for public distribution through PEAR / PECL? If so, you probably want to stick to PEARs.

Otherwise, I don’t see it worth a lot of refactoring. Most importantly, agreeing to a coding standard for your team ... does not have to be a PEAR standard ... just make sure there is a standard convention.

For example, I'm a fan

 function foo () { 

compared to the PEAR standard.

 function foo () { 

On the bottom line, don’t worry about conforming to their standard if it will be a ton of work, especially if you don’t put packages on PECL.