Merge pull request #12148 from koic/fix_an_incorrect_autocorrect_for_…

…lint_non_atomic_file_operation Fix an incorrect autocorrect for `Lint/NonAtomicFileOperation`
rubocop · Aug 25, 2023 · 0ff3a62 · 0ff3a62
2 parents 342bbbe + 7aaf70f
commit 0ff3a62
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 6 deletions.
diff --git a/changelog/fix_an_incorrect_autocorrect_for_lint_non_atomic_file_operation.md b/changelog/fix_an_incorrect_autocorrect_for_lint_non_atomic_file_operation.md
@@ -0,0 +1 @@
+* [#12148](https://github.com/rubocop/rubocop/pull/12148): Fix an incorrect autocorrect for `Lint/NonAtomicFileOperation` when using `FileUtils.remove_dir`, `FileUtils.remove_entry`, or `FileUtils.remove_entry_secure`. ([@koic][])
diff --git a/lib/rubocop/cop/lint/non_atomic_file_operation.rb b/lib/rubocop/cop/lint/non_atomic_file_operation.rb
@@ -51,10 +51,12 @@ class NonAtomicFileOperation < Base
         MAKE_FORCE_METHODS = %i[makedirs mkdir_p mkpath].freeze
         MAKE_METHODS = %i[mkdir].freeze
         REMOVE_FORCE_METHODS = %i[rm_f rm_rf].freeze
-        REMOVE_METHODS = %i[remove remove_dir remove_entry remove_entry_secure
-                            delete unlink remove_file rm rmdir safe_unlink].freeze
-        RESTRICT_ON_SEND = (MAKE_METHODS + MAKE_FORCE_METHODS + REMOVE_METHODS +
-          REMOVE_FORCE_METHODS).freeze
+        REMOVE_METHODS = %i[remove delete unlink remove_file rm rmdir safe_unlink].freeze
+        RECURSIVE_REMOVE_METHODS = %i[remove_dir remove_entry remove_entry_secure].freeze
+        RESTRICT_ON_SEND = (
+          MAKE_METHODS + MAKE_FORCE_METHODS + REMOVE_METHODS + RECURSIVE_REMOVE_METHODS +
+          REMOVE_FORCE_METHODS
+        ).freeze
 
         # @!method send_exist_node(node)
         def_node_search :send_exist_node, <<~PATTERN
@@ -140,6 +142,8 @@ def replacement_method(node)
             'mkdir_p'
           elsif REMOVE_METHODS.include?(node.method_name)
             'rm_f'
+          elsif RECURSIVE_REMOVE_METHODS.include?(node.method_name)
+            'rm_rf'
           else
             node.method_name
           end

diff --git a/spec/rubocop/cop/lint/non_atomic_file_operation_spec.rb b/spec/rubocop/cop/lint/non_atomic_file_operation_spec.rb
@@ -34,8 +34,7 @@
     end
   end
 
-  %i[remove remove_dir remove_entry remove_entry_secure delete unlink
-     remove_file rm rmdir safe_unlink].each do |remove_method|
+  %i[remove delete unlink remove_file rm rmdir safe_unlink].each do |remove_method|
     it 'registers an offense when use `FileTest.exist?` before remove file' do
       expect_offense(<<~RUBY, remove_method: remove_method)
         if FileTest.exist?(path)
@@ -53,6 +52,24 @@
     end
   end
 
+  %i[remove_dir remove_entry remove_entry_secure].each do |remove_method|
+    it 'registers an offense when use `FileTest.exist?` before recursive remove file' do
+      expect_offense(<<~RUBY, remove_method: remove_method)
+        if FileTest.exist?(path)
+        ^^^^^^^^^^^^^^^^^^^^^^^^ Remove unnecessary existence check `FileTest.exist?`.
+          FileUtils.#{remove_method}(path)
+          ^^^^^^^^^^^{remove_method}^^^^^^ Use atomic file operation method `FileUtils.rm_rf`.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+
+        #{trailing_whitespace}#{trailing_whitespace}FileUtils.rm_rf(path)
+
+      RUBY
+    end
+  end
+
   %i[rm_f rm_rf].each do |remove_method|
     it 'registers an offense when use `FileTest.exist?` before force remove file' do
       expect_offense(<<~RUBY)