[Fix #9403] Add autocorrect for Style/EvalWithLocation

rubocop · Feb 12, 2021 · 2d3e1bc · 2d3e1bc
1 parent b2aa853
commit 2d3e1bc
Show file tree

Hide file tree

Showing 4 changed files with 166 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5440,3 +5440,4 @@
 [@AndreiEres]: https://github.com/AndreiEres
 [@jdufresne]: https://github.com/jdufresne
 [@adithyabsk]: https://github.com/adithyabsk
+[@cteece]: https://github.com/ceteece
diff --git a/changelog/new_add_autocorrect_for_styleevalwithlocation_cop.md b/changelog/new_add_autocorrect_for_styleevalwithlocation_cop.md
@@ -0,0 +1 @@
+* [#9403](https://github.com/rubocop-hq/rubocop/issues/9403): Add autocorrect for `Style/EvalWithLocation` cop. ([@cteece][])
diff --git a/lib/rubocop/cop/style/eval_with_location.rb b/lib/rubocop/cop/style/eval_with_location.rb
@@ -12,6 +12,11 @@ module Style
       # The cop also checks that the line number given relative to `__LINE__` is
       # correct.
       #
+      # This cop will autocorrect incorrect or missing filename and line number
+      # values. However, if `eval` is called without a binding argument, the cop
+      # will not attempt to automatically add a binding, or add filename and
+      # line values.
+      #
       # @example
       #   # bad
       #   eval <<-RUBY
@@ -37,6 +42,8 @@ module Style
       #     end
       #   RUBY
       class EvalWithLocation < Base
+        extend AutoCorrector
+
         MSG = 'Pass `__FILE__` and `__LINE__` to `%<method_name>s`.'
         MSG_EVAL = 'Pass a binding, `__FILE__` and `__LINE__` to `eval`.'
         MSG_INCORRECT_FILE = 'Incorrect file for `%<method_name>s`; ' \
@@ -66,21 +73,28 @@ def on_send(node)
           code = node.arguments.first
           return unless code && (code.str_type? || code.dstr_type?)
 
+          check_location(node, code)
+        end
+
+        private
+
+        def check_location(node, code)
           file, line = file_and_line(node)
 
           if line
             check_file(node, file)
             check_line(node, code)
+          elsif file
+            check_file(node, file)
+            add_offense_for_missing_line(node, code)
           else
-            register_offense(node)
+            add_offense_for_missing_location(node, code)
           end
         end
 
-        private
-
-        def register_offense(node)
+        def register_offense(node, &block)
           msg = node.method?(:eval) ? MSG_EVAL : format(MSG, method_name: node.method_name)
-          add_offense(node, message: msg)
+          add_offense(node, message: msg, &block)
         end
 
         def special_file_keyword?(node)
@@ -98,6 +112,10 @@ def file_and_line(node)
           [node.arguments[base], node.arguments[base + 1]]
         end
 
+        def with_binding?(node)
+          node.method?(:eval) ? node.arguments.size >= 2 : true
+        end
+
         # FIXME: It's a Style/ConditionalAssignment's false positive.
         # rubocop:disable Style/ConditionalAssignment
         def with_lineno?(node)
@@ -109,18 +127,16 @@ def with_lineno?(node)
         end
         # rubocop:enable Style/ConditionalAssignment
 
-        def message_incorrect_line(method_name, actual, sign, line_diff)
-          expected =
-            if line_diff.zero?
-              '__LINE__'
-            else
-              "__LINE__ #{sign} #{line_diff}"
-            end
+        def add_offense_for_incorrect_line(method_name, line_node, sign, line_diff)
+          expected = expected_line(sign, line_diff)
+          message = format(MSG_INCORRECT_LINE,
+                           method_name: method_name,
+                           actual: line_node.source,
+                           expected: expected)
 
-          format(MSG_INCORRECT_LINE,
-                 method_name: method_name,
-                 actual: actual.source,
-                 expected: expected)
+          add_offense(line_node.loc.expression, message: message) do |corrector|
+            corrector.replace(line_node, expected)
+          end
         end
 
         def check_file(node, file_node)
@@ -131,20 +147,25 @@ def check_file(node, file_node)
                            expected: '__FILE__',
                            actual: file_node.source)
 
-          add_offense(file_node, message: message)
+          add_offense(file_node, message: message) do |corrector|
+            corrector.replace(file_node, '__FILE__')
+          end
         end
 
         def check_line(node, code)
           line_node = node.arguments.last
-          lineno_range = line_node.loc.expression
-          line_diff = string_first_line(code) - lineno_range.first_line
+          line_diff = line_difference(line_node, code)
           if line_diff.zero?
             add_offense_for_same_line(node, line_node)
           else
             add_offense_for_different_line(node, line_node, line_diff)
           end
         end
 
+        def line_difference(line_node, code)
+          string_first_line(code) - line_node.loc.expression.first_line
+        end
+
         def string_first_line(str_node)
           if str_node.heredoc?
             str_node.loc.heredoc_body.first_line
@@ -156,20 +177,47 @@ def string_first_line(str_node)
         def add_offense_for_same_line(node, line_node)
           return if special_line_keyword?(line_node)
 
-          add_offense(
-            line_node.loc.expression,
-            message: message_incorrect_line(node.method_name, line_node, nil, 0)
-          )
+          add_offense_for_incorrect_line(node.method_name, line_node, nil, 0)
         end
 
         def add_offense_for_different_line(node, line_node, line_diff)
           sign = line_diff.positive? ? :+ : :-
           return if line_with_offset?(line_node, sign, line_diff.abs)
 
-          add_offense(
-            line_node.loc.expression,
-            message: message_incorrect_line(node.method_name, line_node, sign, line_diff.abs)
-          )
+          add_offense_for_incorrect_line(node.method_name, line_node, sign, line_diff.abs)
+        end
+
+        def expected_line(sign, line_diff)
+          if line_diff.zero?
+            '__LINE__'
+          else
+            "__LINE__ #{sign} #{line_diff.abs}"
+          end
+        end
+
+        def add_offense_for_missing_line(node, code)
+          register_offense(node) do |corrector|
+            line_str = missing_line(node, code)
+            corrector.insert_after(node.loc.expression.end, ", #{line_str}")
+          end
+        end
+
+        def add_offense_for_missing_location(node, code)
+          if node.method?(:eval) && !with_binding?(node)
+            register_offense(node)
+            return
+          end
+
+          register_offense(node) do |corrector|
+            line_str = missing_line(node, code)
+            corrector.insert_after(node.loc.expression.end, ", __FILE__, #{line_str}")
+          end
+        end
+
+        def missing_line(node, code)
+          line_diff = line_difference(node.arguments.last, code)
+          sign = line_diff.positive? ? :+ : :-
+          expected_line(sign, line_diff)
         end
       end
     end

diff --git a/spec/rubocop/cop/style/eval_with_location_spec.rb b/spec/rubocop/cop/style/eval_with_location_spec.rb
@@ -8,6 +8,8 @@
         do_something
       CODE
     RUBY
+
+    expect_no_corrections
   end
 
   it 'registers an offense when using `Kernel.eval` without any arguments' do
@@ -17,6 +19,8 @@
         do_something
       CODE
     RUBY
+
+    expect_no_corrections
   end
 
   it 'registers an offense when using `::Kernel.eval` without any arguments' do
@@ -26,6 +30,8 @@
         do_something
       CODE
     RUBY
+
+    expect_no_corrections
   end
 
   it 'does not register an offense if `eval` is called on another object' do
@@ -41,6 +47,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      eval <<-CODE, binding, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'registers an offense when using `#eval` without lineno' do
@@ -50,13 +62,23 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      eval <<-CODE, binding, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'registers an offense when using `#eval` with an incorrect line number' do
     expect_offense(<<~RUBY)
       eval 'do_something', binding, __FILE__, __LINE__ + 1
                                               ^^^^^^^^^^^^ Incorrect line number for `eval`; use `__LINE__` instead of `__LINE__ + 1`.
     RUBY
+
+    expect_correction(<<~RUBY)
+      eval 'do_something', binding, __FILE__, __LINE__
+    RUBY
   end
 
   it 'registers an offense when using `#eval` with a heredoc and ' \
@@ -67,6 +89,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      eval <<-CODE, binding, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'registers an offense when using `#eval` with a string on a new line ' do
@@ -77,6 +105,13 @@
            __LINE__)
            ^^^^^^^^ Incorrect line number for `eval`; use `__LINE__ - 3` instead of `__LINE__`.
     RUBY
