squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

Cant get @codingStandardsIgnoreStart and @codingStandardsIgnoreEnd to work #696

Closed internalsystemerror closed 9 years ago

internalsystemerror commented 9 years ago

PHP: 5.6.13 PHPCS: 2.3.4

So I'm unsure whether this is a known bug (can't find it in issues, closed or open), or i'm just using it wrong, not including a needed sniff etc. As it is, I'm unable to get the @codingStandardsIgnoreStart and @codingStandardsIgnoreEnd comments to work... Here are my tests:

<?php

// @codingStandardsIgnoreStart
defined('TEST') || define('TEST', getenv('TEST') ? : 'test');
// @codingStandardsIgnoreEnd

The command to run: phpcs --standard=PSR2 test.php

Expected: No output Actual Result: Warning - A file should declare new symbols (classes, functions, constants, etc.) and cause no other side effects, or it should execute logic with side effects, but should not do both. The first symbol is defined on line 4 and the first side effect is on line 4.

If however, I include @codingStandardsIgnoreFile, at the top, then the result is as expected.

gsherwood commented 9 years ago

Those tags only mute errors reported on the lines between the comments. They stop that bit of code being looked at by the sniffs.

The particular error you are trying to mute is reported on line 1, so you're pretty much out of luck unless you ignore the whole file, or add a sniff-specific ignore pattern into a custom ruleset.xml file for it.

Your ruleset.xml file would look something like this:

<?xml version="1.0"?>
<ruleset name="MyStandard">
  <description>My custom coding standard.</description>
  <rule ref="PSR2" />
  <rule ref="PSR1.Files.SideEffects">
     <exclude-pattern>test.php</exclude-pattern>
  </rule>
 </ruleset>

But you'd replace test.php in that exclude pattern to be a regex that will match whatever files you want to exclude. You can add as many exclude patterns as you'd like. More examples are on the wiki, along with other good stuff you can do in a custom ruleset: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-ruleset.xml

After you've got your ruleset.xml file saved somewhere, use it like this: phpcs --standard=/path/to/ruleset.xml /path/to/code

Does that help?

internalsystemerror commented 9 years ago

Thanks, that does actually help me. I'd rather ignore that specific error for this file, allowing other sniffs to continue to be checked.

The real file contains more code, however i realised this was the only line that caused any errors. If you notice in the error it says line 4, and not line 1. I'm not entirely sure how I've misused these comments, could you elaborate?

gsherwood commented 9 years ago

If you notice in the error it says line 4, and not line 1

The error message itself was generated by the specific sniff and is referencing line 4, but the error report will be showing the error on line 1, which is typically used to indicate that the file itself has a problem. Sniffs that do things like report incorrect line endings or file names use that same trick. There isn't any way to say "the whole file has an error, not a specific line" so the sniffs attach the error to line 1.

internalsystemerror commented 9 years ago

I think I'm understanding... So its just the case that these comment tags won't work for that specific type of error, because the line causes the entire file to fail?

gsherwood commented 9 years ago

So its just the case that these comment tags won't work for that specific type of error, because the line causes the entire file to fail?

Something like that. The comments are a very high-level mute switch and the individual sniffs don't actually see them.

What you're probably after is for the sniff to run normally but to pretend that line doesn't actually exist in the file because it will make the sniff throw an error. No feature like that exists in PHPCS and I can't see it being added as it would probably break quite a lot of things if the code just appears to not be there. Reporting, line numbers, indentation checks and auto fixing come to mind.

So you're left with excluding the sniff from checking that specific file for now.

internalsystemerror commented 9 years ago

Thanks, I'll do that, it may be worth adding this to the documentation however, as my original understand of these annotations is to prevent anything between them from throwing any errors at all.

I'll close this now, thanks for your help