Fix false negatives for Style/EvalWithLocation for Kernel.eval an…

…d when given improper arguments.

Previously if `eval` methods were given any arguments in the file and line spots, the cop would not register an offense. This change allows the cop to detect improper values for file (anything other than `__FILE__`) and line (as before, `__LINE__` with an offset as appropriate).

changelog/fix_fix_false_negatives_for.md

@@ -0,0 +1 @@
+* [#9411](https://github.com/rubocop-hq/rubocop/pull/9411): Fix false negatives for `Style/EvalWithLocation` for `Kernel.eval` and when given improper arguments. ([@dvandersluis][])

lib/rubocop/cop/style/eval_with_location.rb

@@ -39,25 +39,15 @@ module Style
       class EvalWithLocation < Base
         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`; ' \
+                             'use `%<expected>s` instead of `%<actual>s`.'
         MSG_INCORRECT_LINE = 'Incorrect line number for `%<method_name>s`; ' \
                              'use `%<expected>s` instead of `%<actual>s`.'
 
         RESTRICT_ON_SEND = %i[eval class_eval module_eval instance_eval].freeze
 
-        def_node_matcher :eval_without_location?, <<~PATTERN
-          {
-            (send nil? :eval ${str dstr})
-            (send nil? :eval ${str dstr} _)
-            (send nil? :eval ${str dstr} _ #special_file_keyword?)
-            (send nil? :eval ${str dstr} _ #special_file_keyword? _)
-
-            (send _ {:class_eval :module_eval :instance_eval}
-              ${str dstr})
-            (send _ {:class_eval :module_eval :instance_eval}
-              ${str dstr} #special_file_keyword?)
-            (send _ {:class_eval :module_eval :instance_eval}
-              ${str dstr} #special_file_keyword? _)
-          }
+        def_node_matcher :valid_eval_receiver?, <<~PATTERN
+          { nil? (const {nil? cbase} :Kernel) }
         PATTERN
 
         def_node_matcher :line_with_offset?, <<~PATTERN
@@ -68,18 +58,31 @@ class EvalWithLocation < Base
         PATTERN
 
         def on_send(node)
-          eval_without_location?(node) do |code|
-            if with_lineno?(node)
-              on_with_lineno(node, code)
-            else
-              msg = node.method?(:eval) ? MSG_EVAL : format(MSG, method_name: node.method_name)
-              add_offense(node, message: msg)
-            end
+          # Classes should not redefine eval, but in case one does, it shouldn't
+          # register an offense. Only `eval` without a receiver and `Kernel.eval`
+          # are considered.
+          return if node.method?(:eval) && !valid_eval_receiver?(node.receiver)
+
+          code = node.arguments.first
+          return unless code.str_type? || code.dstr_type?
+
+          file, line = file_and_line(node)
+
+          if line
+            check_file(node, file)
+            check_line(node, code)
+          else
+            register_offense(node)
           end
         end
 
         private
 
+        def register_offense(node)
+          msg = node.method?(:eval) ? MSG_EVAL : format(MSG, method_name: node.method_name)
+          add_offense(node, message: msg)
+        end
+
         def special_file_keyword?(node)
           node.str_type? &&
             node.source == '__FILE__'
@@ -90,6 +93,11 @@ def special_line_keyword?(node)
             node.source == '__LINE__'
         end
 
+        def file_and_line(node)
+          base = node.method?(:eval) ? 2 : 1
+          [node.arguments[base], node.arguments[base + 1]]
+        end
+
         # FIXME: It's a Style/ConditionalAssignment's false positive.
         # rubocop:disable Style/ConditionalAssignment
         def with_lineno?(node)
@@ -115,7 +123,18 @@ def message_incorrect_line(method_name, actual, sign, line_diff)
                  expected: expected)
         end
 
-        def on_with_lineno(node, code)
+        def check_file(node, file_node)
+          return true if special_file_keyword?(file_node)
+
+          message = format(MSG_INCORRECT_FILE,
+                           method_name: node.method_name,
+                           expected: '__FILE__',
+                           actual: file_node.source)
+
+          add_offense(file_node, message: message)
+        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

spec/rubocop/cop/style/eval_with_location_spec.rb

@@ -12,6 +12,30 @@
     RUBY
   end
 
+  it 'registers an offense when using `Kernel.eval` without any arguments' do
+    expect_offense(<<~RUBY)
+      Kernel.eval <<-CODE
+      ^^^^^^^^^^^^^^^^^^^ Pass a binding, `__FILE__` and `__LINE__` to `eval`.
+        do_something
+      CODE
+    RUBY
+  end
+
+  it 'registers an offense when using `::Kernel.eval` without any arguments' do
+    expect_offense(<<~RUBY)
+      ::Kernel.eval <<-CODE
+      ^^^^^^^^^^^^^^^^^^^^^ Pass a binding, `__FILE__` and `__LINE__` to `eval`.
+        do_something
+      CODE
+    RUBY
+  end
+
+  it 'does not register an offense if `eval` is called on another object' do
+    expect_no_offenses(<<~RUBY)
+      foo.eval "CODE"
+    RUBY
+  end
+
   it 'registers an offense when using `#eval` with `binding` only' do
     expect_offense(<<~RUBY)
       eval <<-CODE, binding
@@ -120,4 +144,53 @@
            __LINE__ - 1
     RUBY
   end
+
+  it 'registers an offense when using `eval` with improper arguments' do
+    expect_offense(<<~RUBY)
+      eval <<-CODE, binding, 'foo', 'bar'
+                                    ^^^^^ Incorrect line number for `eval`; use `__LINE__ + 1` instead of `'bar'`.
+                             ^^^^^ Incorrect file for `eval`; use `__FILE__` instead of `'foo'`.
+        do_something
+      CODE
+    RUBY
+  end
+
+  it 'registers an offense when using `instance_eval` with improper arguments' do
+    expect_offense(<<~RUBY)
+      instance_eval <<-CODE, 'foo', 'bar'
+                                    ^^^^^ Incorrect line number for `instance_eval`; use `__LINE__ + 1` instead of `'bar'`.
+                             ^^^^^ Incorrect file for `instance_eval`; use `__FILE__` instead of `'foo'`.
+        do_something
+      CODE
+    RUBY
+  end
+
+  it 'registers an offense when using `class_eval` with improper arguments' do
+    expect_offense(<<~RUBY)
+      class_eval <<-CODE, 'foo', 'bar'
+                                 ^^^^^ Incorrect line number for `class_eval`; use `__LINE__ + 1` instead of `'bar'`.
+                          ^^^^^ Incorrect file for `class_eval`; use `__FILE__` instead of `'foo'`.
+        do_something
+      CODE
+    RUBY
+  end
+
+  it 'registers an offense when using `module_eval` with improper arguments' do
+    expect_offense(<<~RUBY)
+      module_eval <<-CODE, 'foo', 'bar'
+                                  ^^^^^ Incorrect line number for `module_eval`; use `__LINE__ + 1` instead of `'bar'`.
+                           ^^^^^ Incorrect file for `module_eval`; use `__FILE__` instead of `'foo'`.
+        do_something
+      CODE
+    RUBY
+  end
+
+  it 'registers an offense when using correct file argument but incorrect line' do
+    expect_offense(<<~RUBY)
+      module_eval <<-CODE, __FILE__, 'bar'
+                                     ^^^^^ Incorrect line number for `module_eval`; use `__LINE__ + 1` instead of `'bar'`.
+        do_something
+      CODE
+    RUBY
+  end
 end