From 3c7acbad997def48cb52c0c3df8f2ce3d4771a3a Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Fri, 11 Oct 2024 18:22:35 +0300 Subject: [PATCH 1/2] Increase coverage --- .../thread_safety/class_instance_variable.rb | 1 - .../mutable_class_instance_variable.rb | 2 +- .../class_and_module_attributes_spec.rb | 8 ++++++++ .../class_instance_variable_spec.rb | 14 ++++++++++++++ .../mutable_class_instance_variable_spec.rb | 18 ++++++++++++++++++ 5 files changed, 41 insertions(+), 2 deletions(-) diff --git a/lib/rubocop/cop/thread_safety/class_instance_variable.rb b/lib/rubocop/cop/thread_safety/class_instance_variable.rb index 4128d2b..2b7811c 100644 --- a/lib/rubocop/cop/thread_safety/class_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/class_instance_variable.rb @@ -133,7 +133,6 @@ def in_def_class_methods_dsl?(node) node.ancestors.any? do |ancestor| break if new_lexical_scope?(ancestor) next unless ancestor.block_type? - next unless ancestor.children.first.is_a? AST::SendNode ancestor.children.first.command? :class_methods end diff --git a/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb b/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb index 808b1a3..86def82 100644 --- a/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb @@ -90,7 +90,7 @@ def on_ivasgn(node) end def on_or_asgn(node) - return unless node.assignment_node&.ivasgn_type? + return unless node.assignment_node.ivasgn_type? return unless in_class?(node) on_assignment(node.expression) diff --git a/spec/rubocop/cop/thread_safety/class_and_module_attributes_spec.rb b/spec/rubocop/cop/thread_safety/class_and_module_attributes_spec.rb index 078434d..637cc20 100644 --- a/spec/rubocop/cop/thread_safety/class_and_module_attributes_spec.rb +++ b/spec/rubocop/cop/thread_safety/class_and_module_attributes_spec.rb @@ -90,6 +90,14 @@ class << self RUBY end + it 'registers no offense for `attr_accessor` within procs' do + expect_no_offenses(<<~RUBY) + module Test + DEFINE_ACCESSOR = ->{ attr_accessor :foobar } + end + RUBY + end + context 'when in a singleton class method' do it 'registers no offense for `attr_accessor`' do expect_no_offenses(<<~RUBY) diff --git a/spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb index e53b185..7476ec4 100644 --- a/spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb @@ -266,6 +266,20 @@ def some_method(params) RUBY end + it 'does not register an offenses for synchronized ivar_set in a method marked by module_function' do + expect_no_offenses(<<~RUBY) + module Test + def some_method(params) + $mutex.synchronize do + instance_variable_set(:@params, params) + end + end + + module_function :some_method + end + RUBY + end + it 'registers an offense for assigning an ivar in a method below module_function directive' do expect_offense(<<~RUBY) module Test diff --git a/spec/rubocop/cop/thread_safety/mutable_class_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/mutable_class_instance_variable_spec.rb index 564b389..05535b4 100644 --- a/spec/rubocop/cop/thread_safety/mutable_class_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/mutable_class_instance_variable_spec.rb @@ -41,6 +41,12 @@ def surround(code) @var ||= #{o}.freeze RUBY end + + it 'registers no offense for `or-asgn` node without ivar' do + expect_no_offenses(surround(<<~RUBY)) + var ||= %{o} + RUBY + end end end @@ -346,6 +352,12 @@ def some_method @a, @b, @c = 'foo'.freeze, [2].freeze, 3 RUBY end + + it 'registers no offenses for multiple mutable objects without class' do + expect_no_offenses(<<~RUBY) + @a, @b, @c = 'foo', [2], 3 + RUBY + end end it 'freezes a heredoc' do @@ -677,6 +689,12 @@ def assignment? @a, @b, @c = 'foo'.freeze, [2].freeze, 3 RUBY end + + it 'registers no offenses for single assignment' do + expect_no_offenses(surround(<<~RUBY)) + @a, @b = 1 + RUBY + end end end end From 9d8d81e6449c9d0d868124a191f2d9b800dce5c8 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Fri, 11 Oct 2024 18:50:52 +0300 Subject: [PATCH 2/2] Handle legacy `LambdaNode --- .../class_instance_variable_spec.rb | 17 +++++++++++++++++ spec/spec_helper.rb | 10 ++++++++++ 2 files changed, 27 insertions(+) diff --git a/spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb index 7476ec4..96d0db0 100644 --- a/spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb @@ -157,6 +157,23 @@ def some_method(params) RUBY end + # rubocop:disable RSpec/ExampleLength + it 'registers an offense for assigning an ivar in class_methods within lambda', :with_legacy_lambda_node do + expect_offense(<<~RUBY) + module Test + class_methods do + ->() { + def some_method(params) + @params = params + ^^^^^^^ #{msg} + end + } + end + end + RUBY + end + # rubocop:enable RSpec/ExampleLength + it 'registers an offense for assigning an ivar in a class singleton method' do expect_offense(<<~RUBY) class Test diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2c6f662..0586e81 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -16,6 +16,7 @@ $LOAD_PATH.unshift File.expand_path('../lib', __dir__) require 'rubocop-thread_safety' +require 'rubocop-ast' require 'rubocop/rspec/support' require_relative 'shared_contexts' @@ -40,6 +41,15 @@ config.fail_fast = ENV.key? 'RSPEC_FAIL_FAST' end + config.around :each, :with_legacy_lambda_node do |example| + initial_value = RuboCop::AST::Builder.emit_lambda + RuboCop::AST::Builder.emit_lambda = true + + example.call + ensure + RuboCop::AST::Builder.emit_lambda = initial_value + end + config.filter_run_excluding unsupported_on: :prism if ENV['PARSER_ENGINE'] == 'parser_prism' config.disable_monkey_patching!