diff --git a/CHANGELOG.md b/CHANGELOG.md index f1a6f4a..a8ece4c 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/lib/rubocop/cop/thread_safety/instance_variable_in_class_method.rb b/lib/rubocop/cop/thread_safety/instance_variable_in_class_method.rb index 1969806..10de928 100644 --- a/lib/rubocop/cop/thread_safety/instance_variable_in_class_method.rb +++ b/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 diff --git a/spec/rubocop/cop/thread_safety/instance_variable_in_class_method_spec.rb b/spec/rubocop/cop/thread_safety/instance_variable_in_class_method_spec.rb index 0fa1da8..5eb322b 100644 --- a/spec/rubocop/cop/thread_safety/instance_variable_in_class_method_spec.rb +++ b/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