Change logic for when to use EOL disable comment

The decision whether to use a single EOL comment to disable a cop, or a
disable/enable pair on separate lines, should not be based on whether the
code range of the offense is single line or multi-line. It should be based
on whether the comment fits on the given line.

The single line comment WILL disable the offense even if it's reported on a
multi-line range.

We also need to make sure we don't add more than one EOL comment per line
(fix later).

lib/rubocop/cop/autocorrect_logic.rb

@@ -39,17 +39,28 @@ def autocorrect_enabled?
 
       def disable_offense(node)
         range = node.location.expression
-        range_by_lines = range_by_lines(range)
-
-        if range.line == range.last_line
-          disable_offense_at_end_of_line(range_by_lines)
+        eol_comment = " # rubocop:disable #{cop_name}"
+        needed_line_length = range.column +
+                             (range.source_line + eol_comment).length
+        if needed_line_length <= max_line_length
+          disable_offense_at_end_of_line(range_of_first_line(range),
+                                         eol_comment)
         else
-          disable_offense_before_and_after(range_by_lines)
+          disable_offense_before_and_after(range_by_lines(range))
         end
       end
 
       private
 
+      def range_of_first_line(range)
+        begin_of_first_line = range.begin_pos - range.column
+        end_of_first_line = begin_of_first_line + range.source_line.length
+
+        Parser::Source::Range.new(range.source_buffer,
+                                  begin_of_first_line,
+                                  end_of_first_line)
+      end
+
       # Expand the given range to include all of any lines it covers. Does not
       # include newline at end of the last line.
       def range_by_lines(range)
@@ -64,13 +75,12 @@ def range_by_lines(range)
                                   end_of_last_line)
       end
 
-      def disable_offense_at_end_of_line(range_by_lines)
-        lambda do |corrector|
-          corrector.insert_after(
-            range_by_lines,
-            " # rubocop:disable #{cop_name}"
-          )
-        end
+      def max_line_length
+        config.for_cop('Metrics/LineLength')['Max'] || 80
+      end
+
+      def disable_offense_at_end_of_line(range, eol_comment)
+        ->(corrector) { corrector.insert_after(range, eol_comment) }
       end
 
       def disable_offense_before_and_after(range_by_lines)

lib/rubocop/cop/cop.rb

@@ -159,9 +159,9 @@ def correct(node)
           correction = autocorrect(node)
           return :uncorrected unless correction
 
-          @corrections << correction
+          @corrections << Correction.new(correction, node, self)
         elsif disable_uncorrectable?
-          @corrections << Correction.new(disable_offense(node), node, self)
+          disable_uncorrectable(node)
         end
         :corrected
       end
@@ -174,6 +174,15 @@ def reason_to_not_correct(node)
         nil
       end
 
+      def disable_uncorrectable(node)
+        @disabled_lines ||= {}
+        line = node.location.line
+        return if @disabled_lines.key?(line)
+
+        @disabled_lines[line] = true
+        @corrections << Correction.new(disable_offense(node), node, self)
+      end
+
       def config_to_allow_offenses
         Formatter::DisabledConfigFormatter
           .config_to_allow_offenses[cop_name] ||= {}

spec/rubocop/cli/cli_disable_uncorrectable_spec.rb

@@ -28,62 +28,91 @@
       RUBY
     end
 
