From 91b16241a0d9f8d3c31d0bba7da15f867942682c Mon Sep 17 00:00:00 2001
From: ydah <t.yudai92@gmail.com>
Date: Mon, 26 Feb 2024 16:40:57 +0900
Subject: [PATCH] Fix an incorrect autocorrect for `RSpec/ChangeByZero` when
 compound expectations with line break before `.by(0)`

Fix: https://github.com/rubocop/rubocop-rspec/issues/1815
---
 CHANGELOG.md                                  |  1 +
 lib/rubocop/cop/rspec/change_by_zero.rb       | 28 +++++++-
 spec/rubocop/cop/rspec/change_by_zero_spec.rb | 68 +++++++++++++++++++
 3 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index eb2f01bd7..bcad4b98c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,7 @@
 - Add configuration option `ResponseMethods` to `RSpec/Rails/HaveHttpStatus`. ([@ydah])
 - Fix a false negative for `RSpec/DescribedClass` when class with constant. ([@ydah])
 - Fix a false positive for `RSpec/ExampleWithoutDescription` when `specify` with multi-line block and missing description. ([@ydah])
+- Fix an incorrect autocorrect for `RSpec/ChangeByZero` when compound expectations with line break before `.by(0)`. ([@ydah])
 
 ## 2.26.1 (2024-01-05)
 
diff --git a/lib/rubocop/cop/rspec/change_by_zero.rb b/lib/rubocop/cop/rspec/change_by_zero.rb
index 457b645a4..d37ddd4dd 100644
--- a/lib/rubocop/cop/rspec/change_by_zero.rb
+++ b/lib/rubocop/cop/rspec/change_by_zero.rb
@@ -59,6 +59,8 @@ module RSpec
       #
       class ChangeByZero < Base
         extend AutoCorrector
+        include RangeHelp
+
         MSG = 'Prefer `not_to change` over `to %<method>s.by(0)`.'
         MSG_COMPOUND = 'Prefer %<preferred>s with compound expectations ' \
                        'over `%<method>s.by(0)`.'
@@ -140,8 +142,32 @@ def autocorrect_compound(corrector, node)
 
           change_nodes(node) do |change_node|
             corrector.replace(change_node.loc.selector, negated_matcher)
-            range = node.loc.dot.with(end_pos: node.source_range.end_pos)
+            insert_operator(corrector, node, change_node)
+            remove_by_zero(corrector, node, change_node)
+          end
+        end
+
+        def insert_operator(corrector, node, change_node)
+          operator = node.right_siblings.first
+          return unless %i[& |].include?(operator)
+
+          corrector.insert_after(
+            replace_node(node, change_node), " #{operator}"
+          )
+        end
+
+        def replace_node(node, change_node)
+          expect_change_with_arguments(node) ? change_node : change_node.parent
+        end
+
+        def remove_by_zero(corrector, node, change_node)
+          range = node.loc.dot.with(end_pos: node.source_range.end_pos)
+          if change_node.loc.line == range.line
             corrector.remove(range)
+          else
+            corrector.remove(
+              range_by_whole_lines(range, include_final_newline: true)
+            )
           end
         end
 
diff --git a/spec/rubocop/cop/rspec/change_by_zero_spec.rb b/spec/rubocop/cop/rspec/change_by_zero_spec.rb
index a67894099..5f313895b 100644
--- a/spec/rubocop/cop/rspec/change_by_zero_spec.rb
+++ b/spec/rubocop/cop/rspec/change_by_zero_spec.rb
@@ -258,6 +258,74 @@
         end
       RUBY
     end
+
+    it 'registers an offense and autocorrect when ' \
+       'the argument to `by` is zero with compound expectations (`and`/`or`) ' \
+       'with line break before `.by(0)`' do
+      expect_offense(<<~RUBY)
+        it do
+          expect { foo }
+            .to change(Foo, :bar)
+                ^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+            .by(0)
+            .and change(Foo, :baz)
+                 ^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+            .by(0)
+          expect { foo }
+            .to change { Foo.bar }
+                ^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+            .by(0)
+            .or change { Foo.baz }
+                ^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+            .by(0)
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        it do
+          expect { foo }
+            .to not_change(Foo, :bar)
+            .and not_change(Foo, :baz)
+          expect { foo }
+            .to not_change { Foo.bar }
+            .or not_change { Foo.baz }
+        end
+      RUBY
+    end
+
+    it 'registers an offense and autocorrect when ' \
+       'the argument to `by` is zero with compound expectations (`&`/`|`) ' \
+       'with line break before `.by(0)`' do
+      expect_offense(<<~RUBY)
+        it do
+          expect { foo }
+            .to change(Foo, :bar)
+                ^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+            .by(0) &
+            change(Foo, :baz)
+            ^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+            .by(0)
+          expect { foo }
+            .to change { Foo.bar }
+                ^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+            .by(0) |
+            change { Foo.baz }
+            ^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+            .by(0)
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        it do
+          expect { foo }
+            .to not_change(Foo, :bar) &
+            not_change(Foo, :baz)
+          expect { foo }
+            .to not_change { Foo.bar } |
+            not_change { Foo.baz }
+        end
+      RUBY
+    end
   end
 
   it 'does not register an offense when the argument to `by` is not zero' do