Merge pull request #55 from viralpraxis/detect-offenses-within-class-…

…eval-block

Detect offenses within `class_eval/exec` block

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## master (unreleased)
 
+* [#55](https://github.com/rubocop/rubocop-thread_safety/pull/55): Enhance `ThreadSafety::InstanceVariableInClassMethod` cop to detect offenses within `class_eval/exec` blocks. ([@viralpraxis](https://github.com/viralpraxis))
 * [#54](https://github.com/rubocop/rubocop-thread_safety/pull/54): Drop support for RuboCop older than 1.48. ([@viralpraxis](https://github.com/viralpraxis))
 * [#52](https://github.com/rubocop/rubocop-thread_safety/pull/52): Add new `RackMiddlewareInstanceVariable` cop to detect instance variables in Rack middleware. ([@viralpraxis](https://github.com/viralpraxis))
 * [#48](https://github.com/rubocop/rubocop-thread_safety/pull/48): Do not report instance variables in `ActionDispatch` callbacks in singleton methods. ([@viralpraxis](https://github.com/viralpraxis))

lib/rubocop/cop/thread_safety/instance_variable_in_class_method.rb

@@ -103,6 +103,7 @@ def class_method_definition?(node)
             in_def_sclass?(node) ||
             in_def_class_methods?(node) ||
             in_def_module_function?(node) ||
+            in_class_eval?(node) ||
             singleton_method_definition?(node)
         end
 
@@ -162,6 +163,17 @@ def in_def_module_function?(node)
             defn.right_siblings.any? { |sibling| module_function_for?(sibling, defn.method_name) }
         end
 
+        def in_class_eval?(node)
+          defn = node.ancestors.find do |ancestor|
+            break if ancestor.def_type? || new_lexical_scope?(ancestor)
+
+            ancestor.block_type?
+          end
+          return false unless defn
+
+          class_eval_scope?(defn)
+        end
+
         def singleton_method_definition?(node)
           node.ancestors.any? do |ancestor|
             break if new_lexical_scope?(ancestor)
@@ -237,6 +249,11 @@ def match_name?(arg_name, method_name)
             )
           }
         PATTERN
+
+        # @!method class_eval_scope?(node)
+        def_node_matcher :class_eval_scope?, <<~PATTERN
+          (block (send (const {nil? cbase} _) {:class_eval :class_exec}) ...)
+        PATTERN
       end
     end
   end

spec/rubocop/cop/thread_safety/instance_variable_in_class_method_spec.rb

@@ -290,6 +290,37 @@ def some_method(params)
     RUBY
   end
 
+  it 'registers an offense for instance variable within `class_eval` block' do # rubocop:disable RSpec/ExampleLength
+    expect_offense(<<~RUBY)
+      def separate_with(separator)
+        Example.class_eval do
+          @separator = separator
+          ^^^^^^^^^^ Avoid instance variables in class methods.
+        end
+      end
+    RUBY
+
+    expect_offense(<<~RUBY)
+      def separate_with(separator)
+        ::Example.class_eval do
+          @separator = separator
+          ^^^^^^^^^^ Avoid instance variables in class methods.
+        end
+      end
+    RUBY
+  end
+
+  it 'registers an offense for instance variable within `class_exec` block' do
+    expect_offense(<<~RUBY)
+      def separate_with(separator)
+        Example.class_exec do
+          @separator = separator
+          ^^^^^^^^^^ Avoid instance variables in class methods.
+        end
+      end
+    RUBY
+  end
+
   it 'registers no offense for ivar_set in define_method' do
     expect_no_offenses(<<~RUBY)
       class Test
@@ -386,6 +417,46 @@ def another_method(params)
     RUBY
   end
 
+  it 'does not register an offense for instance variable within `module_eval` block' do
+    expect_no_offenses(<<~RUBY)
+      def separate_with(separator)
+        Utilities.module_eval do
+          @separator = separator
+        end
+      end
+    RUBY
+  end
+
+  it 'does not register an offense for instance variable within `module_exec` block' do
+    expect_no_offenses(<<~RUBY)
+      def separate_with(separator)
+        Utilities.module_exec do
+          @separator = separator
+        end
+      end
+    RUBY
+  end
+
+  it 'does not register an offense for instance variable within `class_*` with new instance method' do
+    expect_no_offenses(<<~RUBY)
+      def separate_with(separator)
+        Example.class_eval do
+          def separator
+            @separator
+          end
+        end
+      end
+    RUBY
+  end
+
+  it 'does not register an offense for instance variable within `class_*` with string argument' do
+    expect_no_offenses(<<~RUBY)
+      def separate_with(separator)
+        Example.class_eval "@f = Kernel.exit"
+      end
+    RUBY
+  end
+
   context 'with `ActionDispatch` callbacks' do
     %i[
       prepend_around_action