From da7ff7dd9d3849dbe4cbe5ee635e2d32ae61e73f Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 5 Jul 2022 00:18:54 +0900 Subject: [PATCH] [Fix #10786] Fix a false positive for `Lint/NonAtomicFileOperation` Fixes #10786. This PR fixes a false positive for `Lint/NonAtomicFileOperation` when using complex conditional. --- ...ositive_for_lint_non_atomic_file_operation.md | 1 + .../cop/lint/non_atomic_file_operation.rb | 14 +++++++++----- .../cop/lint/non_atomic_file_operation_spec.rb | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 changelog/fix_a_false_positive_for_lint_non_atomic_file_operation.md diff --git a/changelog/fix_a_false_positive_for_lint_non_atomic_file_operation.md b/changelog/fix_a_false_positive_for_lint_non_atomic_file_operation.md new file mode 100644 index 00000000000..0758edae4d4 --- /dev/null +++ b/changelog/fix_a_false_positive_for_lint_non_atomic_file_operation.md @@ -0,0 +1 @@ +* [#10786](https://github.com/rubocop/rubocop/issues/10786): Fix a false positive for `Lint/NonAtomicFileOperation` when using complex conditional. ([@koic][]) diff --git a/lib/rubocop/cop/lint/non_atomic_file_operation.rb b/lib/rubocop/cop/lint/non_atomic_file_operation.rb index 9aad8addc21..f8d51982cf9 100644 --- a/lib/rubocop/cop/lint/non_atomic_file_operation.rb +++ b/lib/rubocop/cop/lint/non_atomic_file_operation.rb @@ -71,18 +71,22 @@ class NonAtomicFileOperation < Base PATTERN def on_send(node) - return unless node.parent&.if_type? - return if node.parent.else_branch + return unless (parent = node.parent) && parent.if_type? + return if allowable_use_with_if?(parent) return if explicit_not_force?(node) - return unless (exist_node = send_exist_node(node.parent).first) + return unless (exist_node = send_exist_node(parent).first) return unless exist_node.first_argument == node.first_argument - offense(node, exist_node) + register_offense(node, exist_node) end private - def offense(node, exist_node) + def allowable_use_with_if?(if_node) + if_node.condition.and_type? || if_node.condition.or_type? || if_node.else_branch + end + + def register_offense(node, exist_node) range = range_between(node.parent.loc.keyword.begin_pos, exist_node.loc.expression.end_pos) diff --git a/spec/rubocop/cop/lint/non_atomic_file_operation_spec.rb b/spec/rubocop/cop/lint/non_atomic_file_operation_spec.rb index 0d1f09b236e..7a0d5e66842 100644 --- a/spec/rubocop/cop/lint/non_atomic_file_operation_spec.rb +++ b/spec/rubocop/cop/lint/non_atomic_file_operation_spec.rb @@ -183,4 +183,20 @@ end RUBY end + + it 'does not register an offense when using complex conditional with `&&`' do + expect_no_offenses(<<~RUBY) + if FileTest.exist?(path) && File.stat(path).socket? + FileUtils.mkdir(path) + end + RUBY + end + + it 'does not register an offense when using complex conditional with `||`' do + expect_no_offenses(<<~RUBY) + if FileTest.exist?(path) || condition + FileUtils.mkdir(path) + end + RUBY + end end