Add NegatedMatcher configuration option to RSpec/ChangeByZero

This PR is add `NegatedMatcher` option to `RSpec/ChangeByZero`.

In the case of composite expectations, cop suggest
using the negation matchers of `RSpec::Matchers#change`.

By default doe's not support autocorrect,
but if you set the negation matcher of `RSpec::Matchers#change`
defined in `NegatedMatcher` option, will be autocorrect.

CHANGELOG.md

@@ -11,6 +11,7 @@
 * Fix a false positive for `RSpec/Capybara/SpecificMatcher`. ([@ydah][])
 * Fix a false negative for `RSpec/Capybara/SpecificMatcher` for `have_field`. ([@ydah][])
 * Fix a false positive for `RSpec/Capybara/SpecificMatcher` when may not have a `href` by `have_link`. ([@ydah][])
+* Add `NegatedMatcher` configuration option to `RSpec/ChangeByZero`. ([@ydah][])
 
 ## 2.12.1 (2022-07-03)
 

config/default.yml

@@ -192,8 +192,10 @@ RSpec/BeforeAfterAll:
 RSpec/ChangeByZero:
   Description: Prefer negated matchers over `to change.by(0)`.
   Enabled: pending
-  VersionAdded: 2.11.0
+  VersionAdded: '2.11'
+  VersionChanged: '2.13'
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ChangeByZero
+  NegatedMatcher: ~
 
 RSpec/ContextMethod:
   Description: "`context` should not be used for specifying methods."

docs/modules/ROOT/pages/cops_rspec.adoc

@@ -391,16 +391,25 @@ end
 | Pending
 | Yes
 | Yes
-| 2.11.0
-| -
+| 2.11
+| 2.13
 |===
 
 Prefer negated matchers over `to change.by(0)`.
 
-This cop does not support autocorrection in some cases.
+In the case of composite expectations, cop suggest using the
+negation matchers of `RSpec::Matchers#change`.
+
+By default the cop does not support autocorrect of
+compound expectations, but if you set the
+negated matcher for `change`, e.g. `not_change` with
+the `NegatedMatcher` option, the cop will perform the autocorrection.
+`NegatedMatcher` option, will be autocorrect.
 
 === Examples
 
+==== NegatedMatcher: ~ (default)
+
 [source,ruby]
 ----
 # bad
@@ -429,6 +438,38 @@ expect { run }
   .and not_change { Foo.baz }
 ----
 
+==== NegatedMatcher: not_change
+
+[source,ruby]
+----
+# bad (support autocorrection to good case)
+expect { run }
+  .to change(Foo, :bar).by(0)
+  .and change(Foo, :baz).by(0)
+expect { run }
+  .to change { Foo.bar }.by(0)
+  .and change { Foo.baz }.by(0)
+
+# good
+define_negated_matcher :not_change, :change
+expect { run }
+  .to not_change(Foo, :bar)
+  .and not_change(Foo, :baz)
+expect { run }
+  .to not_change { Foo.bar }
+  .and not_change { Foo.baz }
+----
+
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| NegatedMatcher
+| `<none>`
+| 
+|===
+
 === References
 
 * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ChangeByZero

lib/rubocop/cop/rspec/change_by_zero.rb

@@ -5,9 +5,16 @@ module Cop
     module RSpec
       # Prefer negated matchers over `to change.by(0)`.
       #
-      # This cop does not support autocorrection in some cases.
+      # In the case of composite expectations, cop suggest using the
+      # negation matchers of `RSpec::Matchers#change`.
       #
-      # @example
+      # By default the cop does not support autocorrect of
+      # compound expectations, but if you set the
+      # negated matcher for `change`, e.g. `not_change` with
+      # the `NegatedMatcher` option, the cop will perform the autocorrection.
+      # `NegatedMatcher` option, will be autocorrect.
+      #
+      # @example NegatedMatcher: ~ (default)
       #   # bad
       #   expect { run }.to change(Foo, :bar).by(0)
       #   expect { run }.to change { Foo.bar }.by(0)
@@ -33,10 +40,28 @@ module RSpec
       #     .to not_change { Foo.bar }
       #     .and not_change { Foo.baz }
       #
+      # @example NegatedMatcher: not_change
+      #   # bad (support autocorrection to good case)
+      #   expect { run }
+      #     .to change(Foo, :bar).by(0)
+      #     .and change(Foo, :baz).by(0)
+      #   expect { run }
+      #     .to change { Foo.bar }.by(0)
+      #     .and change { Foo.baz }.by(0)
+      #
+      #   # good
+      #   define_negated_matcher :not_change, :change
+      #   expect { run }
+      #     .to not_change(Foo, :bar)
+      #     .and not_change(Foo, :baz)
+      #   expect { run }
+      #     .to not_change { Foo.bar }
+      #     .and not_change { Foo.baz }
+      #
       class ChangeByZero < Base
         extend AutoCorrector
         MSG = 'Prefer `not_to change` over `to change.by(0)`.'
-        MSG_COMPOUND = 'Prefer negated matchers with compound expectations ' \
+        MSG_COMPOUND = 'Prefer %<preferred>s with compound expectations ' \
                           'over `change.by(0)`.'
         RESTRICT_ON_SEND = %i[change].freeze
 
@@ -57,6 +82,11 @@ class ChangeByZero < Base
             (int 0))
         PATTERN
 
+        # @!method change_nodes(node)
+        def_node_search :change_nodes, <<-PATTERN
+          $(send nil? :change ...)
+        PATTERN
+
         def on_send(node)
           expect_change_with_arguments(node.parent) do
             check_offense(node.parent)
