diff --git a/CHANGELOG.md b/CHANGELOG.md index 33051a5..2467eef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## master +- [#76](https://github.com/rubocop/rubocop-thread_safety/pull/76): Detect offenses when using safe navigation for `ThreadSafety/DirChdir`, `ThreadSafety/NewThread` and `ThreadSafety/RackMiddlewareInstanceVariable` cops. ([@viralpraxis](https://github.com/viralpraxis)) - [#73](https://github.com/rubocop/rubocop-thread_safety/pull/73): Add `AllowCallWithBlock` option to `ThreadSafety/DirChdir` cop. ([@viralpraxis](https://github.com/viralpraxis)) ## 0.6.0 diff --git a/lib/rubocop/cop/thread_safety/class_and_module_attributes.rb b/lib/rubocop/cop/thread_safety/class_and_module_attributes.rb index b853292..efd48dc 100644 --- a/lib/rubocop/cop/thread_safety/class_and_module_attributes.rb +++ b/lib/rubocop/cop/thread_safety/class_and_module_attributes.rb @@ -49,7 +49,7 @@ class ClassAndModuleAttributes < Base ...) MATCHER - def on_send(node) + def on_send(node) # rubocop:disable InternalAffairs/OnSendWithoutOnCSend return unless mattr?(node) || (!class_attribute_allowed? && class_attr?(node)) || singleton_attr?(node) add_offense(node) diff --git a/lib/rubocop/cop/thread_safety/class_instance_variable.rb b/lib/rubocop/cop/thread_safety/class_instance_variable.rb index 2b7811c..1136c20 100644 --- a/lib/rubocop/cop/thread_safety/class_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/class_instance_variable.rb @@ -87,7 +87,7 @@ def on_ivar(node) end alias on_ivasgn on_ivar - def on_send(node) + def on_send(node) # rubocop:disable InternalAffairs/OnSendWithoutOnCSend return unless instance_variable_call?(node) return unless class_method_definition?(node) return if method_definition?(node) diff --git a/lib/rubocop/cop/thread_safety/dir_chdir.rb b/lib/rubocop/cop/thread_safety/dir_chdir.rb index 12951b2..7f4a316 100644 --- a/lib/rubocop/cop/thread_safety/dir_chdir.rb +++ b/lib/rubocop/cop/thread_safety/dir_chdir.rb @@ -27,14 +27,14 @@ module ThreadSafety # end # class DirChdir < Base - MESSAGE = 'Avoid using `%s.%s` due to its process-wide effect.' + MESSAGE = 'Avoid using `%s%s%s` due to its process-wide effect.' RESTRICT_ON_SEND = %i[chdir cd].freeze # @!method chdir?(node) def_node_matcher :chdir?, <<~MATCHER { - (send (const {nil? cbase} {:Dir :FileUtils}) :chdir ...) - (send (const {nil? cbase} :FileUtils) :cd ...) + ({send csend} (const {nil? cbase} {:Dir :FileUtils}) :chdir ...) + ({send csend} (const {nil? cbase} :FileUtils) :cd ...) } MATCHER @@ -44,9 +44,15 @@ def on_send(node) add_offense( node, - message: format(MESSAGE, module: node.receiver.short_name, method: node.method_name) + message: format( + MESSAGE, + module: node.receiver.short_name, + method: node.method_name, + dot: node.loc.dot.source + ) ) end + alias on_csend on_send private diff --git a/lib/rubocop/cop/thread_safety/new_thread.rb b/lib/rubocop/cop/thread_safety/new_thread.rb index 0bbe470..faf7e41 100644 --- a/lib/rubocop/cop/thread_safety/new_thread.rb +++ b/lib/rubocop/cop/thread_safety/new_thread.rb @@ -16,12 +16,13 @@ class NewThread < Base # @!method new_thread?(node) def_node_matcher :new_thread?, <<~MATCHER - (send (const {nil? cbase} :Thread) {:new :fork :start} ...) + ({send csend} (const {nil? cbase} :Thread) {:new :fork :start} ...) MATCHER def on_send(node) new_thread?(node) { add_offense(node) } end + alias on_csend on_send end end end diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb index c864b43..c3cfa83 100644 --- a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -93,6 +93,7 @@ def on_send(node) add_offense node end + alias on_csend on_send private diff --git a/spec/rubocop/cop/thread_safety/dir_chdir_spec.rb b/spec/rubocop/cop/thread_safety/dir_chdir_spec.rb index c3e202d..53dc4f3 100644 --- a/spec/rubocop/cop/thread_safety/dir_chdir_spec.rb +++ b/spec/rubocop/cop/thread_safety/dir_chdir_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::ThreadSafety::DirChdir, :config do - %w[Dir.chdir FileUtils.chdir FileUtils.cd].each do |expression| + %w[Dir.chdir Dir&.chdir FileUtils.chdir FileUtils.cd].each do |expression| context "with `#{expression}` call" do it 'registers an offense' do expect_offense(<<~RUBY, expression: expression) diff --git a/spec/rubocop/cop/thread_safety/new_thread_spec.rb b/spec/rubocop/cop/thread_safety/new_thread_spec.rb index b848cab..be2072a 100644 --- a/spec/rubocop/cop/thread_safety/new_thread_spec.rb +++ b/spec/rubocop/cop/thread_safety/new_thread_spec.rb @@ -45,6 +45,13 @@ RUBY end + it 'registers an offense for starting a new thread with safe navigation' do + expect_offense(<<~RUBY) + Thread&.new { do_work } + ^^^^^^^^^^^ #{msg} + RUBY + end + it 'does not register an offense for calling new on other classes' do expect_no_offenses('Other.new { do_work }') end diff --git a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb index 8bd6296..ff0ca11 100644 --- a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb @@ -311,6 +311,25 @@ def call(env) RUBY end + it 'registers an offense with safe navigation' do + expect_offense(<<~RUBY) + class TestMiddleware + def initialize(app) + @app = app + foo = SomeClass.new + instance_variable_set(:counter, 1) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} + end + + def call(env) + @app.call(env) + instance_variable_get("@counter") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} + end + end + RUBY + end + it 'registers no offenses' do expect_no_offenses(<<~RUBY) class TestMiddleware