Re: [PEAR] Re: PHP_CodeSniffer: Patchlevel updates break our Coding Standards

On Thu, Feb 28, 2013 at 10:29 AM, Michael Gauthier <[email protected]>wrote:

> To keep things technical, can you elaborate more by what you mean about
> cherry-picking specific sniffs?
>
> What's the advantage of providing packaged standards if you should be
> specifying each sniff within the standard you want to use? Are they
> primarily organizational?
>

The important thing to note is that the collection of Generic sniffs *is
not a standard*. It says this in the ruleset.xml file and it does not
appear in the list of installed PHPCS standards (phpcs -i). It is just a
collection of sniffs that people may find useful.

If you force PHPCS to run the Generic standard (--standard=Generic) over
your code, you will *always* get errors because the sniffs instead conflict
with each other. I personally make use of this ability via testing, but I
dont know of anybody who has ever tried to use it as an actual standard.
You'd figure out pretty quickly that you can never please it. Some sniffs
say use spaces for indent and some say do not. Some say use brace style x
and some say use brace style y. You cherry pick the bits you want and
create your own custom standard. This is how many developers already do
things, so I think the concept is generally well understood. PHPCS also
ships with concrete examples of this via the actual coding standards that
are available.

Here is an example:
https://raw.github.com/squizlabs/PHP_CodeSniffer/master/CodeSniffer/Standards/PEAR/ruleset.xml
Here is another:
https://raw.github.com/squizlabs/PHP_CodeSniffer/master/CodeSniffer/Standards/PSR1/ruleset.xml

There are more if you want to look. Almost all included standards include
generic sniffs. There are also plenty of other standard around for PHP
projects that do exactly the same thing.



> Should the <exclude> option be removed or deprecated to prevent problems
> like the one Christian encountered? The online documentation contains an
> example much like Christian's without any warning of it's negative
> consequences:
>
> http://pear.php.net/manual/en/**package.php.php-codesniffer.**
> annotated-ruleset.php<http://pear.php.net/manual/en/package.php.php-codesniffer.annotated-ruleset.php>



No, it should not be removed. I think you've just missed the point of it.

Take the PSR2 standard for example. Some people seem to really really hate
the fact that it requires developers to use spaces instead of tabs. But it
is a published standard and that is what it says.

So do this:
<?xml version="1.0"?>
<ruleset name="MyStandard">
 <description>PSR2 without the requirement to use spaces.</description>
 <rule ref="PSR2">
  <exclude name="Generic.WhiteSpace.DisallowTabIndent"/>
 </rule>
</ruleset>

You know the PSR2 standard is a real standard and you are simply modifying
the bits that you don't like. You know the standard will not change just
because I personally decide to change it. It will change it in-line with
the documented standard. Changes to a documented standard require me to
change PHPCS, but not to increase a major version number.

But you also know that if it does change (perhaps to add some additional
checks) you will still have the complete PSR2 standard minus the space
indent bit.

Christian's standard says: include everything in the Generic folder but
exclude these few things I don't like. That would be fine if the Generic
folder was actually a documented coding standard (like the PEAR folder, for
example). But it isn't; it is a dumping ground, hence the problem.

I hope that makes it clear.

Greg