-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Comments
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 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. |
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.
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.
I think, that pull-request brokes detection |
@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? |
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)
The text was updated successfully, but these errors were encountered: