Merge pull request #73 from viralpraxis/add-allow-call-with-block-opt…

…ion-to-dir-chdir-cop

Add `AllowCallWithBlock` option to `ThreadSafety/DirChdir` cop

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Change log
 
+## master
+
+- [#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
 
 * [#59](https://github.com/rubocop/rubocop-thread_safety/pull/59): Rename `ThreadSafety::InstanceVariableInClassMethod` cop to `ThreadSafety::ClassInstanceVariable` to better reflect its purpose. ([@viralpraxis](https://github.com/viralpraxis))

config/default.yml

@@ -35,6 +35,7 @@ ThreadSafety/NewThread:
 ThreadSafety/DirChdir:
   Description: Avoid using `Dir.chdir` due to its process-wide effect.
   Enabled: true
+  AllowCallWithBlock: false
 
 ThreadSafety/RackMiddlewareInstanceVariable:
   Description: Avoid instance variables in Rack middleware.

docs/modules/ROOT/pages/cops_threadsafety.adoc

@@ -136,6 +136,8 @@ end
 |===
 
 Avoid using `Dir.chdir` due to its process-wide effect.
+If `AllowCallWithBlock` (disabled by default) option is enabled,
+calling `Dir.chdir` with block will be allowed.
 
 [#examples-threadsafetydirchdir]
 === Examples
@@ -149,6 +151,39 @@ Dir.chdir("/var/run")
 FileUtils.chdir("/var/run")
 ----
 
+[#allowcallwithblock_-false-_default_-threadsafetydirchdir]
+==== AllowCallWithBlock: false (default)
+
+[source,ruby]
+----
+# good
+Dir.chdir("/var/run") do
+  puts Dir.pwd
+end
+----
+
+[#allowcallwithblock_-true-threadsafetydirchdir]
+==== AllowCallWithBlock: true
+
+[source,ruby]
+----
+# bad
+Dir.chdir("/var/run") do
+  puts Dir.pwd
+end
+----
+
+[#configurable-attributes-threadsafetydirchdir]
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| AllowCallWithBlock
+| `false`
+| Boolean
+|===
+
 [#threadsafetymutableclassinstancevariable]
 == ThreadSafety/MutableClassInstanceVariable
 

lib/rubocop/cop/thread_safety/dir_chdir.rb

@@ -4,13 +4,28 @@ module RuboCop
   module Cop
     module ThreadSafety
       # Avoid using `Dir.chdir` due to its process-wide effect.
+      # If `AllowCallWithBlock` (disabled by default) option is enabled,
+      # calling `Dir.chdir` with block will be allowed.
       #
       # @example
       #   # bad
       #   Dir.chdir("/var/run")
       #
       #   # bad
       #   FileUtils.chdir("/var/run")
+      #
+      # @example AllowCallWithBlock: false (default)
+      #   # good
+      #   Dir.chdir("/var/run") do
+      #     puts Dir.pwd
+      #   end
+      #
+      # @example AllowCallWithBlock: true
+      #   # bad
+      #   Dir.chdir("/var/run") do
+      #     puts Dir.pwd
+      #   end
+      #
       class DirChdir < Base
         MESSAGE = 'Avoid using `%<module>s.%<method>s` due to its process-wide effect.'
         RESTRICT_ON_SEND = %i[chdir cd].freeze
@@ -24,12 +39,19 @@ class DirChdir < Base
         MATCHER
 
         def on_send(node)
-          chdir?(node) do
-            add_offense(
-              node,
-              message: format(MESSAGE, module: node.receiver.short_name, method: node.method_name)
-            )
-          end
+          return unless chdir?(node)
+          return if allow_call_with_block? && (node.block_argument? || node.parent&.block_type?)
+
+          add_offense(
+            node,
+            message: format(MESSAGE, module: node.receiver.short_name, method: node.method_name)
+          )
+        end
+
+        private
+
+        def allow_call_with_block?
+          !!cop_config['AllowCallWithBlock']
         end
       end
     end

spec/rubocop/cop/thread_safety/dir_chdir_spec.rb

@@ -24,6 +24,33 @@
         ^^^^^^^^^^^^^^^^^^^^^^^ #{msg}
       RUBY
     end
+
+    it 'registers an offense with provided block' do
+      expect_offense(<<~RUBY)
+        Dir.chdir("/var/run") do
+        ^^^^^^^^^^^^^^^^^^^^^ #{msg}
+          puts Dir.pwd
+        end
+      RUBY
+    end
+
+    it 'registers an offense with provided block with argument' do
+      expect_offense(<<~RUBY)
+        Dir.chdir("/var/run") do |dir|
+        ^^^^^^^^^^^^^^^^^^^^^ #{msg}
+          puts dir
+        end
+      RUBY
+    end
+
+    it 'registers an offense with provided block argument' do
+      expect_offense(<<~RUBY)
+        def change_dir(&block)
+          Dir.chdir("/var/run", &block)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg}
+        end
+      RUBY
+    end
   end
 
   context 'with `FileUtils.chdir` method' do
@@ -54,9 +81,46 @@
     end
   end
 
-  context 'when received is not `Dir`' do
+  context 'when receiver is not `Dir`' do
     it 'does not register an offense' do
       expect_no_offenses 'chdir("/tmp")'
     end
   end
+
+  context 'with `AllowCallWithBlock` config set to `true`' do
+    let(:cop_config) do
+      { 'Enabled' => true, 'AllowCallWithBlock' => true }
+    end
+
+    it 'registers an offense' do
+      expect_offense(<<~RUBY)
+        Dir.chdir("/var/run")
+        ^^^^^^^^^^^^^^^^^^^^^ #{msg}
+      RUBY
+    end
+
+    it 'does not register an offense with provided block' do
+      expect_no_offenses(<<~RUBY)
+        Dir.chdir("/var/run") do
+          puts Dir.pwd
+        end
+      RUBY
+    end
+
+    it 'does not register an offense with provided block with argument' do
+      expect_no_offenses(<<~RUBY)
+        Dir.chdir("/var/run") do |dir|
+          puts dir
+        end
+      RUBY
+    end
+
+    it 'does not register an offense with provided block argument' do
+      expect_no_offenses(<<~RUBY)
+        def change_dir(&block)
+          Dir.chdir("/var/run", &block)
+        end
+      RUBY
+    end
+  end
 end