diff --git a/CHANGELOG.md b/CHANGELOG.md index d094bb270..18046b88d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Fix a false positive in `RSpec/IndexedLet` with suffixes after index-like numbers. ([@pirj]) - Fix an error for `RSpec/Rails/HaveHttpStatus` with comparison with strings containing non-numeric characters. ([@ydah]) - Fix an error for `RSpec/MatchArray` when `match_array` with no argument. ([@ydah]) +- Add support `a_block_changing` and `changing` for `RSpec/ChangeByZero`. ([@ydah]) ## 2.20.0 (2023-04-18) diff --git a/lib/rubocop/cop/rspec/change_by_zero.rb b/lib/rubocop/cop/rspec/change_by_zero.rb index bcbff02e3..232e0bbc2 100644 --- a/lib/rubocop/cop/rspec/change_by_zero.rb +++ b/lib/rubocop/cop/rspec/change_by_zero.rb @@ -59,15 +59,16 @@ module RSpec # class ChangeByZero < Base extend AutoCorrector - MSG = 'Prefer `not_to change` over `to change.by(0)`.' + MSG = 'Prefer `not_to change` over `to %s.by(0)`.' MSG_COMPOUND = 'Prefer %s with compound expectations ' \ - 'over `change.by(0)`.' - RESTRICT_ON_SEND = %i[change].freeze + 'over `%s.by(0)`.' + CHANGE_METHODS = Set[:change, :a_block_changing, :changing].freeze + RESTRICT_ON_SEND = CHANGE_METHODS.freeze # @!method expect_change_with_arguments(node) def_node_matcher :expect_change_with_arguments, <<-PATTERN (send - (send nil? :change ...) :by + $(send nil? CHANGE_METHODS ...) :by (int 0)) PATTERN @@ -75,7 +76,7 @@ class ChangeByZero < Base def_node_matcher :expect_change_with_block, <<-PATTERN (send (block - (send nil? :change) + $(send nil? CHANGE_METHODS) (args) (send (...) _)) :by (int 0)) @@ -83,40 +84,53 @@ class ChangeByZero < Base # @!method change_nodes(node) def_node_search :change_nodes, <<-PATTERN - $(send nil? :change ...) + $(send nil? CHANGE_METHODS ...) PATTERN def on_send(node) - expect_change_with_arguments(node.parent) do - check_offense(node.parent) + expect_change_with_arguments(node.parent) do |change| + register_offense(node.parent, change) end - expect_change_with_block(node.parent.parent) do - check_offense(node.parent.parent) + expect_change_with_block(node.parent.parent) do |change| + register_offense(node.parent.parent, change) end end private - def check_offense(node) - expression = node.source_range + # rubocop:disable Metrics/MethodLength + def register_offense(node, change_node) if compound_expectations?(node) - add_offense(expression, message: message_compound) do |corrector| + add_offense(node.source_range, + message: message_compound(change_node)) do |corrector| autocorrect_compound(corrector, node) end else - add_offense(expression) do |corrector| - autocorrect(corrector, node) + add_offense(node.source_range, + message: message(change_node)) do |corrector| + autocorrect(corrector, node, change_node) end end end + # rubocop:enable Metrics/MethodLength def compound_expectations?(node) %i[and or & |].include?(node.parent.method_name) end - def autocorrect(corrector, node) + def message(change_node) + format(MSG, method: change_node.method_name) + end + + def message_compound(change_node) + format(MSG_COMPOUND, preferred: preferred_method, + method: change_node.method_name) + end + + def autocorrect(corrector, node, change_node) corrector.replace(node.parent.loc.selector, 'not_to') + corrector.replace(change_node.loc.selector, 'change') range = node.loc.dot.with(end_pos: node.source_range.end_pos) corrector.remove(range) end @@ -135,10 +149,6 @@ 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 diff --git a/spec/rubocop/cop/rspec/change_by_zero_spec.rb b/spec/rubocop/cop/rspec/change_by_zero_spec.rb index d47ee9d66..3efad5838 100644 --- a/spec/rubocop/cop/rspec/change_by_zero_spec.rb +++ b/spec/rubocop/cop/rspec/change_by_zero_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::RSpec::ChangeByZero, :config do - it 'registers an offense when the argument to `by` is zero' do + it 'registers an offense when using `change` and argument to `by` is zero' do expect_offense(<<-RUBY) it do expect { foo }.to change(Foo, :bar).by(0) @@ -25,6 +25,38 @@ RUBY end + it 'registers an offense when using `a_block_changing` ' \ + 'and argument to `by` is zero' do + expect_offense(<<-RUBY) + it do + expect { foo }.to a_block_changing(Foo, :bar).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to a_block_changing.by(0)`. + end + RUBY + + expect_correction(<<-RUBY) + it do + expect { foo }.not_to change(Foo, :bar) + end + RUBY + end + + it 'registers an offense when using `changing` ' \ + 'and argument to `by` is zero' do + expect_offense(<<-RUBY) + it do + expect { foo }.to changing(Foo, :bar).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_to change` over `to changing.by(0)`. + end + RUBY + + expect_correction(<<-RUBY) + it do + expect { foo }.not_to change(Foo, :bar) + end + RUBY + end + 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