rubocop / rubocop

A Ruby static code analyzer and formatter, based on the community Ruby style guide.
https://docs.rubocop.org
MIT License
12.61k stars 3.05k forks source link

Allow unsafe, auto-correctable cops to be marked as 'todo' with --disable-uncorrectable #7958

Open laurmurclar opened 4 years ago

laurmurclar commented 4 years ago

Is your feature request related to a problem? Please describe.

I'm trying to add # rubocop:todo SomeCop comments to all existing violations in our codebase. I've been using rubocop --safe-auto-correct --disable-uncorrectable to automatically generate these comments.

However, when a cop is unsafe and auto-correctable (eg. Lint/BooleanSymbol), Rubocop just highlights the offense instead of adding a comment.

$ bundle exec 'rubocop --safe-auto-correct --disable-uncorrectable --only Lint/BooleanSymbol example.rb'
Inspecting 1 file
W

Offenses:

example.rb:6:3: W: Lint/BooleanSymbol: Symbol with a boolean name - you probably meant to use true.
  :true
  ^^^^^

1 file inspected, 1 offense detected

If I use --auto-correct instead of --safe-auto-correct, the offense is auto-corrected (which is undesired in my case, since the fix is "unsafe").

A cop which is both unsafe and not auto-correctable (eg. Lint/DisjunctiveAssignmentInConstructor can be marked as todo by using --auto-correct --disable-uncorrectable

$ bundle exec 'rubocop --auto-correct --disable-uncorrectable --only Lint/DisjunctiveAssignmentInConstructor example.rb'
Inspecting 1 file
W

Offenses:

example.rb:2:6: W: [Todo] Lint/DisjunctiveAssignmentInConstructor: Unnecessary disjunctive assignment. Use plain assignment.
  @x ||= 1
     ^^^

1 file inspected, 1 offense detected, 1 offense corrected

Add AutoCorrect: false to the Lint/BooleanSymbol config in .rubocop.yml prevents the auto-correction, but a todo comment is still not added.

Describe the solution you'd like

I would like to be able to run --disable-uncorrectable to add todo comments to all offenses for unsafe cops without auto-correcting them. To me, it makes sense to include that behaviour in --safe-auto-correct --disable-uncorrectable, but I can see why it might make sense to have a separate flag.

Describe alternatives you've considered

The alternative is manually adding comments to offending code.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

laurmurclar commented 3 years ago

This still seems useful to me - especially for organizations with large, or older codebases

thijsnado commented 2 years ago

I'd even love if this could be enabled for "safe" auto correctable. I've been bitten by rubocop bugs where something wasn't as safe as it claimed to be. I've seen rubocop introduce syntax errors with the auto correcting behavior. It would be great to have a clean slate by just marking existing issues as TODO regardless of their safety.

dvandersluis commented 2 years ago

@thijsnado if you have examples of RuboCop making unsafe corrections that are marked safe today, can you please open an issue for them so we can fix them? It's never going to be perfect but we try to make sure RuboCop isn't doing something unexpected.

ilvez commented 2 years ago

I also would like to see this feature. Currently we have implemented our own script to do this stuff, but it's really naive and creates # rubocop:todo comments globally for files. This is not really good solution in the long run, since devs can ignore those global comments easily and new rule offenses can be added easily.

If possible then it would be good to choose if to create those comments for all offenses or safely auto-correctable ones, but I probably could go with only latter as well.

Just for reference, this is our hacked together solution at the moment:


rubocop --format j app spec | jq -rc '.files[] | { path: .path, rule: .offenses[].cop_name } | "sed -i \"1i# rubocop:todo \(.rule)\" \(.path)"' | uniq | sh
thijsnado commented 3 weeks ago

@dvandersluis, sorry for the late response. I've found the rubocop/rubocop codebase to be generally very reliable but as you get in the more obscure third party rubocop libraries that have less eyes on them the chance of a bug slipping through in the auto corrector increases. Unfortunately doing something like

TheCopIWantToMarkAsTodo:
  AutoCorrect: false

will disable the ability to auto correct but doesn't add in the todo comments. This can be very irritating with a large, risky codebase where we want to start using the new rule for new code but don't want to risk breaking stable legacy code.

zverok commented 3 weeks ago

One more vote for an ability “just disable this cop, not try to run the autocorrection” when working with a large codebase (I actually came up with a ticket of my own about such a functionality, #13205, somehow I completely missed --disable-uncorrectable existance)