diff --git a/CHANGELOG.md b/CHANGELOG.md index 60c7ee103..952ff9c05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Master (Unreleased) +- Add `NegatedMatcher` configuration option `RSpec/ExpectChange`. ([@Darhazer]) - `RSpec/ScatteredLet` now preserves the order of `let`s during auto-correction. ([@Darhazer]) - Fix a false negative for `RSpec/EmptyLineAfterFinalLet` inside `shared_examples` / `include_examples` / `it_behaves_like` blocks. ([@Darhazer]) diff --git a/config/default.yml b/config/default.yml index 351bfc735..2c16ed827 100644 --- a/config/default.yml +++ b/config/default.yml @@ -453,7 +453,8 @@ RSpec/ExpectChange: - block SafeAutoCorrect: false VersionAdded: '1.22' - VersionChanged: '2.5' + VersionChanged: "<>" + NegatedMatcher: ~ Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ExpectChange RSpec/ExpectInHook: diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index b9af833cf..9ba48bf00 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -2141,7 +2141,7 @@ expect(false).to eq(true) | Yes | Always (Unsafe) | 1.22 -| 2.5 +| <> |=== Checks for consistent style of change matcher. @@ -2151,6 +2151,10 @@ or a block. This cop can be configured using the `EnforcedStyle` option. +When using compound expectations with `change` and a negated matcher +(e.g., `not_change`), you can configure the `NegatedMatcher` option +to ensure consistent style enforcement across both matchers. + [#safety-rspecexpectchange] === Safety @@ -2204,6 +2208,18 @@ expect { run }.to change(Foo, :bar) expect { run }.to change { Foo.bar } ---- +[#_negatedmatcher_-not_change_-_with-compound-expectations_-rspecexpectchange] +==== `NegatedMatcher: not_change` (with compound expectations) + +[source,ruby] +---- +# bad +expect { run }.to change(Foo, :bar).and not_change { Foo.baz } + +# good +expect { run }.to change(Foo, :bar).and not_change(Foo, :baz) +---- + [#configurable-attributes-rspecexpectchange] === Configurable attributes @@ -2213,6 +2229,10 @@ expect { run }.to change { Foo.bar } | EnforcedStyle | `method_call` | `method_call`, `block` + +| NegatedMatcher +| `` +| |=== [#references-rspecexpectchange] diff --git a/lib/rubocop/cop/rspec/expect_change.rb b/lib/rubocop/cop/rspec/expect_change.rb index 12f339e2b..1e6f4d792 100644 --- a/lib/rubocop/cop/rspec/expect_change.rb +++ b/lib/rubocop/cop/rspec/expect_change.rb @@ -10,6 +10,10 @@ module RSpec # # This cop can be configured using the `EnforcedStyle` option. # + # When using compound expectations with `change` and a negated matcher + # (e.g., `not_change`), you can configure the `NegatedMatcher` option + # to ensure consistent style enforcement across both matchers. + # # @safety # Autocorrection is unsafe because `method_call` style calls the # receiver *once* and sends the message to it before and after @@ -48,23 +52,29 @@ module RSpec # # good # expect { run }.to change { Foo.bar } # + # @example `NegatedMatcher: not_change` (with compound expectations) + # # bad + # expect { run }.to change(Foo, :bar).and not_change { Foo.baz } + # + # # good + # expect { run }.to change(Foo, :bar).and not_change(Foo, :baz) + # class ExpectChange < Base extend AutoCorrector include ConfigurableEnforcedStyle - MSG_BLOCK = 'Prefer `change(%s, :%s)`.' - MSG_CALL = 'Prefer `change { %s.%s }`.' - RESTRICT_ON_SEND = %i[change].freeze + MSG_BLOCK = 'Prefer `%s(%s, :%s)`.' + MSG_CALL = 'Prefer `%s { %s.%s }`.' - # @!method expect_change_with_arguments(node) - def_node_matcher :expect_change_with_arguments, <<~PATTERN - (send nil? :change $_ ({sym str} $_)) + # @!method expect_matcher_with_arguments(node) + def_node_matcher :expect_matcher_with_arguments, <<~PATTERN + (send nil? _ $_ ({sym str} $_)) PATTERN - # @!method expect_change_with_block(node) - def_node_matcher :expect_change_with_block, <<~PATTERN + # @!method expect_matcher_with_block(node) + def_node_matcher :expect_matcher_with_block, <<~PATTERN (block - (send nil? :change) + (send nil? _) (args) (send ${ @@ -78,11 +88,14 @@ class ExpectChange < Base def on_send(node) return unless style == :block + return unless matcher_method?(node.method_name) - expect_change_with_arguments(node) do |receiver, message| - msg = format(MSG_CALL, obj: receiver.source, attr: message) + expect_matcher_with_arguments(node) do |receiver, message| + matcher_name = node.method_name.to_s + msg = format(MSG_CALL, matcher: matcher_name, + obj: receiver.source, attr: message) add_offense(node, message: msg) do |corrector| - replacement = "change { #{receiver.source}.#{message} }" + replacement = "#{matcher_name} { #{receiver.source}.#{message} }" corrector.replace(node, replacement) end end @@ -90,15 +103,36 @@ def on_send(node) def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler return unless style == :method_call + return unless matcher_method?(node.method_name) - expect_change_with_block(node) do |receiver, message| - msg = format(MSG_BLOCK, obj: receiver.source, attr: message) + expect_matcher_with_block(node) do |receiver, message| + matcher_name = node.method_name.to_s + msg = format(MSG_BLOCK, matcher: matcher_name, + obj: receiver.source, attr: message) add_offense(node, message: msg) do |corrector| - replacement = "change(#{receiver.source}, :#{message})" + replacement = "#{matcher_name}(#{receiver.source}, :#{message})" corrector.replace(node, replacement) end end end + + private + + def matcher_method_names + @matcher_method_names ||= begin + names = [:change] + names << negated_matcher.to_sym if negated_matcher + names + end + end + + def matcher_method?(method_name) + matcher_method_names.include?(method_name) + end + + def negated_matcher + cop_config['NegatedMatcher'] + end end end end diff --git a/spec/rubocop/cop/rspec/expect_change_spec.rb b/spec/rubocop/cop/rspec/expect_change_spec.rb index e26b54234..78722b509 100644 --- a/spec/rubocop/cop/rspec/expect_change_spec.rb +++ b/spec/rubocop/cop/rspec/expect_change_spec.rb @@ -270,4 +270,126 @@ RUBY end end + + context "with `NegatedMatcher: 'not_change'`" do + let(:cop_config) do + { 'EnforcedStyle' => enforced_style, 'NegatedMatcher' => 'not_change' } + end + + context 'with EnforcedStyle `method_call`' do + let(:enforced_style) { 'method_call' } + + it 'flags negated matcher with block style' do + expect_offense(<<~RUBY) + it do + expect { run }.to not_change { User.count } + ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change(User, :count)`. + end + RUBY + + expect_correction(<<~RUBY) + it do + expect { run }.to not_change(User, :count) + end + RUBY + end + + it 'flags negated matcher in compound expectations' do + expect_offense(<<~RUBY) + it do + expect { run }.to change(Foo, :bar).and not_change { Foo.baz } + ^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change(Foo, :baz)`. + end + RUBY + + expect_correction(<<~RUBY) + it do + expect { run }.to change(Foo, :bar).and not_change(Foo, :baz) + end + RUBY + end + + it 'flags both change and negated matcher in compound expectations' do + expect_offense(<<~RUBY) + it do + expect { run }.to change { Foo.bar }.and not_change { Foo.baz } + ^^^^^^^^^^^^^^^^^^ Prefer `change(Foo, :bar)`. + ^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change(Foo, :baz)`. + end + RUBY + + expect_correction(<<~RUBY) + it do + expect { run }.to change(Foo, :bar).and not_change(Foo, :baz) + end + RUBY + end + + it 'ignores when both use method_call style' do + expect_no_offenses(<<~RUBY) + it do + expect { run }.to change(Foo, :bar).and not_change(Foo, :baz) + end + RUBY + end + end + + context 'with EnforcedStyle `block`' do + let(:enforced_style) { 'block' } + + it 'flags negated matcher with method call style' do + expect_offense(<<~RUBY) + it do + expect { run }.to not_change(User, :count) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change { User.count }`. + end + RUBY + + expect_correction(<<~RUBY) + it do + expect { run }.to not_change { User.count } + end + RUBY + end + + it 'flags negated matcher in compound expectations' do + expect_offense(<<~RUBY) + it do + expect { run }.to change { Foo.bar }.and not_change(Foo, :baz) + ^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change { Foo.baz }`. + end + RUBY + + expect_correction(<<~RUBY) + it do + expect { run }.to change { Foo.bar }.and not_change { Foo.baz } + end + RUBY + end + + it 'flags both change and negated matcher in compound expectations' do + expect_offense(<<~RUBY) + it do + expect { run }.to change(Foo, :bar).and not_change(Foo, :baz) + ^^^^^^^^^^^^^^^^^ Prefer `change { Foo.bar }`. + ^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change { Foo.baz }`. + end + RUBY + + expect_correction(<<~RUBY) + it do + expect { run }.to change { Foo.bar }.and not_change { Foo.baz } + end + RUBY + end + + it 'ignores when both use block style' do + expect_no_offenses(<<~RUBY) + it do + expect { run }.to change { Foo.bar }.and not_change { Foo.baz } + end + RUBY + end + end + end end