Merge pull request #2548 from jonas054/2546_fix_bug_in_extra_disable

[Fix #2546] Handle double rubocop:disable of a cop

CHANGELOG.md

@@ -95,6 +95,7 @@
 * [#2066](https://github.com/bbatsov/rubocop/issues/2066): `Style/TrivialAccessors` doesn't flag what appear to be trivial accessor method definitions, if they are nested inside a call to `instance_eval`. ([@alexdowad][])
 * `Style/SymbolArray` doesn't flag arrays of symbols if a symbol contains a space character. ([@alexdowad][])
 * `Style/SymbolArray` can auto-correct offenses. ([@alexdowad][])
+* [#2546](https://github.com/bbatsov/rubocop/issues/2546): Report when two `rubocop:disable` comments (not the single line kind) for a given cop apppear in a file with no `rubocop:enable` in between. ([@jonas054][])
 
 ### Changes
 

lib/rubocop/comment_config.rb

@@ -35,6 +35,12 @@ def analyze
         if single_line
           disabled_line_ranges[cop_name] << (line..line) if disabled
         elsif disabled
+          if disablement_start_line_numbers[cop_name]
+            # Cop already disabled on this line, so we end the current disabled
+            # range before we start a new range.
+            start_line = disablement_start_line_numbers.delete(cop_name)
+            disabled_line_ranges[cop_name] << (start_line..line)
+          end
           disablement_start_line_numbers[cop_name] = line
         else
           start_line = disablement_start_line_numbers.delete(cop_name)

lib/rubocop/cop/lint/unneeded_disable.rb

@@ -22,6 +22,16 @@ def check(offenses, cop_disabled_line_ranges, comments)
           disabled_ranges = cop_disabled_line_ranges[COP_NAME] || [0..0]
 
           cop_disabled_line_ranges.each do |cop, line_ranges|
+            line_ranges.each_cons(2) do |previous_range, range|
+              next if previous_range.end != range.begin
+
+              # If a cop is disabled in a range that begins on the same line as
+              # the end of the previous range, it means that the cop was
+              # already disabled by an earlier comment. So it's unneeded
+              # whether there are offenses or not.
+              comment = comments.find { |c| c.loc.line == range.begin }
+              unneeded_cops[comment].add(cop)
+            end
             cop_offenses = offenses.select { |o| o.cop_name == cop }
             line_ranges.each do |line_range|
               comment = comments.find { |c| c.loc.line == line_range.begin }

spec/rubocop/comment_config_spec.rb

@@ -12,33 +12,43 @@
         '',
         '# rubocop:disable Metrics/MethodLength',
         'def some_method',
-        "  puts 'foo'",
+        "  puts 'foo'",                                      # 5
         'end',
         '# rubocop:enable Metrics/MethodLength',
         '',
         '# rubocop:disable all',
-        'some_method',
+        'some_method',                                       # 10
         '# rubocop:enable all',
         '',
         "code = 'This is evil.'",
         'eval(code) # rubocop:disable Lint/Eval',
-        "puts 'This is not evil.'",
+        "puts 'This is not evil.'",                          # 15
         '',
         'def some_method',
         "  puts 'Disabling indented single line' # rubocop:disable " \
         'Metrics/LineLength',
         'end',
-        '',
+        '',                                                  # 20
         'string = <<END',
         'This is a string not a real comment # rubocop:disable Style/Loop',
         'END',
         '',
-        'foo # rubocop:disable Style/MethodCallParentheses',
+        'foo # rubocop:disable Style/MethodCallParentheses', # 25
         '',
         '# rubocop:enable Lint/Void',
         '',
         '# rubocop:disable Style/For',
-        'foo'
+        'foo',                                               # 30
+        '',
+        'class One',
+        '  # rubocop:disable Style/ClassVars',
+        '  @@class_var = 1',
+        'end',                                               # 35
+        '',
+        'class Two',
+        '  # rubocop:disable Style/ClassVars',
+        '  @@class_var = 2',
+        'end'                                                # 40
       ]
     end
 
@@ -106,5 +116,10 @@ def disabled_lines_of_cop(cop)
       alias_disabled_lines = disabled_lines_of_cop('Alias')
       expect(alias_disabled_lines).not_to include(25)
     end
+
+    it 'can handle double disable of one cop' do
+      expect(disabled_lines_of_cop('Style/ClassVars'))
+        .to eq([9, 10, 11] + (33..40).to_a)
+    end
   end
 end

spec/rubocop/cop/lint/unneeded_disable_spec.rb

@@ -217,6 +217,51 @@
         end
       end
 
+      context 'and there are two offenses' do
+        let(:message) do
+          'Replace class var @@class_var with a class instance var.'
+        end
+        let(:cop_name) { 'Style/ClassVars' }
+        let(:offenses) do
+          [
+            RuboCop::Cop::Offense.new(:convention,
+                                      OpenStruct.new(line: 3, column: 3),
+                                      message,
+                                      cop_name),
+            RuboCop::Cop::Offense.new(:convention,
+                                      OpenStruct.new(line: 8, column: 3),
+                                      message,
+                                      cop_name)
+          ]
+        end
+
+        context 'and a comment disables' do
+          context 'one cop twice' do
+            let(:source) do
+              ['class One',
+               '  # rubocop:disable Style/ClassVars',
+               '  @@class_var = 1',
+               'end',
+               '',
+               'class Two',
+               '  # rubocop:disable Style/ClassVars',
+               '  @@class_var = 2',
+               'end'].join("\n")
+            end
+            let(:cop_disabled_line_ranges) do
+              { 'Style/ClassVars' => [2..7, 7..9] }
+            end
+
+            it 'returns an offense' do
+              expect(cop.messages)
+                .to eq(['Unnecessary disabling of `Style/ClassVars`.'])
+              expect(cop.highlights)
+                .to eq(['# rubocop:disable Style/ClassVars'])
+            end
+          end
+        end
+      end
+
       context 'and there is an offense' do
         let(:offenses) do
           [