Improve RSpec/ExpectChange

Co-authored-by: Yusuke Iwaki <[email protected]>
Co-authored-by: Phil Pirozhkov <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * Drop Ruby 2.5 support. ([@ydah][])
 * Add new `RSpec/ChangeByZero` cop. ([@ydah][])
+* Improve `RSpec/ExpectChange` to detect namespaced and top-level constants. ([@M-Yamashita01][])
 
 ## 2.10.0 (2022-04-19)
 
@@ -684,3 +685,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
 [@oshiro3]: https://github.com/oshiro3
 [@ydah]: https://github.com/ydah
 [@t3h2mas]: https://github.com/t3h2mas
+[@M-Yamashita01]: https://github.com/M-Yamashita01

lib/rubocop/cop/rspec/expect_change.rb

@@ -39,15 +39,15 @@ class ExpectChange < Base
 
         # @!method expect_change_with_arguments(node)
         def_node_matcher :expect_change_with_arguments, <<-PATTERN
-          (send nil? :change $_ (sym $_))
+          (send nil? :change $_ ({sym str} $_))
         PATTERN
 
         # @!method expect_change_with_block(node)
         def_node_matcher :expect_change_with_block, <<-PATTERN
           (block
             (send nil? :change)
             (args)
-            (send ({const send} nil? $_) $_)
+            (send $_ $_)
           )
         PATTERN
 
@@ -67,9 +67,9 @@ def on_block(node)
           return unless style == :method_call
 
           expect_change_with_block(node) do |receiver, message|
-            msg = format(MSG_BLOCK, obj: receiver, attr: message)
+            msg = format(MSG_BLOCK, 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

@@ -23,15 +23,15 @@
       RUBY
     end
 
-    it 'ignores blocks that cannot be converted to obj/attribute pair' do
+    it 'ignores blocks when the method is called with arguments' do
       expect_no_offenses(<<-RUBY)
         it do
           expect { run }.to change { User.sum(:points) }
         end
       RUBY
     end
 
-    it 'ignores change method of object that happens to receive a block' do
+    it 'ignores when not an expectation' do
       expect_no_offenses(<<-RUBY)
         it do
           Record.change { User.count }
@@ -47,6 +47,53 @@
         end
       RUBY
     end
+
+    it 'ignores the usage that adheres to the enforced style' do
+      expect_no_offenses(<<-RUBY)
+        it do
+          expect { run }.to change(User, :count).by(1)
+        end
+      RUBY
+    end
+
+    it 'flags when the received is a namespaced constant' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { run }.to change { User::Token::Auth.count }.by(1)
+                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `change(User::Token::Auth, :count)`.
+        end
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it do
+          expect { run }.to change(User::Token::Auth, :count).by(1)
+        end
+      RUBY
+    end
+
+    it 'flags when the receiver is a top-level constant' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { run }.to change { ::User.count }.by(1)
+                            ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `change(::User, :count)`.
+        end
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it do
+          expect { run }.to change(::User, :count).by(1)
+        end
+      RUBY
+    end
+
+    it 'flags chained method calls' do
+      expect_offense(<<-RUBY)
+        it do
+          expect { file.upload! }.to change { user.uploads.count }.by(1)
+                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `change(user.uploads, :count)`.
+        end
+      RUBY
+    end
   end
 
   context 'with EnforcedStyle `block`' do
@@ -67,41 +114,41 @@
       RUBY
     end
 
-    it 'flags change matcher when receiver is a constant' do
+    it 'flags change matcher when receiver is a namespaced constant' do
       expect_offense(<<-RUBY)
         it do
-          expect { run }.to change(User, :count)
-                            ^^^^^^^^^^^^^^^^^^^^ Prefer `change { User.count }`.
+          expect { run }.to change(User::Token::Auth, :count).by(1)
+                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { User::Token::Auth.count }`.
         end
       RUBY
 
       expect_correction(<<-RUBY)
         it do
-          expect { run }.to change { User.count }
+          expect { run }.to change { User::Token::Auth.count }.by(1)
         end
       RUBY
     end
 
-    it 'flags change matcher when receiver is a top-level constant' do
+    it 'flags change matcher when receiver is a variable' do
       expect_offense(<<-RUBY)
         it do
-          expect { run }.to change(::User, :count)
-                            ^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { ::User.count }`.
+          expect { run }.to change(user, :count)
+                            ^^^^^^^^^^^^^^^^^^^^ Prefer `change { user.count }`.
         end
       RUBY
 
       expect_correction(<<-RUBY)
         it do
-          expect { run }.to change { ::User.count }
+          expect { run }.to change { user.count }
         end
       RUBY
     end
 
-    it 'flags change matcher when receiver is a variable' do
+    it 'flags change matcher when message is a string' do
       expect_offense(<<-RUBY)
         it do
-          expect { run }.to change(user, :status)
-                            ^^^^^^^^^^^^^^^^^^^^^ Prefer `change { user.status }`.
+          expect { run }.to change(user, 'status')
+                            ^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { user.status }`.
         end
       RUBY
 
@@ -127,6 +174,21 @@
       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 'registers an offense for change matcher with an instance variable' do
       expect_offense(<<-RUBY)
         it do