Lint/UnneededDisable reports a warning if rubocop:disable all is used inside a block with already disabled cop

[ojab]

If # rubocop:disable all is used inside of code block with some cops already disabled, Lint/UselessAssignment reports a warning. For example for code:

# rubocop:disable Style/ParallelAssignment
# rubocop:disable all
x, y = 1, 2

Actual behavior

$ rubocop /tmp/1.rb
Inspecting 1 file
W

Offenses:

/tmp/1.rb:1:1: W: Lint/UnneededDisable: Unnecessary disabling of Style/ParallelAssignment.
# rubocop:disable Style/ParallelAssignment
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/tmp/1.rb:2:1: W: Lint/UnneededDisable: Unnecessary disabling of Style/ParallelAssignment.
# rubocop:disable all
^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected

Expected behavior

No warnings reported, # rubocop:disable all is used to disable all remaining cops, not all initially enabled cops.

RuboCop version

$ rubocop -V
0.43.0 (using Parser 2.3.1.4, running on ruby 2.3.1 x86_64-darwin16)

[the-bass]

It's not the Lint/UselessAssignment cop that reports the warning but Lint/UnneededDisable, which, according to the comments in the code, was just build for this very case. Excerpt:

# This cop detects instances of rubocop:disable comments that can be
# removed without causing any offenses to be reported.

So I guess this is the expected behavior. You can disable the cop by simply adding

Lint/UnneededDisable:
  Enabled: false

to your .rubocop.yml.

[bbatsov]

Damn, I didn't really read the offense output. :-) Not a bug indeed.

[ojab]

Ouch, yeah, subject mentioned a wrong cop.

I still think that there is an issue: right now there is no way to disable all cops in situations like:

class Whatever
  # So so code
  # rubocop:disable Cop1,Cop2,Cop3
  def method_one
  end

  def method_two
  end

  # Code is total garbage
  # rubocop:disable all
  def method_three
  end
end

which is quite useful during cleanups (i. e. we do not want to introduce new offenses in the first part and don't care about second part because no one has touched it yet).

AFAIU right now we can:
a) Disable Lint/UnneededDisable via magic comment
b) Enable all disabled cops before rubocop:disable all
c) List all cops one by one instead of all
Less than elegant, I'd say.

As I've said in the first comment, I expect # rubocop:disable all to disable all remaining cops, not all initially enabled cops.

And, even if my reading of # rubocop:disable all expected outcome is wrong, Lint/UnneededDisable should not emit two warnings: one of the magic comments is in fact necessary to disable Cop.

[jonas054]

Absolutely. Lint/UnneededDisable should only report Unnecessary disabling of all cops for rubocop:disable all when it's not needed. Saying Unnecessary disabling of Style/ParallelAssignment for that line makes no sense. Reopening.

[the-bass]

The cop Lint/UnneededDisable ...

...detects instances of rubocop:disable comments that can be removed without causing any offenses to be reported.

(see https://github.com/bbatsov/rubocop/blob/ec3123fc3454b080e1100e35480c6466d1240fff/lib/rubocop/cop/lint/unneeded_disable.rb#L6)

This is clearly the case in a file with

# rubocop:disable Style/ParallelAssignment
# rubocop:disable all
x, y = 1, 2

So

Expected behavior: No warnings reported

should not be the expected behavior. If this cop wouldn't detect an offense here, I'd call it a bug.