Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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])

Expand Down
3 changes: 2 additions & 1 deletion config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,8 @@ RSpec/ExpectChange:
- block
SafeAutoCorrect: false
VersionAdded: '1.22'
VersionChanged: '2.5'
VersionChanged: "<<next>>"
NegatedMatcher: ~
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ExpectChange

RSpec/ExpectInHook:
Expand Down
22 changes: 21 additions & 1 deletion docs/modules/ROOT/pages/cops_rspec.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -2141,7 +2141,7 @@ expect(false).to eq(true)
| Yes
| Always (Unsafe)
| 1.22
| 2.5
| <<next>>
|===

Checks for consistent style of change matcher.
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -2213,6 +2229,10 @@ expect { run }.to change { Foo.bar }
| EnforcedStyle
| `method_call`
| `method_call`, `block`

| NegatedMatcher
| `<none>`
|
|===

[#references-rspecexpectchange]
Expand Down
64 changes: 49 additions & 15 deletions lib/rubocop/cop/rspec/expect_change.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(%<obj>s, :%<attr>s)`.'
MSG_CALL = 'Prefer `change { %<obj>s.%<attr>s }`.'
RESTRICT_ON_SEND = %i[change].freeze
MSG_BLOCK = 'Prefer `%<matcher>s(%<obj>s, :%<attr>s)`.'
MSG_CALL = 'Prefer `%<matcher>s { %<obj>s.%<attr>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
${
Expand All @@ -78,27 +88,51 @@ 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
end

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
Expand Down
122 changes: 122 additions & 0 deletions spec/rubocop/cop/rspec/expect_change_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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