Skip to content

Commit ae49a4e

Browse files
committed
Add new RSpec/ChangeWithoutExpect cop
Follow up: #2071
1 parent 14f3dd2 commit ae49a4e

File tree

5 files changed

+234
-0
lines changed

5 files changed

+234
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
- Fix issue when `Style/ContextWording` is configured with a Prefix being interpreted as a boolean, like `on`. ([@sakuro])
77
- Add new `RSpec/IncludeExamples` cop to enforce using `it_behaves_like` over `include_examples`. ([@dvandersluis])
88
- Change `RSpec/ScatteredSetup` to allow `around` hooks to be scattered. ([@ydah])
9+
- Add new `RSpec/ChangeWithoutExpect` cop. ([@ydah])
910

1011
## 3.5.0 (2025-02-16)
1112

config/default.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,12 @@ RSpec/ChangeByZero:
199199
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ChangeByZero
200200
NegatedMatcher: ~
201201

202+
RSpec/ChangeWithoutExpect:
203+
Description: Checks for `change` matcher usage without an `expect` block.
204+
Enabled: pending
205+
VersionAdded: "<<next>>"
206+
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ChangeWithoutExpect
207+
202208
RSpec/ClassCheck:
203209
Description: Enforces consistent use of `be_a` or `be_kind_of`.
204210
StyleGuide: "#is-a-vs-kind-of"
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module RSpec
6+
# Checks for `change` matcher usage without an `expect` block.
7+
#
8+
# This cop only considered a matcher if it is chained with
9+
# `from`, `by`, `by_at_least`, or `by_at_most`.
10+
#
11+
# @example
12+
# # bad
13+
# it 'changes the count' do
14+
# change(Counter, :count).by(1)
15+
# end
16+
#
17+
# # bad
18+
# it 'changes the count' do
19+
# change(Counter, :count).by_at_least(1)
20+
# end
21+
#
22+
# # bad
23+
# it 'changes the count' do
24+
# change(Counter, :count).by_at_most(1)
25+
# end
26+
#
27+
# # bad
28+
# it 'changes the count' do
29+
# change(Counter, :count).from(0)
30+
# end
31+
#
32+
# # bad
33+
# it 'changes the count' do
34+
# change(Counter, :count).from(0).to(1)
35+
# end
36+
#
37+
# # good - no chained matchers
38+
# it 'changes the count' do
39+
# change(Counter, :count)
40+
# end
41+
#
42+
# # good - ignore not change matcher
43+
# it 'changes the count' do
44+
# change(Counter, :count).from(0).by(1)
45+
# end
46+
#
47+
# # good - within expect block
48+
# it 'changes the count' do
49+
# expect { subject }.to change(Counter, :count).by(1)
50+
# end
51+
#
52+
class ChangeWithoutExpect < RuboCop::Cop::Base
53+
include RangeHelp
54+
55+
MSG = 'Use `change` matcher within an `expect` block.'
56+
RESTRICT_ON_SEND = [:change].freeze
57+
SINGLE_RESTRICTED_METHODS = %i[by by_at_least by_at_most from].freeze
58+
59+
# @!method expectation?(node)
60+
def_node_search :expectation?, <<~PATTERN
61+
(send
62+
{
63+
(block (send nil? :expect ...) ...)
64+
(send nil? :expect ...)
65+
}
66+
{:to :not_to}
67+
...)
68+
PATTERN
69+
70+
def on_send(node)
71+
return if within_expectation?(node)
72+
return unless offensive_chain?(node)
73+
74+
add_offense(node)
75+
end
76+
77+
private
78+
79+
def offensive_chain?(node)
80+
chain_methods = chain_methods(node)
81+
return false if chain_methods.empty?
82+
83+
# Case 1: single restricted method (by, by_at_least, by_at_most, from)
84+
if chain_methods.size == 1 &&
85+
SINGLE_RESTRICTED_METHODS.include?(chain_methods.first)
86+
return true
87+
end
88+
89+
# Case 2: exactly from().to() pattern
90+
if chain_methods.size == 2 &&
91+
chain_methods.first == :from &&
92+
chain_methods.last == :to
93+
return true
94+
end
95+
96+
false
97+
end
98+
99+
def chain_methods(node)
100+
methods = []
101+
chain = node
102+
103+
while chain.send_type? && chain.parent&.send_type?
104+
chain = chain.parent
105+
methods << chain.method_name
106+
end
107+
108+
methods
109+
end
110+
111+
def within_expectation?(node)
112+
node.each_ancestor(:send).any? do |ancestor|
113+
expectation?(ancestor)
114+
end
115+
end
116+
end
117+
end
118+
end
119+
end

lib/rubocop/cop/rspec_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
require_relative 'rspec/be_nil'
1212
require_relative 'rspec/before_after_all'
1313
require_relative 'rspec/change_by_zero'
14+
require_relative 'rspec/change_without_expect'
1415
require_relative 'rspec/class_check'
1516
require_relative 'rspec/contain_exactly'
1617
require_relative 'rspec/context_method'
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::RSpec::ChangeWithoutExpect, :config do
4+
it 'registers an offense for a `change` call with only `by` ' \
5+
'without an `expect` block' do
6+
expect_offense(<<~RUBY)
7+
it 'changes the count' do
8+
change(Counter, :count).by(1)
9+
^^^^^^^^^^^^^^^^^^^^^^^ Use `change` matcher within an `expect` block.
10+
end
11+
RUBY
12+
end
13+
14+
it 'registers an offense for a `change` call with only `by_at_least` ' \
15+
'without an `expect` block' do
16+
expect_offense(<<~RUBY)
17+
it 'changes the count' do
18+
change(Counter, :count).by_at_least(1)
19+
^^^^^^^^^^^^^^^^^^^^^^^ Use `change` matcher within an `expect` block.
20+
end
21+
RUBY
22+
end
23+
24+
it 'registers an offense for a `change` call with only `by_at_most` ' \
25+
'without an `expect` block' do
26+
expect_offense(<<~RUBY)
27+
it 'changes the count' do
28+
change(Counter, :count).by_at_most(1)
29+
^^^^^^^^^^^^^^^^^^^^^^^ Use `change` matcher within an `expect` block.
30+
end
31+
RUBY
32+
end
33+
34+
it 'registers an offense for a `change` call with only `from` ' \
35+
'without an `expect` block' do
36+
expect_offense(<<~RUBY)
37+
it 'changes the count' do
38+
change(Counter, :count).from(0)
39+
^^^^^^^^^^^^^^^^^^^^^^^ Use `change` matcher within an `expect` block.
40+
end
41+
RUBY
42+
end
43+
44+
it 'registers an offense for a `change` call with exactly `from().to()` ' \
45+
'without an `expect` block' do
46+
expect_offense(<<~RUBY)
47+
it 'changes the count' do
48+
change(Counter, :count).from(0).to(1)
49+
^^^^^^^^^^^^^^^^^^^^^^^ Use `change` matcher within an `expect` block.
50+
end
51+
RUBY
52+
end
53+
54+
it 'does not register an offense for a `change` call with `from().by()` ' \
55+
'without an `expect` block' do
56+
expect_no_offenses(<<~RUBY)
57+
it 'changes the count' do
58+
change(Counter, :count).from(0).by(1)
59+
end
60+
RUBY
61+
end
62+
63+
it 'does not register an offense for a simple `change` call ' \
64+
'without chains' do
65+
expect_no_offenses(<<~RUBY)
66+
it 'changes the count' do
67+
change(Counter, :count)
68+
end
69+
RUBY
70+
end
71+
72+
it 'does not register an offense for a `change` call with unsupported ' \
73+
'chain method' do
74+
expect_no_offenses(<<~RUBY)
75+
it 'changes the count' do
76+
change(Counter, :count).some_other_method(123)
77+
end
78+
RUBY
79+
end
80+
81+
it 'does not register an offense for `change` with chains within an ' \
82+
'`expect` block' do
83+
expect_no_offenses(<<~RUBY)
84+
it 'changes the count' do
85+
expect { subject }.to change(Counter, :count).by(1)
86+
end
87+
RUBY
88+
end
89+
90+
it 'does not register an offense for `change` with from-to chain ' \
91+
'within an `expect` block' do
92+
expect_no_offenses(<<~RUBY)
93+
it 'changes the count' do
94+
expect { subject }.to change(Counter, :count).from(0).to(1)
95+
end
96+
RUBY
97+
end
98+
99+
it 'does not register an offense for `change` with chains ' \
100+
'within an `expect` with not_to' do
101+
expect_no_offenses(<<~RUBY)
102+
it 'does not change the count' do
103+
expect { subject }.not_to change(Counter, :count).by(1)
104+
end
105+
RUBY
106+
end
107+
end

0 commit comments

Comments
 (0)