Can't fix errors automatically.

[nixcms]

Hi! Firstly, we very appreciate the work you do! PHPCS is a very helpful tool!

We use PHPCS + WordPress coding standard in our project. We want to use new --report=diff param ( this param should be available in the PHPCS 2.0 ) in order to fix thousands of the errors reported in our CI testing automatically.

But we couldn't make it work for us. Please see out test - case and give us advice if we do somesing wrong. Thanks in advance.

Step-by-step test case :

1. Installing PHPCS

https://github.com/squizlabs/PHP_CodeSniffer

curl -OL https://github.com/squizlabs/PHP_CodeSniffer/releases/download/2.0.0RC1/phpcs.phar
curl -OL https://github.com/squizlabs/PHP_CodeSniffer/releases/download/2.0.0RC1/phpcbf.phar

1. Installing wordpress standard

https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards

git clone https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards.git wpcs
cd phpcs
scripts/phpcs --config-set installed_paths ../wpcs

1. Command line commands

We have error in the file functions.php, we see it with command parameter --report=summary

php phpcs.phar --standard=wpcs/WordPress functions.php

dev@kuf:~/www/pmt/wp-content/themes/pmtsystem$ php phpcs.phar --standard=wpcs/WordPress functions.php

FILE: /home/dev/www/pmt/wp-content/themes/pmtsystem/functions.php
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
--------------------------------------------------------------------------------
 63 | ERROR | Space after opening control structure is required
 63 | ERROR | No space before opening parenthesis is prohibited
--------------------------------------------------------------------------------

Time: 37ms; Memory: 4Mb

We tried to create a patch in order to apply it and fix errors automatically But functions.php.diff file is empty if we use a command below : php phpcs.phar --standard=wpcs/WordPress --report-diff=functions.php.diff functions.php

Also we've tried to fix errors automatically with phpcbf command instead of the phpcs command:

php phpcbf.phar --standard=wpcs/WordPress functions.php.diff

But it doesn't make sense because patch file is empty.

We tried to execute this command for different coding standards. It caused the same effect ( empty patch file ): php phpcs.phar --standard=PSR1 --report=summary functions.php

[gsherwood]

To fix errors, the WordPress coding standard would need to support this feature. It is the sniffs themselves that both report errors and fix errors. While PHPCS ships with a lot of sniffs that can fix the errors they find, it can't fix errors reported by custom sniffs inside custom standards. If a custom standard is using some of the included sniffs, then those errors can be fixed.

When PHPCS prints its report, it will tell you how many of the errors found can be fixed automatically. It will mark those that it can fix as well. If you dont see anything marked, it isn't able to fix any of them.

Try checking your code with an included standard like --standard=Squiz to see what it looks like. Unless you file is incredibly basic, the Squiz standard will complain about something.

[nixcms]

Yes now we see that phpcs works like a charm with Squiz, PSR2 etc...

php phpcs.phar --standard=Squiz --report-diff=functions.php.diff functions.php
patch -p0 -ui functions.php.diff

or

php phpcs.phar --standard=PSR2 --report-diff=functions.php.diff functions.php
patch -p0 -ui functions.php.diff

So we should investigate WordPress Coding Standard in order to have this feature. Thanx.

[fulldecent]

Ok, for putting this into practice. May we please ask for advice on one specific change to get the WordPress Coding Standard project on task.

Here is the PHPCS original which uses addFixableError https://github.com/squizlabs/PHP_CodeSniffer/blob/phpcs-fixer/CodeSniffer/Standards/Squiz/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php

And here is the modified version in WordPress-Coding-Standards which uses addError. https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/master/WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php

1. If we would like to have WordPress-Coding-Standards revert to the distribution phpcs-fixer/CodeSniffer/Standards/Squiz/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php rather than the our custom file, would we be able to do this in our ruleset? Is there documentation for this?
2. If we would like to make a minor change to the distribution sniff, for example $supportedTokenizers to only include 'PHP', then would you recommend we subclass Squiz_Sniffs_WhiteSpace_ControlStructureSpacingSniff as a best practice? If so is there any advice for this technique?

Thank you for your help and we are excited to get our first fixable error committed so that we have a starting point!

[gsherwood]

1. You'd remove the custom sniff from your repo and then just include the Squiz one in your ruleset.xml file like this:

<rule ref="Squiz.WhiteSpace.ControlStructureSpacing"/>

There are a lot of things you can do in the ruleset to customise your standard. You should take a look at this and see if any of it helps: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-ruleset.xml

1. You're best bet is to just exclude files from being checked with the sniff. Something like this instead:

<rule ref="Squiz.WhiteSpace.ControlStructureSpacing"/>
    <exclude-pattern>*.js$</exclude-pattern>
</rule>

Or you can subclass it if you want a bit more control.

[fulldecent]

@gsherwood

Ok, and for something like this: https://github.com/squizlabs/PHP_CodeSniffer/blob/58c0fdc412824222f33d15e418f826e6dcbff4e6/CodeSniffer/Standards/Squiz/Sniffs/Strings/DoubleQuoteUsageSniff.php#L85-L97

Here we have an additional rule "The use of variables in double quoted strings is not allowed."

Do you recommend that we maintain this separate sniff or should we be subclassing your sniff and adding this feature in the process() function?

[aik099]

@fulldecent is link correct? You're referring to the Squiz standard sniff there. What is your change then?

[aik099]

Sounds like you're talking about https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/master/WordPress/Sniffs/Strings/DoubleQuoteUsageSniff.php sniff. I see now, that your sniff version doesn't have extra check for variable usage in double quoted strings, that is now present in Squiz standard.

Here is what I propose:

1. use Squiz sniff version
2. remove own sniff version
3. disable ContainsVar warning message by setting it's severity to 0

<rule ref="Squiz.Strings.DoubleQuoteUsage.ContainsVar">
    <severity>0</severity>
</rule>

Here is how I did that: https://github.com/aik099/CodingStandard/blob/master/CodingStandard/ruleset.xml#L130-L132

[gsherwood]

Thanks for the reply @aik099. That's the right way to do it.

[fulldecent]

@aik099 Thank you, this looks great, sending to https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/pull/236