Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rule suppresion for a line or scope #1211

Closed
wants to merge 3 commits into from
Closed

Add rule suppresion for a line or scope #1211

wants to merge 3 commits into from

Conversation

mrubiosan
Copy link

Hey there. How do you feel about adding the functionality to ignore some rules for a given line of code, or block? Right now, you can use the // @codingStandardsIgnoreStart notation, but it has the downside of ignoring all of them. With this feature you can specify exactly which violation you are aware of and should be ignored.

I'm not familiar with the tokenizing process and unaware of the myriad of possibilities, so most likely there's something wrong with how I'm traversing them.

So let this serve as a proof of concept at least. The way it works:

<?php
// @codingStandardsIgnoreRule(Generic.Commenting.Todo)
function foo()
{
    //TODO: blah
    echo 'foo!';
}

/**
 * You can also use multiline or doc comments, and the class name instead of the rule name
 * @codingStandarsIgnoreRule(Generic_Sniffs_Commenting_TodoSniff)
 */
function bar()
{
    //TODO: bleh
    echo 'bar!';
}

@gsherwood
Copy link
Member

There is an open feature request for this here: #604

So yes, I would consider adding it, and I probably will add it sometime in the future. But I have to first figure out what the syntax should be, and I think I'd like to change the way those ignore comments actually look as well. They are too long and not consistent with other tools that have since added their own syntax for it.

I'd also only put this into the 3.x branch - to be released as 3.0.0 soonish - because the way everything works is completely different and I wouldn't want to write this feature twice.

So unfortunately, I wont merge this PR into PHPCS due to the target version and the fact I want to change the syntax of this feature (a BC break). Thanks for the time you've taken to put it together though. It might be worth popping over to the issue I linked earlier and commenting there about the syntax.

@gsherwood gsherwood closed this Nov 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants