Php_codesniffer: Selectively disable rules with codingStandardsIgnoreStart and codingStandardsIgnoreLine

[westonruter]

PHPCS supports comment directives like:

// @codingStandardsIgnoreFile
// @codingStandardsIgnoreStart
// @codingStandardsIgnoreLine

However, these are all or nothing. What would be even more useful is if we can selectively ignore certain sniffs or errors within sniffs. For instance:

$q = new WP_Query( array(
    'posts_per_page' => 200, // @codingStandardsIgnoreLine WordPress.VIP.SlowDBQuery.slow_db_query 
) );

or

// @codingStandardsIgnoreStart WordPress.VIP.SlowDBQuery.slow_db_query
$q = new WP_Query( array(
    'posts_per_page' => 200,
) );
// @codingStandardsIgnoreEnd WordPress.VIP.SlowDBQuery.slow_db_query

This draws inspiration from JSCS and its directives like:

//jscs:disable requireCamelCaseOrUpperCaseIdentifiers

We have this in the WordPress-Coding-Standards project in a sniff-specific ad hoc system of end-of-statement comments:

echo $foo; // xss ok

Which suppresses the WordPress.XSS.EscapeOutput sniff. But it would be very useful if this was standardized in PHPCS itself and available to all sniffs

[JDGrimes]

:+1:

[GaryJones]

What format would the ignoring of multiple disabled rules take? Comma-separation?

[westonruter]

Comma-separated rules is what JSCS uses: http://jscs.info/overview.html#disabling-specific-rules

// jscs:disable requireCurlyBraces, requireDotNotation

[jjeaton]

:+1:

[thiemowmde]

:+1:

[DanielSiepmann]

:thumbsup:

[michaelhogg]

:+1:

[pdelre]

:+1:

[andrewhowdencom]

scss-lint disables lint on a given "scope". See: https://github.com/brigade/scss-lint#disabling-linters-via-source

// scss-lint:disable ExampleSniff

eslint appears to work in a similar way. See: http://eslint.org/docs/user-guide/configuring.html#disabling-rules-with-inline-comments

/* eslint-disable no-alert */

In both cases it's useful to disable select lints for a given file, particularly as you're introducing new requirements over time (such as making code style stricter).

[gsherwood]

Related PEAR request: http://pear.php.net/bugs/bug.php?id=18885

[Magnum5234]

👍

[kenguest]

👍

[mikaelz]

:+1:

[KacerCZ]

Slevomat Coding standard has implemented something similar in its sniffs.
https://github.com/slevomat/coding-standard#suppressing-sniffs-locally

[gsherwood]

The prep for this change has been pushed: https://github.com/squizlabs/PHP_CodeSniffer/commit/532517044940717548518f887b76d42cca7486d6

I've added a new shorter comment syntax and put a bit more core support in. The syntax for disabling PHPCS and ignoring lines is now phpcs:disable and phpcs:ignore.

The next step is to get these comments to allow a list of sniff codes after them so they can selectively disable or ignore some errors.

If that's working, a possible extension would be to allow the new phpcs:enable syntax to re-enable selected sniff codes only, and the new phpcs:ignoreFile syntax to set ignore rules for a whole file, but I'm not sure how useful those features are and if I'll have time to do them.

[glen-84]

@gsherwood Will we be able to put explanation text at the end of the line?

// phpcs:disable WordPress.VIP.SlowDBQuery.slow_db_query -- Because reason

[gsherwood]

Will we be able to put explanation text at the end of the line?

That's not something I had considered yet, but it's a good idea. It would need some sort of separator like you've put in your comment. Have you seen that syntax used elsewhere? Anyone else have any suggestions?

[JDGrimes]

Have you seen that syntax used elsewhere? Anyone else have any suggestions?

-- is one syntax for MySQL comments, and I think that it makes sense here. Can't think of anything better off of the top of my head.

[gsherwood]

Part 2 of this has been committed here: https://github.com/squizlabs/PHP_CodeSniffer/commit/1f07a69c00ef3b933f295f6c047e297b679677e7

The phpcs:disable and phpcs:ignore comments can now selectively ignore specific sniffs

• E.g., // phpcs:disable Generic.Commenting.Todo.Found for a specific message
• E.g., // phpcs:disable Generic.Commenting.Todo for a whole sniff
• E.g., // phpcs:disable Generic.Commenting for a whole category of sniffs
• E.g., // phpcs:disable Generic for a whole standard

Multiple sniff codes can be specified by comma seperating them

• E.g., // phpcs:disable Generic.Commenting.Todo,PSR1.Files

