Skip to content

Commit af2fd8a

Browse files
authored
Merge pull request #1042 from M-Yamashita01/feature-work-well-with-nested-class
Fixes RSpec/ExpectChange to work with nested class names.
2 parents eb0add8 + 049ac09 commit af2fd8a

File tree

3 files changed

+81
-17
lines changed

3 files changed

+81
-17
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
* Drop Ruby 2.5 support. ([@ydah][])
66
* Add new `RSpec/ChangeByZero` cop. ([@ydah][])
7+
* Improve `RSpec/ExpectChange` to detect namespaced and top-level constants. ([@M-Yamashita01][])
78

89
## 2.10.0 (2022-04-19)
910

@@ -684,3 +685,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
684685
[@oshiro3]: https://github.com/oshiro3
685686
[@ydah]: https://github.com/ydah
686687
[@t3h2mas]: https://github.com/t3h2mas
688+
[@M-Yamashita01]: https://github.com/M-Yamashita01

lib/rubocop/cop/rspec/expect_change.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,15 @@ class ExpectChange < Base
3939

4040
# @!method expect_change_with_arguments(node)
4141
def_node_matcher :expect_change_with_arguments, <<-PATTERN
42-
(send nil? :change $_ (sym $_))
42+
(send nil? :change $_ ({sym str} $_))
4343
PATTERN
4444

4545
# @!method expect_change_with_block(node)
4646
def_node_matcher :expect_change_with_block, <<-PATTERN
4747
(block
4848
(send nil? :change)
4949
(args)
50-
(send ({const send} nil? $_) $_)
50+
(send $_ $_)
5151
)
5252
PATTERN
5353

@@ -67,9 +67,9 @@ def on_block(node)
6767
return unless style == :method_call
6868

6969
expect_change_with_block(node) do |receiver, message|
70-
msg = format(MSG_BLOCK, obj: receiver, attr: message)
70+
msg = format(MSG_BLOCK, obj: receiver.source, attr: message)
7171
add_offense(node, message: msg) do |corrector|
72-
replacement = "change(#{receiver}, :#{message})"
72+
replacement = "change(#{receiver.source}, :#{message})"
7373
corrector.replace(node, replacement)
7474
end
7575
end

spec/rubocop/cop/rspec/expect_change_spec.rb

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,15 @@
2323
RUBY
2424
end
2525

26-
it 'ignores blocks that cannot be converted to obj/attribute pair' do
26+
it 'ignores blocks when the method is called with arguments' do
2727
expect_no_offenses(<<-RUBY)
2828
it do
2929
expect { run }.to change { User.sum(:points) }
3030
end
3131
RUBY
3232
end
3333

34-
it 'ignores change method of object that happens to receive a block' do
34+
it 'ignores when not an expectation' do
3535
expect_no_offenses(<<-RUBY)
3636
it do
3737
Record.change { User.count }
@@ -47,6 +47,53 @@
4747
end
4848
RUBY
4949
end
50+
51+
it 'ignores the usage that adheres to the enforced style' do
52+
expect_no_offenses(<<-RUBY)
53+
it do
54+
expect { run }.to change(User, :count).by(1)
55+
end
56+
RUBY
57+
end
58+
59+
it 'flags when the received is a namespaced constant' do
60+
expect_offense(<<-RUBY)
61+
it do
62+
expect { run }.to change { User::Token::Auth.count }.by(1)
63+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `change(User::Token::Auth, :count)`.
64+
end
65+
RUBY
66+
67+
expect_correction(<<-RUBY)
68+
it do
69+
expect { run }.to change(User::Token::Auth, :count).by(1)
70+
end
71+
RUBY
72+
end
73+
74+
it 'flags when the receiver is a top-level constant' do
75+
expect_offense(<<-RUBY)
76+
it do
77+
expect { run }.to change { ::User.count }.by(1)
78+
^^^^^^^^^^^^^^^^^^^^^^^ Prefer `change(::User, :count)`.
79+
end
80+
RUBY
81+
82+
expect_correction(<<-RUBY)
83+
it do
84+
expect { run }.to change(::User, :count).by(1)
85+
end
86+
RUBY
87+
end
88+
89+
it 'flags chained method calls' do
90+
expect_offense(<<-RUBY)
91+
it do
92+
expect { file.upload! }.to change { user.uploads.count }.by(1)
93+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `change(user.uploads, :count)`.
94+
end
95+
RUBY
96+
end
5097
end
5198

5299
context 'with EnforcedStyle `block`' do
@@ -67,41 +114,41 @@
67114
RUBY
68115
end
69116

70-
it 'flags change matcher when receiver is a constant' do
117+
it 'flags change matcher when receiver is a namespaced constant' do
71118
expect_offense(<<-RUBY)
72119
it do
73-
expect { run }.to change(User, :count)
74-
^^^^^^^^^^^^^^^^^^^^ Prefer `change { User.count }`.
120+
expect { run }.to change(User::Token::Auth, :count).by(1)
121+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { User::Token::Auth.count }`.
75122
end
76123
RUBY
77124

78125
expect_correction(<<-RUBY)
79126
it do
80-
expect { run }.to change { User.count }
127+
expect { run }.to change { User::Token::Auth.count }.by(1)
81128
end
82129
RUBY
83130
end
84131

85-
it 'flags change matcher when receiver is a top-level constant' do
132+
it 'flags change matcher when receiver is a variable' do
86133
expect_offense(<<-RUBY)
87134
it do
88-
expect { run }.to change(::User, :count)
89-
^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { ::User.count }`.
135+
expect { run }.to change(user, :count)
136+
^^^^^^^^^^^^^^^^^^^^ Prefer `change { user.count }`.
90137
end
91138
RUBY
92139

93140
expect_correction(<<-RUBY)
94141
it do
95-
expect { run }.to change { ::User.count }
142+
expect { run }.to change { user.count }
96143
end
97144
RUBY
98145
end
99146

100-
it 'flags change matcher when receiver is a variable' do
147+
it 'flags change matcher when message is a string' do
101148
expect_offense(<<-RUBY)
102149
it do
103-
expect { run }.to change(user, :status)
104-
^^^^^^^^^^^^^^^^^^^^^ Prefer `change { user.status }`.
150+
expect { run }.to change(user, 'status')
151+
^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { user.status }`.
105152
end
106153
RUBY
107154

@@ -127,6 +174,21 @@
127174
RUBY
128175
end
129176

177+
it 'flags change matcher when receiver is a top-level constant' do
178+
expect_offense(<<-RUBY)
179+
it do
180+
expect { run }.to change(::User, :count)
181+
^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { ::User.count }`.
182+
end
183+
RUBY
184+
185+
expect_correction(<<-RUBY)
186+
it do
187+
expect { run }.to change { ::User.count }
188+
end
189+
RUBY
190+
end
191+
130192
it 'registers an offense for change matcher with an instance variable' do
131193
expect_offense(<<-RUBY)
132194
it do

0 commit comments

Comments
 (0)