@@ -72,7 +102,9 @@ def on_send(node)
         def check_offense(node)
           expression = node.loc.expression
           if compound_expectations?(node)
-            add_offense(expression, message: MSG_COMPOUND)
+            add_offense(expression, message: message_compound) do |corrector|
+              autocorrect_compound(corrector, node)
+            end
           else
             add_offense(expression) do |corrector|
               autocorrect(corrector, node)
@@ -89,6 +121,28 @@ def autocorrect(corrector, node)
           range = node.loc.dot.with(end_pos: node.loc.expression.end_pos)
           corrector.remove(range)
         end
+
+        def autocorrect_compound(corrector, node)
+          return unless negated_matcher
+
+          change_nodes(node) do |change_node|
+            corrector.replace(change_node.loc.selector, negated_matcher)
+            range = node.loc.dot.with(end_pos: node.loc.expression.end_pos)
+            corrector.remove(range)
+          end
+        end
+
+        def negated_matcher
+          cop_config['NegatedMatcher']
+        end
+
+        def message_compound
+          format(MSG_COMPOUND, preferred: preferred_method)
+        end
+
+        def preferred_method
+          negated_matcher ? "`#{negated_matcher}`" : 'negated matchers'
+        end
       end
     end
   end

spec/rubocop/cop/rspec/change_by_zero_spec.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-RSpec.describe RuboCop::Cop::RSpec::ChangeByZero do
+RSpec.describe RuboCop::Cop::RSpec::ChangeByZero, :config do
   it 'registers an offense when the argument to `by` is zero' do
     expect_offense(<<-RUBY)
       it do
@@ -25,54 +25,119 @@
     RUBY
   end
 
-  it 'registers an offense when the argument to `by` is zero ' \
-    'with compound expectations' do
-    expect_offense(<<-RUBY)
-      it do
-        expect { foo }
-          .to change(Foo, :bar).by(0)
-              ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-          .and change(Foo, :baz).by(0)
-               ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-        expect { foo }
-          .to change { Foo.bar }.by(0)
-              ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-          .and change { Foo.baz }.by(0)
-               ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-        expect { foo }
-          .to change(Foo, :bar).by(0) &
-              ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-              change(Foo, :baz).by(0)
-              ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-        expect { foo }
-          .to change { Foo.bar }.by(0) &
-              ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-              change { Foo.baz }.by(0)
-              ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-        expect { foo }
-          .to change(Foo, :bar).by(0)
-              ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-          .or change(Foo, :baz).by(0)
-              ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-        expect { foo }
-          .to change { Foo.bar }.by(0)
-              ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-          .or change { Foo.baz }.by(0)
-              ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-        expect { foo }
-          .to change(Foo, :bar).by(0) |
-              ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-              change(Foo, :baz).by(0)
-              ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-        expect { foo }
-          .to change { Foo.bar }.by(0) |
-              ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-              change { Foo.baz }.by(0)
-              ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
-      end
-    RUBY
+  context 'when `NegatedMatcher` is not defined (default)' do
+    it 'registers an offense when the argument to `by` is zero ' \
+      'with compound expectations by `and`' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { foo }
+            .to change(Foo, :bar).by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+            .and change(Foo, :baz).by(0)
+                 ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+          expect { foo }
+            .to change { Foo.bar }.by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+            .and change { Foo.baz }.by(0)
+                 ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+        end
+      RUBY
+
+      expect_no_corrections
+    end
+
+    it 'registers an offense when the argument to `by` is zero ' \
+      'with compound expectations by `&`' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { foo }
+            .to change(Foo, :bar).by(0) &
+                ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+                change(Foo, :baz).by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+          expect { foo }
+            .to change { Foo.bar }.by(0) &
+                ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+                change { Foo.baz }.by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+        end
+      RUBY
+
+      expect_no_corrections
+    end
+
+    it 'registers an offense when the argument to `by` is zero ' \
+      'with compound expectations by `or`' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { foo }
+            .to change(Foo, :bar).by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+            .or change(Foo, :baz).by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+          expect { foo }
+            .to change { Foo.bar }.by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+            .or change { Foo.baz }.by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+        end
+      RUBY
+
+      expect_no_corrections
+    end
+
+    it 'registers an offense when the argument to `by` is zero ' \
+      'with compound expectations by `|`' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { foo }
+            .to change(Foo, :bar).by(0) |
+                ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+                change(Foo, :baz).by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+          expect { foo }
+            .to change { Foo.bar }.by(0) |
+                ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+                change { Foo.baz }.by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`.
+        end
+      RUBY
+
+      expect_no_corrections
+    end
+  end
+
+  context "with `NegatedMatcher: 'not_change'`" do
+    let(:cop_config) { { 'NegatedMatcher' => 'not_change' } }
+
+    it 'registers an offense and autocorrect when ' \
+      'the argument to `by` is zero with compound expectations' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { foo }
+            .to change(Foo, :bar).by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+            .and change(Foo, :baz).by(0)
+                 ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+          expect { foo }
+            .to change { Foo.bar }.by(0)
+                ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+            .and change { Foo.baz }.by(0)
+                 ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`.
+        end
+      RUBY
 
-    expect_no_corrections
+      expect_correction(<<-RUBY)
+        it do
+          expect { foo }
+            .to not_change(Foo, :bar)
+            .and not_change(Foo, :baz)
+          expect { foo }
+            .to not_change { Foo.bar }
+            .and not_change { Foo.baz }
+        end
+      RUBY
+    end
   end
 
   it 'does not register an offense when the argument to `by` is not zero' do