-    it 'adds one-line disable statement for one-line offenses' do
-      create_file('example.rb', <<-RUBY.strip_indent)
-        def is_example
-          true
-        end
-      RUBY
-      expect(exit_code).to eq(0)
-      expect($stderr.string).to eq('')
-      expect($stdout.string).to eq(<<-OUTPUT.strip_indent)
-        == example.rb ==
-        C:  1:  1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
-        C:  1:  5: [Corrected] Naming/PredicateName: Rename is_example to example?.
-        
-        1 file inspected, 2 offenses detected, 2 offenses corrected
-      OUTPUT
-      expect(IO.read('example.rb')).to eq(<<-RUBY.strip_indent)
-        # rubocop:disable Naming/PredicateName
-        # frozen_string_literal: true
+    context 'if a one-line disable statement fits' do
+      it 'adds it' do
+        create_file('example.rb', <<-RUBY.strip_indent)
+          def is_example
+            true
+          end
+        RUBY
+        expect(exit_code).to eq(0)
+        expect($stderr.string).to eq('')
+        expect($stdout.string).to eq(<<-OUTPUT.strip_indent)
+          == example.rb ==
+          C:  1:  1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
+          C:  1:  5: [Corrected] Naming/PredicateName: Rename is_example to example?.
+
+          1 file inspected, 2 offenses detected, 2 offenses corrected
+        OUTPUT
+        expect(IO.read('example.rb')).to eq(<<-RUBY.strip_indent)
+          # frozen_string_literal: true
+
+          def is_example # rubocop:disable Naming/PredicateName
+            true
+          end
+        RUBY
+      end
+
+      context 'and there are two offenses of the same kind on one line' do
+        it 'adds a single one-line disable statement' do
+          create_file('.rubocop.yml', <<-YAML.strip_indent)
+            Style/IpAddresses:
+              Enabled: true
+          YAML
+          create_file('example.rb', <<-RUBY.strip_indent)
+            ip('1.2.3.4', '5.6.7.8')
+          RUBY
+          expect(exit_code).to eq(0)
+          expect($stderr.string).to eq('')
+          expect($stdout.string).to eq(<<-OUTPUT.strip_indent)
+            == example.rb ==
+            C:  1:  1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
+            C:  1:  4: [Corrected] Style/IpAddresses: Do not hardcode IP addresses.
+            C:  1: 15: [Corrected] Style/IpAddresses: Do not hardcode IP addresses.
+
+            1 file inspected, 3 offenses detected, 3 offenses corrected
+          OUTPUT
+          expect(IO.read('example.rb')).to eq(<<-RUBY.strip_indent)
+            # frozen_string_literal: true
 
-        def is_example
-          true
+            ip('1.2.3.4', '5.6.7.8') # rubocop:disable Style/IpAddresses
+          RUBY
         end
-        # rubocop:enable Naming/PredicateName
-      RUBY
+      end
     end
 
-    it 'adds before-and-after disable statement for multiline offenses' do
-      create_file('.rubocop.yml', <<-YAML.strip_indent)
-        Metrics/MethodLength:
-          Max: 1
-      YAML
-      create_file('example.rb', <<-RUBY.strip_indent)
-        def example
-          puts 'line 1'
-          puts 'line 2'
-        end
-      RUBY
-      expect(exit_code).to eq(0)
-      expect($stderr.string).to eq('')
-      expect($stdout.string).to eq(<<-OUTPUT.strip_indent)
-        == example.rb ==
-        C:  1:  1: [Corrected] Metrics/MethodLength: Method has too many lines. [2/1]
-        C:  1:  1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
-        
-        1 file inspected, 2 offenses detected, 2 offenses corrected
-      OUTPUT
-      expect(IO.read('example.rb')).to eq(<<-RUBY.strip_indent)
-        # rubocop:disable Metrics/MethodLength
-        # frozen_string_literal: true
+    context "if a one-line disable statement doesn't fit" do
+      it 'adds before-and-after disable statement' do
+        create_file('.rubocop.yml', <<-YAML.strip_indent)
+          Metrics/MethodLength:
+            Max: 1
+        YAML
+        create_file('example.rb', <<-RUBY.strip_indent)
+          def long_method_name(_taking, _a_few, _parameters, _resulting_in_a_long_line)
+            puts 'line 1'
+            puts 'line 2'
+          end
+        RUBY
+        expect(exit_code).to eq(0)
+        expect($stderr.string).to eq('')
+        expect($stdout.string).to eq(<<-OUTPUT.strip_indent)
+          == example.rb ==
+          C:  1:  1: [Corrected] Metrics/MethodLength: Method has too many lines. [2/1]
+          C:  1:  1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
 
-        def example
-          puts 'line 1'
-          puts 'line 2'
-        end
-        # rubocop:enable Metrics/MethodLength
-      RUBY
+          1 file inspected, 2 offenses detected, 2 offenses corrected
+        OUTPUT
+        expect(IO.read('example.rb')).to eq(<<-RUBY.strip_indent)
+          # rubocop:disable Metrics/MethodLength
+          # frozen_string_literal: true
+
+          def long_method_name(_taking, _a_few, _parameters, _resulting_in_a_long_line)
+            puts 'line 1'
+            puts 'line 2'
+          end
+          # rubocop:enable Metrics/MethodLength
+        RUBY
+      end
     end
   end
 end