Merge pull request #76 from viralpraxis/fix-internal-affairs-on-send-…

…without-on-csend-cop

Fix offenses for `InternalAffairs/OnSendWithoutOnCSend` cop

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

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)

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)

lib/rubocop/cop/thread_safety/dir_chdir.rb

@@ -27,14 +27,14 @@ module ThreadSafety
       #   end
       #
       class DirChdir < Base
-        MESSAGE = 'Avoid using `%<module>s.%<method>s` due to its process-wide effect.'
+        MESSAGE = 'Avoid using `%<module>s%<dot>s%<method>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
 

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

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
 

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)

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

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