Move to PHP-CS-Fixer

[Slamdunk]

• Map one-to-one ruleset.xml to Config.php (I'll do this after the topic discussion)
• Update README.md

Hi, following #9 (comment) here I propose to drop PHP_CodeSniffer in favour of PHP-CS-Fixer with the following reasons:

1. No XML, just plain PHP
2. Each Fixer class is self-describing, containing:
   1. all documentation
   2. all configurations documentation
   3. code samples for all configurations
3. Tests are plain PHPUnit test cases: implementing new/custom fixers or adding new test cases is straightforward
4. With a simple test like ConfigTest.php we can keep up to date with newer fixers and manage deprecations easier
5. Contains precious fixers not present in PHP_CodeSniffer like
   1. @PHPUnit60Migration:risky
   2. @DoctrineAnnotation
6. It is build upon Symfony components, so has more robust console interactions
7. Strict PHP Version checking and configuration

[Slamdunk]

@Majkl578 I saw your 👎 and I would like to hear your reasoning.

What it really bugs me about PHP_CodeSniffer is how hard it is to understand what a sniffers does.
I want to help FOSS projects, I want to make them better, but if the learning curve of their tools is steep, I give up.

Let's take an example: in the ruleset.xml of this project we have this rule:

<!-- Require EOL at EOF -->
<rule ref="Generic.Files.EndFileNewline"/>

As a (wannabe) contributor I want to understand what this sniffer does.

1. Comments are a smell: what needs to be commented, needs to be refactored
2. Google isn't of any help (link)
3. Grepping/Finding into the source of PHP_CodeSniffer I find a lot of stuffs with non-trivial code and test cases to understand
4. Until I run it I have no idea if I can automatically fix it or not
5. There is no documentation about what eventual PHP version the fixer is addressed to
6. There is no before-and-after example
7. There is no readable documentation: I am still unable to find where the Wiki or the binary produce what is in the Docs/*xml

And all this for such a simple sniffer.

On the opposite, if you look for what single_blank_line_at_eof PHP-CS-Fixer rule does:

1. You can take a look at the global README.md (Google first result is its READE.md link)
2. You can type php-cs-fixer describe single_blank_line_at_eof that outputs the documentation and the code samples
3. The tests are raw PhpUnit input->expected (see SingleBlankLineAtEofFixerTest.php)

[Ocramius]

IMO also 👎 on moving to different tooling (and moving back again after a few months, ping-pong style).

The D2 folks are familiar with PHPCS, which is quite OK and well maintained anyway.

Also, the Slevomat coding standard is more than 50% of the reasons to stay with PHPCS.

[Ocramius]

PHPCS is a good tool, not a perfect one, but we already did all the work here, so let's settle it this way for now, and we can decide to re-open the topic if it pops up again in a few months.

[alcaeus]

Agree with @Ocramius on this. The reasons outlined don't convince me to use php-cs-fixer. As for helping contributors, if a pull request fails with CS errors, we're happy to help them if the error can't be fixed automatically.

[Slamdunk]

IMO also 👎 on moving to different tooling (and moving back again after a few months, ping-pong style).

As mentioned in #9 (comment) I'm ok with this

The D2 folks are familiar with PHPCS, which is quite OK and well maintained anyway.

All the Symfony users are familiar to PHP-CS-Fixer, which is a large user base too.

PHPCS is a good tool, not a perfect one, but we already did all the work here, so let's settle it this way for now, and we can decide to re-open the topic if it pops up again in a few months.

Ok

The reasons outlined don't convince me to use php-cs-fixer.

I don't get money for PHP-CS-Fixer, and I agree that changing the tools is expensive.
But it will be hard to convince me that, for a new project, PHP_CodeSniffer would be easier/faster to setup and maintain.

As for helping contributors, if a pull request fails with CS errors, we're happy to help them if the error can't be fixed automatically.

As long as you like that activity, good for you.

[Majkl578]

@Majkl578 I saw your 👎 and I would like to hear your reasoning.

I was on a phone. 😉

No XML, just plain PHP

No added value in having it in PHP, you are still using plain strings (unlike i.e. classnames, there it'd be useful).

Each Fixer class is self-describing

Some of the PHPCS classes as well, some not. But this isn't really issue on our end, documentation should be discussed on PHPCS repo (squizlabs/PHP_CodeSniffer#1752)..

Tests are plain PHPUnit test cases

So can easily be PHPCS tests, see https://github.com/slevomat/coding-standard/tree/master/tests/Sniffs.

As a (wannabe) contributor

A wannabe contributor won't read through the ruleset.xml, but read i.e. CONTRIBUTING.md, or nothing. Now significant sniffs are also described in README in this repo. Most of the other generic sniffs are simply checks for bad habits / typos etc., like Generic.PHP.DeprecatedFunction. Additionally each sniff has pretty verbose error reporting that would be shown during CI inspection for a PR.

Until I run it I have no idea if I can automatically fix it or not

That's fine, there is no need to know such information before you actually need it. And when you do, you're indicated which error is automatically fixable and which not.

I have no strong opinion about PHP-CS-Fixer, I just have no experience with it. Though I admit, recent PHP-CS-Fixer spam/ad in PRs across GH did slightly irritate me (do they giveaway free hats for 5 PR or so?).
The decision in #9/#11 to add new PHPCS sniffs was simply pragmatic: it was already set up, working and used across some repos - no need to switch to different tool unless there is significant gain. Plus there are PHPCS sniffs from Slevomat and I'm not quite there are equvivalents in PHP-CS-Fixed