Merge pull request #1152 from tejasbubane/fix-1143

Fix false negative in `RSpec/ExpectChange` cop with block style and chained method call

Author: pirj

CHANGELOG.md

@@ -6,6 +6,7 @@
 * Fix an exception in `DescribedClass` when accessing a constant on a variable in a spec that is nested in a namespace. ([@rrosenblum][])
 * Add new `RSpec/IdenticalEqualityAssertion` cop. ([@tejasbubane][])
 * Add `RSpec/Rails/AvoidSetupHook cop. ([@paydaylight][])
+* Fix false negative in `RSpec/ExpectChange` cop with block style and chained method call. ([@tejasbubane][])
 
 ## 2.3.0 (2021-04-28)
 

lib/rubocop/cop/rspec/expect_change.rb

@@ -39,7 +39,7 @@ class ExpectChange < Base
 
         # @!method expect_change_with_arguments(node)
         def_node_matcher :expect_change_with_arguments, <<-PATTERN
-          (send nil? :change ({const send} nil? $_) (sym $_))
+          (send nil? :change $_ (sym $_))
         PATTERN
 
         # @!method expect_change_with_block(node)
@@ -55,9 +55,9 @@ def on_send(node)
           return unless style == :block
 
           expect_change_with_arguments(node) do |receiver, message|
-            msg = format(MSG_CALL, obj: receiver, attr: message)
+            msg = format(MSG_CALL, obj: receiver.source, attr: message)
             add_offense(node, message: msg) do |corrector|
-              replacement = "change { #{receiver}.#{message} }"
+              replacement = "change { #{receiver.source}.#{message} }"
               corrector.replace(node, replacement)
             end
           end

spec/rubocop/cop/rspec/expect_change_spec.rb

@@ -67,7 +67,7 @@
       RUBY
     end
 
-    it 'flags change matcher when receiver is a variable' do
+    it 'flags change matcher when receiver is a constant' do
       expect_offense(<<-RUBY)
         it do
           expect { run }.to change(User, :count)
@@ -82,6 +82,81 @@
       RUBY
     end
 
+    it 'flags change matcher when receiver is a top-level constant' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { run }.to change(::User, :count)
+                            ^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { ::User.count }`.
+        end
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it do
+          expect { run }.to change { ::User.count }
+        end
+      RUBY
+    end
+
+    it 'flags change matcher when receiver is a variable' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { run }.to change(user, :status)
+                            ^^^^^^^^^^^^^^^^^^^^^ Prefer `change { user.status }`.
+        end
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it do
+          expect { run }.to change { user.status }
+        end
+      RUBY
+    end
+
+    it 'registers an offense for change matcher with chained method call' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { paint_users! }.to change(users.green, :count).by(1)
+                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { users.green.count }`.
+        end
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it do
+          expect { paint_users! }.to change { users.green.count }.by(1)
+        end
+      RUBY
+    end
+
+    it 'registers an offense for change matcher with an instance variable' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { paint_users! }.to change(@food, :taste).to(:sour)
+                                     ^^^^^^^^^^^^^^^^^^^^^ Prefer `change { @food.taste }`.
+        end
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it do
+          expect { paint_users! }.to change { @food.taste }.to(:sour)
+        end
+      RUBY
+    end
+
+    it 'registers an offense for change matcher with a global variable' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { paint_users! }.to change($token, :value).to(nil)
+                                     ^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { $token.value }`.
+        end
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it do
+          expect { paint_users! }.to change { $token.value }.to(nil)
+        end
+      RUBY
+    end
+
     it 'ignores methods called change' do
       expect_no_offenses(<<-RUBY)
         it do