Merge pull request #1265 from ydah/add_change_by_zero

Add new `RSpec/ChangeByZero` cop

Author: pirj

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Master (Unreleased)
 
 * Drop Ruby 2.5 support. ([@ydah][])
+* Add new `RSpec/ChangeByZero` cop. ([@ydah][])
 
 ## 2.10.0 (2022-04-19)
 

config/default.yml

@@ -176,6 +176,12 @@ RSpec/BeforeAfterAll:
   StyleGuide: https://rspec.rubystyle.guide/#avoid-hooks-with-context-scope
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/BeforeAfterAll
 
+RSpec/ChangeByZero:
+  Description: Prefer negated matchers over `to change.by(0)`.
+  Enabled: pending
+  VersionAdded: 2.11.0
+  Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ChangeByZero
+
 RSpec/ContextMethod:
   Description: "`context` should not be used for specifying methods."
   Enabled: true

docs/modules/ROOT/pages/cops.adoc

@@ -11,6 +11,7 @@
 * xref:cops_rspec.adoc#rspecbeeql[RSpec/BeEql]
 * xref:cops_rspec.adoc#rspecbenil[RSpec/BeNil]
 * xref:cops_rspec.adoc#rspecbeforeafterall[RSpec/BeforeAfterAll]
+* xref:cops_rspec.adoc#rspecchangebyzero[RSpec/ChangeByZero]
 * xref:cops_rspec.adoc#rspeccontextmethod[RSpec/ContextMethod]
 * xref:cops_rspec.adoc#rspeccontextwording[RSpec/ContextWording]
 * xref:cops_rspec.adoc#rspecdescribeclass[RSpec/DescribeClass]

docs/modules/ROOT/pages/cops_rspec.adoc

@@ -383,6 +383,49 @@ end
 * https://rspec.rubystyle.guide/#avoid-hooks-with-context-scope
 * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/BeforeAfterAll
 
+== RSpec/ChangeByZero
+
+|===
+| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed
+
+| Pending
+| Yes
+| Yes
+| 2.11.0
+| -
+|===
+
+Prefer negated matchers over `to change.by(0)`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+expect { run }.to change(Foo, :bar).by(0)
+expect { run }.to change { Foo.bar }.by(0)
+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
+expect { run }.not_to change(Foo, :bar)
+expect { run }.not_to change { Foo.bar }
+expect { run }
+  .to not_change(Foo, :bar)
+  .and not_change(Foo, :baz)
+expect { run }
+  .to not_change { Foo.bar }
+  .and not_change { Foo.baz }
+----
+
+=== References
+
+* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ChangeByZero
+
 == RSpec/ContextMethod
 
 |===

lib/rubocop/cop/rspec/change_by_zero.rb

@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Prefer negated matchers over `to change.by(0)`.
+      #
+      # @example
+      #   # bad
+      #   expect { run }.to change(Foo, :bar).by(0)
+      #   expect { run }.to change { Foo.bar }.by(0)
+      #   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
+      #   expect { run }.not_to change(Foo, :bar)
+      #   expect { run }.not_to change { Foo.bar }
+      #   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 ' \
+                          'over `change.by(0)`.'
+        RESTRICT_ON_SEND = %i[change].freeze
+
+        # @!method expect_change_with_arguments(node)
+        def_node_matcher :expect_change_with_arguments, <<-PATTERN
+          (send
+            (send nil? :change ...) :by
+            (int 0))
+        PATTERN
+
+        # @!method expect_change_with_block(node)
+        def_node_matcher :expect_change_with_block, <<-PATTERN
+          (send
+            (block
+              (send nil? :change)
+              (args)
+              (send (...) $_)) :by
+            (int 0))
+        PATTERN
+
+        def on_send(node)
+          expect_change_with_arguments(node.parent) do
+            check_offence(node.parent)
+          end
+
+          expect_change_with_block(node.parent.parent) do
+            check_offence(node.parent.parent)
+          end
+        end
+
+        private
+
+        def check_offence(node)
+          expression = node.loc.expression
+          if compound_expectations?(node)
+            add_offense(expression, message: MSG_COMPOUND)
+          else
+            add_offense(expression) do |corrector|
+              autocorrect(corrector, node)
+            end
+          end
+        end
+
+        def compound_expectations?(node)
+          %i[and or].include?(node.parent.method_name)
+        end
+
+        def autocorrect(corrector, node)
+          corrector.replace(node.parent.loc.selector, 'not_to')
+          range = node.loc.dot.with(end_pos: node.loc.expression.end_pos)
+          corrector.remove(range)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rspec_cops.rb

@@ -25,6 +25,7 @@
 require_relative 'rspec/be_eql'
 require_relative 'rspec/be_nil'
 require_relative 'rspec/before_after_all'
+require_relative 'rspec/change_by_zero'
 require_relative 'rspec/context_method'
 require_relative 'rspec/context_wording'
 require_relative 'rspec/describe_class'

spec/rubocop/cop/rspec/change_by_zero_spec.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::RSpec::ChangeByZero do
+  it 'registers an offense when the argument to `by` is zero' do
+    expect_offense(<<-RUBY)
+      it do
+        expect { foo }.to change(Foo, :bar).by(0)
+                          ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
+        expect { foo }.to change(::Foo, :bar).by(0)
+                          ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
+        expect { foo }.to change { Foo.bar }.by(0)
+                          ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
+        expect { foo }.to change(Foo, :bar).by 0
+                          ^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to change.by(0)`.
+      end
+    RUBY
+
+    expect_correction(<<-RUBY)
+      it do
+        expect { foo }.not_to change(Foo, :bar)
+        expect { foo }.not_to change(::Foo, :bar)
+        expect { foo }.not_to change { Foo.bar }
+        expect { foo }.not_to change(Foo, :bar)
+      end
+    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)`.
+          .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
+  end
+
+  it 'does not register an offense when the argument to `by` is not zero' do
+    expect_no_offenses(<<-RUBY)
+      it do
+        expect { foo }.to change(Foo, :bar).by(1)
+        expect { foo }.to change { Foo.bar }.by(1)
+      end
+    RUBY
+  end
+end