diff --git a/changelog/fix_comments_negated_if_else_condition_cop.md b/changelog/fix_comments_negated_if_else_condition_cop.md new file mode 100644 index 00000000000..f87bdf196aa --- /dev/null +++ b/changelog/fix_comments_negated_if_else_condition_cop.md @@ -0,0 +1 @@ +* [#9429](https://github.com/rubocop-hq/rubocop/issues/9429): Fix `Style/NegatedIfElseCondition` autocorrect to keep comments in correct branch. ([@tejasbubane][]) diff --git a/lib/rubocop/cop/style/negated_if_else_condition.rb b/lib/rubocop/cop/style/negated_if_else_condition.rb index f0669f81974..85961e7aa1e 100644 --- a/lib/rubocop/cop/style/negated_if_else_condition.rb +++ b/lib/rubocop/cop/style/negated_if_else_condition.rb @@ -29,6 +29,7 @@ module Style # class NegatedIfElseCondition < Base include RangeHelp + include CommentsHelp extend AutoCorrector MSG = 'Invert the negated condition and swap the %s branches.' @@ -96,10 +97,22 @@ def swap_branches(corrector, node) if node.if_branch.nil? corrector.remove(range_by_whole_lines(node.loc.else, include_final_newline: true)) else - corrector.replace(node.if_branch, node.else_branch.source) - corrector.replace(node.else_branch, node.if_branch.source) + if_range = node_with_comments(node.if_branch) + else_range = node_with_comments(node.else_branch) + + corrector.replace(if_range, else_range.source) + corrector.replace(else_range, if_range.source) end end + + def node_with_comments(node) + first_statement = node.begin_type? ? node.children[0] : node + return node if processed_source.ast_with_comments[first_statement].empty? + + begin_pos = source_range_with_comment(first_statement).begin_pos + end_pos = node.source_range.end_pos + Parser::Source::Range.new(buffer, begin_pos, end_pos) + end end end end diff --git a/spec/rubocop/cop/style/negated_if_else_condition_spec.rb b/spec/rubocop/cop/style/negated_if_else_condition_spec.rb index 3a87f8baa4c..0527ac8f922 100644 --- a/spec/rubocop/cop/style/negated_if_else_condition_spec.rb +++ b/spec/rubocop/cop/style/negated_if_else_condition_spec.rb @@ -129,6 +129,64 @@ RUBY end + it 'moves comments to correct branches during autocorrect' do + expect_offense(<<~RUBY) + if !condition.nil? + ^^^^^^^^^^^^^^^^^^ Invert the negated condition and swap the if-else branches. + # part B + # and foo is 39 + foo = 39 + else + # part A + # and foo is 42 + foo = 42 + end + RUBY + + expect_correction(<<~RUBY) + if condition.nil? + # part A + # and foo is 42 + foo = 42 + else + # part B + # and foo is 39 + foo = 39 + end + RUBY + end + + it 'works with comments and multiple statements' do + expect_offense(<<~RUBY) + if !condition.nil? + ^^^^^^^^^^^^^^^^^^ Invert the negated condition and swap the if-else branches. + # part A + # and foo is 1 and bar is 2 + foo = 1 + bar = 2 + else + # part B + # and foo is 3 and bar is 4 + foo = 3 + bar = 4 + end + RUBY + + expect_correction(<<~RUBY) + if condition.nil? + # part B + # and foo is 3 and bar is 4 + foo = 3 + bar = 4 + else + # part A + # and foo is 1 and bar is 2 + foo = 1 + bar = 2 + end + RUBY + end + it 'does not register an offense when negating condition for `if-elsif`' do expect_no_offenses(<<~RUBY) if !x