Skip to content

Commit

Permalink
[Fix #10813] Fix recursive deletion to suppression in `Lint/NonAtomic…
Browse files Browse the repository at this point in the history
…FileOperation`

Fixes: #10813

This PR is suppress offense in case of
recursive deletion in `Lint/NonAtomicFileOperation`.
  • Loading branch information
ydah authored and bbatsov committed Jul 21, 2022
1 parent 2f8d8c7 commit 0124f73
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 17 deletions.
@@ -0,0 +1 @@
* [#10813](https://github.com/rubocop/rubocop/issues/10813): Fix recursive deletion to suppression in `Lint/NonAtomicFileOperation`. ([@ydah][])
16 changes: 9 additions & 7 deletions lib/rubocop/cop/lint/non_atomic_file_operation.rb
Expand Up @@ -51,12 +51,11 @@ class NonAtomicFileOperation < Base
MSG_CHANGE_FORCE_METHOD = 'Use atomic file operation method `FileUtils.%<method_name>s`.'
MAKE_FORCE_METHODS = %i[makedirs mkdir_p mkpath].freeze
MAKE_METHODS = %i[mkdir].freeze
REMOVE_RECURSIVE_METHODS = %i[rm_r rmtree].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 + REMOVE_RECURSIVE_METHODS).freeze
REMOVE_FORCE_METHODS).freeze

# @!method send_exist_node(node)
def_node_search :send_exist_node, <<-PATTERN
Expand All @@ -79,17 +78,22 @@ class NonAtomicFileOperation < Base
PATTERN

def on_send(node)
return unless (parent = node.parent) && parent.if_type?
return if allowable_use_with_if?(parent)
return unless if_node_child?(node)
return if explicit_not_force?(node)
return unless (exist_node = send_exist_node(parent).first)
return unless (exist_node = send_exist_node(node.parent).first)
return unless exist_node.first_argument == node.first_argument

register_offense(node, exist_node)
end

private

def if_node_child?(node)
return false unless (parent = node.parent)

parent.if_type? && !allowable_use_with_if?(parent)
end

def allowable_use_with_if?(if_node)
if_node.condition.and_type? || if_node.condition.or_type? || if_node.else_branch
end
Expand Down Expand Up @@ -129,8 +133,6 @@ def autocorrect_replace_method(corrector, node)
def replacement_method(node)
if MAKE_METHODS.include?(node.method_name)
'mkdir_p'
elsif REMOVE_RECURSIVE_METHODS.include?(node.method_name)
'rm_rf'
elsif REMOVE_METHODS.include?(node.method_name)
'rm_f'
else
Expand Down
12 changes: 2 additions & 10 deletions spec/rubocop/cop/lint/non_atomic_file_operation_spec.rb
Expand Up @@ -71,20 +71,12 @@
end

%i[rm_r rmtree].each do |remove_method|
it 'registers an offense when use `FileTest.exist?` before remove recursive file' do
expect_offense(<<~RUBY, remove_method: remove_method)
it 'does not register an offense when use `FileTest.exist?` before remove recursive file' do
expect_no_offenses(<<~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

Expand Down

0 comments on commit 0124f73

Please sign in to comment.