[Fix #1396] Add new cop assert_changes_second_argument

Author: joseph-carino

changelog/new_assert_changes_second_argument_cop.md

@@ -0,0 +1 @@
+* [#1396](https://github.com/rubocop/rubocop-rails/issues/1396): Add cop to prevent use of assert_changes with non-string second arguments. ([@joseph-carino][])

config/default.yml

@@ -222,6 +222,13 @@ Rails/ArelStar:
   SafeAutoCorrect: false
   VersionAdded: '2.9'
 
+Rails/AssertChangesSecondArgument:
+  Description: 'Do not use `assert_changes` with a non-string second argument.'
+  Enabled: true
+  VersionAdded: '2.28'
+  Include:
+    - '**/test/**/*'
+
 Rails/AssertNot:
   Description: 'Use `assert_not` instead of `assert !`.'
   Enabled: true

lib/rubocop/cop/rails/assert_changes_second_argument.rb

@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # Checks misuse of the second argument to ActiveSupport `assert_changes` method
+      #
+      # `assert_changes`'s second argument is the failure message emitted when the
+      # first argument (expression) is unchanged in the block.
+      #
+      # A common mistake is to use `assert_changes` with the expected change
+      # value delta as the second argument.
+      # In that case `assert_changes` will validate only that the expression changes,
+      # not that it changes by a specific value.
+      #
+      # Users should provide the 'from' and 'to' parameters,
+      # or use `assert_difference` instead, which does support deltas.
+      #
+      # @example
+      #
+      #   # bad
+      #   assert_changes -> { @value }, 1 do
+      #     @value += 1
+      #   end
+      #
+      #   # good
+      #   assert_changes -> { @value }, from: 0, to: 1 do
+      #     @value += 1
+      #   end
+      #   assert_changes -> { @value }, "Value should change" do
+      #     @value += 1
+      #   end
+      #   assert_difference -> { @value }, 1 do
+      #     @value += 1
+      #   end
+      #
+      class AssertChangesSecondArgument < Base
+        extend AutoCorrector
+
+        MSG = 'Use assert_changes to assert when an expression changes from and to specific values. ' \
+              'Use assert_difference instead to assert when an expression changes by a specific delta. ' \
+              'The second argument to assert_changes is the message emitted on assert failure.'
+
+        def on_send(node)
+          return if node.receiver || !node.method?(:assert_changes)
+          return if node.arguments[1].nil?
+
+          return unless offense?(node.arguments[1])
+
+          add_offense(node.loc.selector) do |corrector|
+            corrector.replace(node.loc.selector, 'assert_difference')
+          end
+        end
+
+        private
+
+        def offense?(arg)
+          !arg.hash_type? && !arg.str_type? && !arg.dstr_type? && !arg.sym_type? && !arg.dsym_type? && !arg.variable?
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -25,6 +25,7 @@
 require_relative 'rails/application_mailer'
 require_relative 'rails/application_record'
 require_relative 'rails/arel_star'
+require_relative 'rails/assert_changes_second_argument'
 require_relative 'rails/assert_not'
 require_relative 'rails/attribute_default_block_value'
 require_relative 'rails/belongs_to'

spec/rubocop/cop/rails/assert_changes_second_argument_spec.rb

@@ -0,0 +1,104 @@
+# frozen_string_literal: true
+
+RSpec.describe(RuboCop::Cop::Rails::AssertChangesSecondArgument, :config) do
+  let(:message) do
+    'Use assert_changes to assert when an expression changes from and to specific values. ' \
+      'Use assert_difference instead to assert when an expression changes by a specific delta. ' \
+      'The second argument to assert_changes is the message emitted on assert failure.'
+  end
+
+  describe('offenses') do
+    it 'adds offense when the second positional argument is an integer' do
+      expect_offense(<<~RUBY)
+        assert_changes @value, -1 do
+        ^^^^^^^^^^^^^^ #{message}
+          @value += 1
+        end
+      RUBY
+    end
+
+    it 'adds offense when the second positional argument is a float' do
+      expect_offense(<<~RUBY)
+        assert_changes @value, -1.0 do
+        ^^^^^^^^^^^^^^ #{message}
+          @value += 1
+        end
+      RUBY
+    end
+
+    it 'does not add offense when the second argument is a string' do
+      expect_no_offenses(<<~RUBY)
+        assert_changes @value, "Value should change" do
+          @value += 1
+        end
+      RUBY
+    end
+
+    it 'does not add offense when the second argument is an interpolated string' do
+      expect_no_offenses(<<~RUBY)
+        assert_changes @value, "\#{thing} should change" do
+          @value += 1
+        end
+      RUBY
+    end
+
+    it 'does not add offense when the second argument is a symbol' do
+      expect_no_offenses(<<~RUBY)
+        assert_changes @value, :should_change do
+          @value += 1
+        end
+      RUBY
+    end
+
+    it 'does not add offense when the second argument is an interpolated symbol' do
+      expect_no_offenses(<<~RUBY)
+        assert_changes @value, :"\#{thing}_should_change" do
+          @value += 1
+        end
+      RUBY
+    end
+
+    it 'does not add offense when the second argument is a variable' do
+      expect_no_offenses(<<~RUBY)
+        message = "Value should change"
+        assert_changes @value, message do
+          @value += 1
+        end
+      RUBY
+    end
+
+    it 'does not add offense when there is only one argument' do
+      expect_no_offenses(<<~RUBY)
+        assert_changes @value do
+          @value += 1
+        end
+      RUBY
+    end
+
+    it 'does not add offense when there is only one positional argument' do
+      expect_no_offenses(<<~RUBY)
+        assert_changes @value, from: 0 do
+          @value += 1
+        end
+      RUBY
+    end
+  end
+
+  describe('autocorrect') do
+    it 'autocorrects method from assert_changes to assert_difference' do
+      source = <<-RUBY
+        assert_changes @value, -1.0 do
+          @value += 1
+        end
+      RUBY
+
+      corrected_source = <<-RUBY
+        assert_difference @value, -1.0 do
+          @value += 1
+        end
+      RUBY
+
+      expect(autocorrect_source(source)).to(eq(corrected_source))
+    end
+  end
+end