Rubocop: More specific disable directives

[dorner]

I've found myself constantly annoyed by how Rubocop forces you to wrap disable directives in a block, both manually disabling and enabling it. It would be amazing if instead you could have something like:

#rubocop:disable:next_line MyCop
#rubocop:disable:next_method MyCop
#rubocop:disable:next_block MyCop

The next_line is the one I feel would be most useful as that's where the surrounding comments look particularly ugly.

Thoughts?

[mikegee]

You can disable a cop for individual lines by putting the disable comment at the end of the line.

for x in (0..19) # rubocop:disable Style/For

You can disable a cop for a method or block by putting the disable comment at the end of the first line.

def long_method # rubocop:disable Metrics/MethodLength
  ...

[dorner]

The first case is kind of messy because it will almost always cause the line to fail the line length cop. And in general it's not as readable as having the disable directive above the line.

The second case only works for cops that apply on the method level, no? Like it wouldn't ensure that e.g. checking for do..end instead of braces are disabled for the whole method?

[Drenmi]

The first case is kind of messy because it will almost always cause the line to fail the line length cop.

The LineLength cop can be configured to not count directives towards line length using the IgnoreCopDirectives option.

The second case only works for cops that apply on the method level, no?

Yes. It only applies to that node. It wouldn't check all nested nodes. I.e. it is not the "next block" directive you suggested.

[printercu]

I've found this issue now and will copy my comment (slightly modified) from https://github.com/bbatsov/rubocop/issues/1690, because this one is more appropriate:

With rubocop:disable ... rubocop:enable problems may occur if someone misses rubocop:enable. Lots of other similar tools have anotations to disable lints for next line, because it's convenient and safe.

I even think that this should be harder to disable linter for the rest of file as it's not the default behaviour most people want (or should want) to avoid unchecked issues. And rubocop:disable should disable linter only for the next line by default.

WDYT?

[jonas054]

And rubocop:disable should disable linter only for the next line by default.

I'd prefer not to change the default, as it would break builds for people and require a lot of work for them. I'm OK with the proposal at the top of this issue. I think # rubocop:disable:next_line MyCop would be fairly easy to implement. # rubocop:disable:next_method MyCop and # rubocop:disable:next_block MyCop would be a lot harder.

[gaffneyc]

I would love to see at least disabling for the next line added to Rubocop. It's almost always what I want. We avoid using inlined comments as I find postfix comments and conditionals easy to miss.

I would also like to propose shortening the names as I think the next is implied.

#rubocop:disable:line MyCop
#rubocop:disable:method MyCop
#rubocop:disable:block MyCop

This also opens up this directive for disabling for the current file only.

#rubocop:disable:file MyCop

[printercu]

This also opens up this directive for disabling for the current file only

I think this is current behaviour, isn't it?

[gaffneyc]

This also opens up this directive for disabling for the current file only

I think this is current behaviour, isn't it?

I'm not actually sure. I would assume so but I also assumed the the disable comment only affected the following line.

[jonas054]

Disabling the rest of the file is the default, but nowadays you get an offense from Lint/MissingCopEnableDirective if you don't have a rubocop:enable comment somewhere below the rubocop:disable.

[bbatsov]

There hasn't been much activity on this ticket and our Core Team is spread too thin on so many tasks, so I'll just close it for the sake of having a cleaner lists of tasks to focus on. We'd gladly review a PR, but it's unlikely that'd we're going to tackle this ourselves in the foreseeable future.