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

Possibility to list unnecessarily disabled cops #1865

Closed
andreaseger opened this issue May 5, 2015 · 3 comments
Closed

Possibility to list unnecessarily disabled cops #1865

andreaseger opened this issue May 5, 2015 · 3 comments

Comments

@andreaseger
Copy link
Contributor

We use in several places # rubocop:disable to whitelist certain areas of source code. The problem I see is that sometimes these comments won't get removed even though the issue was already fixed. In these situations I'd like to get an indication from rubocop that the disable call is useless and should be removed.

So for example you had a long method which could at the time not be split into several junks. It raises an issue on Metrics/MethodLength but you can't do anything about it (right now), temporary solution is to add # rubocop:disable Metrics/MethodLength.
On some later day the method can finally be refactored or some part can be removed which brings it below the limit for MethodLength. Unfortunately the developer didn't take the extra time to count the lines and realize that so he just left the disable-comment in place.
It would be great if rubocop could either show these instances as an issue them-self, indicating to the developer that he can remove the disable-comment. Or if this would need some more tracking a new command-line option could generate these issues.

(Hope I made it clear enough what I meant)

@jonas054
Copy link
Collaborator

jonas054 commented May 5, 2015

I see exactly what you mean. Good suggestion.

The implementation is a bit difficult because the natural place to put this kind of functionality is in a new formatter, or possibly in DisabledLinesFormatter, and the way we call the formatters today doesn't make disabled offenses available to them.

The formatters have a public API that we should keep backwards compatible.

Maybe there are other ways to achieve the functionality, but they are not obvious to me.

jonas054 added a commit to jonas054/rubocop that referenced this issue May 8, 2015
Keep all offenses around, including locally disabled ones,
until reporting time.

Let the formatters do the filtering by adding a new method
wanted_offenses in the public API of BaseFormatter. The
default behavior is to not keep the comment-disabled
offenses, which makes the change backwards compatible.

Likewise, the public API of Offense is amended with a
disabled? method, but the corrected/corrected? methods
stay compatible.

DisabledLinesFormatter outputs a red "(unnecessary)"
after lines with a disabled cop and a line range that
didn't have any offenses for that cop.
jonas054 added a commit to jonas054/rubocop that referenced this issue May 8, 2015
Keep all offenses around, including locally disabled ones,
until reporting time.

Let the formatters do the filtering by adding a new method
wanted_offenses in the public API of BaseFormatter. The
default behavior is to not keep the comment-disabled
offenses, which makes the change backwards compatible.

Likewise, the public API of Offense is amended with a
disabled? method, but the corrected/corrected? methods
stay compatible.

DisabledLinesFormatter outputs a red "(unnecessary)"
after lines with a disabled cop and a line range that
didn't have any offenses for that cop.
@ivan-leschinsky
Copy link

I think, that pull-request brokes detection # rubocop:disable comments when running without command-line interface. For example in the pronto-rubocop gem, issue: prontolabs/pronto-rubocop#13

@mmozuras
Copy link
Contributor

mmozuras commented Sep 6, 2015

@ivan-leschinsky it did, but that's what I get for using APIs that are not strictly official 😄

@bbatsov would you accept a PR that would create an official non-command-line API?

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

No branches or pull requests

4 participants