rubocop icon indicating copy to clipboard operation
rubocop copied to clipboard

Add `DisableCopForEntireFileInSourceCode` cop

Open sambostock opened this issue 3 years ago • 7 comments

Enforces that disabling cops for an entire file happens in a config file, rather than by wrapping the entire file in rubocop:disable and rubocop:enable comments.

# bad
# rubocop:disable Department/CopName
entire_file
# rubocop:enabled Department/CopName
# bad
# rubocop:todo Department/CopName
entire_file
# rubocop:enabled Department/CopName
# good
# Department/CopName disabled in config, such as .rubocop.yml or .rubocop_todo.yml
entire_file
# good
code
# rubocop:disable Department/CopName
subset_of_code
# rubocop:enabled Department/CopName
more_code

Before submitting the PR make sure the following are checked:

  • [x] The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • [x] Wrote good commit messages.
  • ~Commit message starts with [Fix #issue-number] (if the related issue exists).~
  • [x] Feature branch is up-to-date with master (if not - rebase it).
  • [x] Squashed related commits together.
  • [x] Added tests.
  • [x] Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • [x] Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

sambostock avatar Aug 03 '22 03:08 sambostock

I think Lint probably makes more sense here than Style, so that it stays with RedundantCopDisableDirective and RedundantCopEnableDirective.

dvandersluis avatar Aug 05 '22 18:08 dvandersluis

Thanks @sambostock! I think this is a good idea in general, but left some comments about things I'd like us to change her. Particularly, we should use the cop on RuboCop if we're going to add it :smile:

dvandersluis avatar Aug 05 '22 18:08 dvandersluis

Allow rubocop:disable comments, only if they target a subset of the file.

@sambostock Sorry for the radio silence for months! I do like the above suggestion, as the functionality of the new cop seems pretty related to the one of the existing one.

And I have to say that I love your suggestion in general - those directives were definitely meant for more granular use. Probably it worth to re-iterate this in the docs as well.

bbatsov avatar Nov 01 '22 07:11 bbatsov

Btw, now we can do the opposite as well (https://docs.rubocop.org/rubocop/1.38/configuration.html#temporarily-enabling-cops-in-source-code), so perhaps we should account for it.

bbatsov avatar Nov 01 '22 08:11 bbatsov

Oh yes, agreed, thanks for the heads up on that new feature. If we encourage developers to exclude a file from a rule in the config rather than disable comments around the entire file, then similarly we should encourage developers to enable a rule for a specific file from config, rather than wrap the entire file in enable comments.

sambostock avatar Nov 02 '22 21:11 sambostock

Yeah, exactly.

bbatsov avatar Nov 02 '22 21:11 bbatsov