+
+    expect_correction(<<~RUBY)
+      eval('puts 42',
+           binding,
+           __FILE__,
+           __LINE__ - 3)
+    RUBY
   end
 
   it 'registers an offense when using `#class_eval` without any arguments' do
@@ -86,6 +121,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      C.class_eval <<-CODE, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'registers an offense when using `#module_eval` without any arguments' do
@@ -95,6 +136,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      M.module_eval <<-CODE, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'registers an offense when using `#instance_eval` without any arguments' do
@@ -104,6 +151,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      foo.instance_eval <<-CODE, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'registers an offense when using `#class_eval` with an incorrect lineno' do
@@ -113,6 +166,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      C.class_eval <<-CODE, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'accepts `eval` with a heredoc, a filename and `__LINE__ + 1`' do
@@ -151,6 +210,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      eval <<-CODE, binding, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'registers an offense when using `instance_eval` with improper arguments' do
@@ -161,6 +226,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      instance_eval <<-CODE, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'registers an offense when using `class_eval` with improper arguments' do
@@ -171,6 +242,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      class_eval <<-CODE, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'registers an offense when using `module_eval` with improper arguments' do
@@ -181,6 +258,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      module_eval <<-CODE, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'registers an offense when using correct file argument but incorrect line' do
@@ -190,6 +273,12 @@
         do_something
       CODE
     RUBY
+
+    expect_correction(<<~RUBY)
+      module_eval <<-CODE, __FILE__, __LINE__ + 1
+        do_something
+      CODE
+    RUBY
   end
 
   it 'does not register an offense when using eval with block argument' do