diff --git a/changelog/fix_fix_two_autocorrection_issues_for.md b/changelog/fix_fix_two_autocorrection_issues_for.md new file mode 100644 index 00000000000..e4da5771836 --- /dev/null +++ b/changelog/fix_fix_two_autocorrection_issues_for.md @@ -0,0 +1 @@ +* [#9731](https://github.com/rubocop/rubocop/issues/9731): Fix two autocorrection issues for `Style/NegatedIfElseCondition`. ([@dvandersluis][]) diff --git a/lib/rubocop/cop/style/negated_if_else_condition.rb b/lib/rubocop/cop/style/negated_if_else_condition.rb index 469c032781a..fb7ac62ca9b 100644 --- a/lib/rubocop/cop/style/negated_if_else_condition.rb +++ b/lib/rubocop/cop/style/negated_if_else_condition.rb @@ -29,7 +29,6 @@ module Style # class NegatedIfElseCondition < Base include RangeHelp - include CommentsHelp extend AutoCorrector MSG = 'Invert the negated condition and swap the %s branches.' @@ -98,21 +97,30 @@ def swap_branches(corrector, node) if node.if_branch.nil? corrector.remove(range_by_whole_lines(node.loc.else, include_final_newline: true)) else - if_range = node_with_comments(node.if_branch) - else_range = node_with_comments(node.else_branch) + if_range = if_range(node) + else_range = else_range(node) 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? + # Collect the entire if branch, including whitespace and comments + def if_range(node) + if node.ternary? + node.if_branch + else + range_between(node.condition.loc.expression.end_pos, node.loc.else.begin_pos) + end + end - 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) + # Collect the entire else branch, including whitespace and comments + def else_range(node) + if node.ternary? + node.else_branch + else + range_between(node.loc.else.end_pos, node.loc.end.begin_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 0527ac8f922..a760e2bef09 100644 --- a/spec/rubocop/cop/style/negated_if_else_condition_spec.rb +++ b/spec/rubocop/cop/style/negated_if_else_condition_spec.rb @@ -50,6 +50,21 @@ RUBY end + it 'registers an offens and corrects a multiline ternary' do + expect_offense(<<~RUBY) + !x ? + ^^^^ Invert the negated condition and swap the ternary branches. + do_something : + do_something_else # comment + RUBY + + expect_correction(<<~RUBY) + x ? + do_something_else : + do_something # comment + RUBY + end + shared_examples 'negation method' do |method, inverted_method| it "registers an offense and corrects when negating condition with `#{method}` for `if-else`" do expect_offense(<<~RUBY, method: method) @@ -129,6 +144,23 @@ RUBY end + it 'does not register an offense when the `else` branch is empty' do + expect_no_offenses(<<~RUBY) + if !condition.nil? + foo = 42 + else + end + RUBY + end + + it 'does not register an offense when both branches are empty' do + expect_no_offenses(<<~RUBY) + if !condition.nil? + else + end + RUBY + end + it 'moves comments to correct branches during autocorrect' do expect_offense(<<~RUBY) if !condition.nil? @@ -187,6 +219,98 @@ RUBY end + it 'works with comments when one branch is a begin and the other is not' do + expect_offense(<<~RUBY) + if !condition + ^^^^^^^^^^^^^ Invert the negated condition and swap the if-else branches. + # comment + do_a + do_b + else + do_c + end + RUBY + + expect_correction(<<~RUBY) + if condition + do_c + else + # comment + do_a + do_b + end + RUBY + end + + it 'works with comments when neither branch is a begin node' do + expect_offense(<<~RUBY) + if !condition + ^^^^^^^^^^^^^ Invert the negated condition and swap the if-else branches. + # comment + do_b + else + do_c + end + RUBY + + expect_correction(<<~RUBY) + if condition + do_c + else + # comment + do_b + end + RUBY + end + + it 'works with duplicate nodes' do + expect_offense(<<~RUBY) + # outer comment + do_a + + if !condition + ^^^^^^^^^^^^^ Invert the negated condition and swap the if-else branches. + # comment + do_a + else + do_c + end + RUBY + + expect_correction(<<~RUBY) + # outer comment + do_a + + if condition + do_c + else + # comment + do_a + end + RUBY + end + + it 'correctly moves comments at the end of branches' do + expect_offense(<<~RUBY) + if !condition + ^^^^^^^^^^^^^ Invert the negated condition and swap the if-else branches. + do_a + # comment + else + do_c + end + RUBY + + expect_correction(<<~RUBY) + if condition + do_c + else + do_a + # comment + end + RUBY + end + it 'does not register an offense when negating condition for `if-elsif`' do expect_no_offenses(<<~RUBY) if !x