Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix an incorrect autocorrect for Lint/NonAtomicFileOperation #12148

Merged
merged 1 commit into from
Aug 25, 2023
Merged

Fix an incorrect autocorrect for Lint/NonAtomicFileOperation #12148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/fix_an_incorrect_autocorrect_for_lint_non_atomic_file_operation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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][])
12 changes: 8 additions & 4 deletions lib/rubocop/cop/lint/non_atomic_file_operation.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
21 changes: 19 additions & 2 deletions spec/rubocop/cop/lint/non_atomic_file_operation_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down