Rubocop: Disable/Enable cops on blocks of code

[nixpulvis]

It would be nice if we could do the following.

# ...

# rubocop:disable ...
def foo
  # ...
end

# ...

This would only disable the cop for the block (method) foo. This removes the need to reenable the cop manually.

[jonas054]

Features like that can be found in both Perl::Critic and Pylint. If we decide to implement something along those lines, I think we should make it backwards compatible, i.e., chose a different word for disabling cops in the following scope.

The idea was raised in the original issue #34 but I decided to make a simple implementation to begin with.

[nixpulvis]

perhaps # rubocop:disable_block ...

[jonas054]

I'd prefer a different word than _block_, since block has a special meaning in Ruby, and it doesn't mean a method. The best I can come up with is # rubocop:disable_scope.

[bankair]

Hi guys,

At my workplace, we are working with ActiveRecord and MongoMapper in the same place, and rubocop is screaming at us for using MongoMapper validations like:

  validates_uniqueness_of :attribute_1, scope: :attribute_2

Which is pretty standard for a MongoMapper model, but triggers the rails validation cop.

Could be pretty handy to avoid being screamed at for using those regular mongo mapper construct by adding such tag.

Or maybe should I open an issue for stopping to apply rails validation cop to MongoMapper model ?

EDIT: Never mind the 'I would really love a disable flag' rant, I just discovered that it is already implemented. The part about mongo mapper is still true, thought

[bbatsov]

The part about mongo mapper is still true, thought

You can file another ticket for this.

[alexdowad]

Maintainers, do you think that disable_scope is generally useful enough to go ahead and add?

[jonas054]

No. Very little activity in this issue for a long time, so I guess the demand is pretty limited.

[HaleTom]

I'd like to throw in 2c worth of demand for this.
It seems very easy to forget to rubocop:enable after doing a disable for a temporary ignore
Also, if the code order is changed, there's another thing that needs to be checked for.

[alexdowad]

It seems very easy to forget to rubocop:enable after doing a disable for a temporary ignore

Do we flag a useless rubocop:enable comment?

[bbatsov]

Don't think so. If memory servers we flag only redundant rubocop:disable comments.

[HaleTom]

I have seen redundant disable comments being flagged. What I don't get is that second one here is not flagged though... and it actually CAUSES a warning???!!!

# make rubocop happy
class Passes
  # rubocop:disable Style/ClassVars
  @@class_var = 'no problems here'
end

# make rubocopy sad
class Offensive
  # Removing the next comment line makes for a happy rubocop - why?
  # rubocop:disable Style/ClassVars
  @@class_var = 'broken?'
end

[HaleTom]

Thanks @jonas054 for committing the fix for what I discovered already!

[printercu]

What about rubocop:disable_next to disable cops for the next line. It's especially helpful for method definitions, as of this disabling is not supported for long arguments list:

def test(arg, other, 
         rest_args) # rubocop:disable Cop

and the working one now is:

def test(arg, other, # rubocop:disable Cop
         rest_args)

which is not good, because it splits single instruction.

[jonas054]

which is not good, because it splits single instruction

Not a big problem if you ask me. I worry that rubocop:disable_next would add complexity both for users and contributors of RuboCop without improving functionality much.

[printercu]

I think the bigger problem occurres if someone misses rubocop:enable after disabling rubocop for a block of code. I've recently worked with java, it has such annotations/comments for disabling linters for next method/line. I think much of other tools have the same, 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.

[HaleTom]

Agree with printercu: rather than disabling for "file", disabling for "next" is by far the most commonly desired behaviour.

[dmolesUC]

I've been assuming all this time (well, up to the appearance of Lint/MissingCopEnableDirective) that rubocop:disable was scoped to the current block.

[abrthel]

My experience has been the same as dmolesUC3. I would much prefer the linter to only be disabled for a single line (or the current block) following a disable directive.

I'm unsure how difficult it would be to implement but what about a scenario where the linter is only disabled for the next line unless an enable directive follows it? That gives the best of both worlds and allows Lint/MissingCopEnableDirective to be removed,

[schwern]

I would find this useful for the Metric cops. I want to keep them enabled because we should be reminded to reduce the length and complexity of methods, but sometimes doing so is contrived.

I also was surprised to find it is not scoped to the following block. Lint/MissingCopEnableDirective certainly helps to avoid a very easy mistake.

OTOH maybe it should be inconvenient to disable cops. I've often rolled my eyes at what seems a nit-picking offense ready to disable it, only to think about it and find that ok yes, fixing it is an improvement.

[revolter]

So this actually works, right?

Meaning that, in OP's example, you have to change it to:

# ...

def foo # rubocop:disable ...
  # ...
end

# ...

I'm asking because it's not clear from the thread, and it actually looks like "disable/enable cops on methods" was not implemented.

[jonas054]

That's right, it's not been implemented. Disabling a certain line with an end-of-line comment works, and for some cops, e.g. Metrics/MethodLength, that will essentially disable the cop for the whole method. But the cop is not actually disabled in the scope of the method.

[revolter]

Oh, I thought it works with any cop. Pretty confusing 🤔