You can't add your own comments yet. But you can re-enable sniffs once you've disabled them. For example:
```php
// phpcs:disable PSR1,PSR2
echo 'PSR1 and 2 disabled here';
// phpcs:enable PSR2
echo 'PSR1 still disabled here';
// phpcs:enable
echo 'everything re-enabled here';

[gsherwood]

While thinking about adding selective support for phpcs:ignoreFile I realised this isn't needed. The phpcs:ignoreFile comment tells PHPCS to quickly ignore the entire file from all checking. If you want to selectively disable rules for an entire file, you would use phpcs:disable sniff,sniff instead.

So I think the last feature to add is the ability to add your own comments.

[glen-84]

It would need some sort of separator like you've put in your comment.

What if you matched one or more spaces followed by any non-word character?

Something like this: https://regex101.com/r/w1hwws/1

(could be improved a bit, but you get the idea)

Have you seen that syntax used elsewhere?

Not really, I just use it in my own projects/at work.

[gsherwood]

What if you matched one or more spaces followed by any non-word character?

That should be possible given the data that goes into those comments, but I'd prefer to decide on a specific syntax because it makes it easier to document and test.

[mansona]

Hey folks, I just wanted to weigh in here on the syntax of the comments. I would really recommend that you follow the syntax for eslint because it would allow for some good overlaps.

https://eslint.org/docs/user-guide/configuring#disabling-rules-with-inline-comments

as for the separators for "following comments", I would recommend something very simple. How about you just consider it as a list of comma separated rules which makes this valid

// eslint-disable-next-line no-console, this is more of a comment
console.log('face');

and becausethis is more of a comment doesn't match with any rule it just doesn't do anything.

Just an idea

[Frenzie]

@mansona
Wouldn't you expect it to spit out some kind of warning like "unrecognized rule" in that case?

-- is also how you add comments in Lua btw. I like the // phpcs:disable WordPress.VIP.SlowDBQuery.slow_db_query -- Because reason idea.

[gsherwood]

How about you just consider it as a list of comma separated rules which makes this valid

The new comment syntax does use a comma separated list of rules, but PHPCS doesn't know if they are valid or not because it doesn't only have a built-in list of sniffs. Plus, it allows for single words to mean something important (disable or enable an entire standard - see comment above https://github.com/squizlabs/PHP_CodeSniffer/issues/604#issuecomment-340614644) and it supports setting sniff config vars (phpcs:set sniff var value).

I'd like a comment syntax that supports all those formats without confusion.

I could just assume they are sniff codes and throw them into the main array. But that means having to put a comma before your comment, like this:

phpcs:disable sniffCode, because reasons

And it makes life really hard if you want to comment a "disable all" style comment, like this:

phpcs:disable Because reasons

(is Because reasons a sniff code, or should it have been phpcs:disable , Because reasons?)

I think a separator is the way to go, and I think -- is probably a good one. That's going to be an easy change, so I'll implement -- and can change it later if a better idea pops up.

[gsherwood]

I've added support for additional developer comments using the -- separator.

So you can now do this:

// phpcs:disable -- Turn off PHPCS
// phpcs:disable Generic.Commenting.Todo, PSR1.Files.SideEffects -- Because reasons

etc

You can also use block comments if you want something with more explanation. In this case, you don't need to use the comment separator because the explanation is on a different line:

/*
    We pasted this code in from a library doc and it needs to
    remain as is, so disable PHPCS for this block of code.
    phpcs:disable
*/

function Foo() {}

// phpcs:enable

[mansona]

@Frenzie I wouldn't expect an error in this case no, this process is about turning off errors so I don't think it would be necessary.

One thing I would like to add to the wider conversation here, I use these eslint style "overrides" every day, and have been for a few years now but never have I wanted to add "extra comments" to the one line

I don't see anything wrong with the following if you really need that:

// allow for console.log here so that we can tell the user something
// eslint-disable-next-line no-console
console.log('face');

I'd much rather a simple case like this ☝️ than over-complicate the syntax.

[gsherwood]

I don't see anything wrong with the following if you really need that:

// allow for console.log here so that we can tell the user something
// eslint-disable-next-line no-console
console.log('face');
I'd much rather a simple case like this ☝️ than over-complicate the syntax.

You can do that with the new PHPCS syntax as well. The benefit of putting it all on one line (if it is a short message) is that the comment line itself is completely ignored by PHPCS and doesn't need to confirm to your standard's comment rules. That's not something I personally care about, but I think other developers do.

[gsherwood]

I'll throw this idea into feedback for a while, but I think this feature is now done.

[o5]

I'm using sniff Generic.Files.LineLength. I would like to disable it for specific line. It works when I place this comment above of long line. But it does not work on same line:

[
    'foo' => 'very_long_string...' , // phpcs:disable Generic.Files.LineLength.TooLong
]

In case of $lineLimit is 30 warning occurs:

FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 2 | WARNING | Line exceeds 30 characters; contains 87 characters
   |         | (Generic.Files.LineLength.TooLong)

[gsherwood]

@o5 The // phpcs:disable syntax disable that error message from that comment onwards. If you want to ignore that line, you need to use // phpcs:ignore. Some docs here: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignoring-parts-of-a-file

Edit: I'd also suggest ignoring the whole sniff given that the really long comment is probably going to push the line length to MaxExceeded. So I'd suggest // phpcs:ignore Generic.File.LineLength

[o5]

@gsherwood Option $ignoreComments=true for Generic.Files.LineLength resolves my issue :)

[ahmadawais]

This issue shows up a lot in the search so I just wanted to share this as a result of this discussion.

The @codingStandards syntax is deprecated as of v3.2.0, use phpcs instead.

Some examples:

For Multi-line disable

// phpcs:disable
// phpcs:enable

For Single line ignore

// phpcs:ignore

See this page for more information: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignoring-parts-of-a-file

Peace! ✌️

[flobee]

Gooood feature!
One hint: Hints for the editors needed to search and select for eg: @phpcs :) like it works in phpunit apigen Codeception ... and others. Or does it exists and i dont know how?

I prefer using @ annotations which works like:

/**
 * ....
 * @phpcs:ignoreFile

[BenBroide]

For disabling and enabling a specific rule

```
// phpcs:disable WordPress.PHP.DevelopmentFunctions
error_log( $error_message );
// phpcs:enable WordPress.PHP.DevelopmentFunctions