Skip to content

Commit 93d9f14

Browse files
authored
Merge pull request #2117 from rubocop/2075
Fix a false positive for `RSpec/LetSetup` when `let!` used in outer scope
2 parents ef3f1ee + 9b9b787 commit 93d9f14

File tree

4 files changed

+162
-0
lines changed

4 files changed

+162
-0
lines changed

CHANGELOG.md

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

55
- Add new cop `RSpec/LeakyLocalVariable`. ([@lovro-bikic])
66
- Bump RuboCop requirement to +1.81. ([@ydah])
7+
- Fix a false positive for `RSpec/LetSetup` when `let!` used in outer scope. ([@ydah])
78

89
## 3.7.0 (2025-09-01)
910

docs/modules/ROOT/pages/cops_rspec.adoc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3575,6 +3575,17 @@ before { create(:widget) }
35753575
it 'counts widgets' do
35763576
expect(Widget.count).to eq(1)
35773577
end
3578+
3579+
# good
3580+
describe 'a widget' do
3581+
let!(:my_widget) { create(:widget) }
3582+
context 'when visiting its page' do
3583+
let!(:my_widget) { create(:widget, name: 'Special') }
3584+
it 'counts widgets' do
3585+
expect(Widget.count).to eq(1)
3586+
end
3587+
end
3588+
end
35783589
----
35793590
35803591
[#references-rspecletsetup]

lib/rubocop/cop/rspec/let_setup.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,18 @@ module RSpec
2525
# it 'counts widgets' do
2626
# expect(Widget.count).to eq(1)
2727
# end
28+
#
29+
# # good
30+
# describe 'a widget' do
31+
# let!(:my_widget) { create(:widget) }
32+
# context 'when visiting its page' do
33+
# let!(:my_widget) { create(:widget, name: 'Special') }
34+
# it 'counts widgets' do
35+
# expect(Widget.count).to eq(1)
36+
# end
37+
# end
38+
# end
39+
#
2840
class LetSetup < Base
2941
MSG = 'Do not use `let!` to setup objects not referenced in tests.'
3042

@@ -59,6 +71,8 @@ def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
5971

6072
def unused_let_bang(node)
6173
child_let_bang(node) do |method_send, method_name|
74+
next if overrides_outer_let_bang?(node, method_name)
75+
6276
yield(method_send) unless method_called?(node, method_name.to_sym)
6377
end
6478
end
@@ -68,6 +82,22 @@ def child_let_bang(node, &block)
6882
let_bang(let, &block)
6983
end
7084
end
85+
86+
def overrides_outer_let_bang?(node, method_name)
87+
node.each_ancestor(:block).any? do |ancestor|
88+
next unless example_or_shared_group_or_including?(ancestor)
89+
90+
outer_let_bang?(ancestor, method_name)
91+
end
92+
end
93+
94+
def outer_let_bang?(ancestor_node, method_name)
95+
RuboCop::RSpec::ExampleGroup.new(ancestor_node).lets.any? do |let|
96+
let_bang(let) do |_send, name|
97+
name == method_name
98+
end
99+
end
100+
end
71101
end
72102
end
73103
end

spec/rubocop/cop/rspec/let_setup_spec.rb

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,124 @@
137137
end
138138
RUBY
139139
end
140+
141+
it 'ignores let! that overrides outer scope let!' do
142+
expect_no_offenses(<<~RUBY)
143+
describe User, type: :model do
144+
let!(:user) { create(:user) }
145+
146+
it 'is valid' do
147+
expect(user).to be_valid
148+
end
149+
150+
context 'with no direct usage' do
151+
let!(:user) { create(:user, :special) }
152+
153+
it 'creates something' do
154+
expect(SomeModel.count).to eq(2)
155+
end
156+
end
157+
end
158+
RUBY
159+
end
160+
161+
it 'ignores let! overriding outer scope across multiple nesting levels' do
162+
expect_no_offenses(<<~RUBY)
163+
describe User do
164+
let!(:user) { create(:user) }
165+
166+
it 'uses user' do
167+
expect(user).to be_valid
168+
end
169+
170+
context 'level 1' do
171+
context 'level 2' do
172+
let!(:user) { create(:user, :admin) }
173+
174+
it 'creates admin user' do
175+
expect(User.count).to eq(2)
176+
end
177+
end
178+
end
179+
end
180+
RUBY
181+
end
182+
183+
it 'still flags unused let! when outer scope has different name' do
184+
expect_offense(<<~RUBY)
185+
describe User do
186+
let!(:user) { create(:user) }
187+
188+
it 'uses user' do
189+
expect(user).to be_valid
190+
end
191+
192+
context 'different variable' do
193+
let!(:admin) { create(:user, :admin) }
194+
^^^^^^^^^^^^ Do not use `let!` to setup objects not referenced in tests.
195+
196+
it 'creates admin user' do
197+
expect(User.count).to eq(2)
198+
end
199+
end
200+
end
201+
RUBY
202+
end
203+
204+
it 'allows let! override when outer let! is used elsewhere' do
205+
expect_no_offenses(<<~RUBY)
206+
describe User do
207+
let!(:user) { create(:user) }
208+
209+
it 'uses user' do
210+
expect(user).to be_valid
211+
end
212+
213+
context 'special case' do
214+
let!(:user) { create(:user, :special) }
215+
216+
it 'setup only' do
217+
expect(User.count).to eq(2)
218+
end
219+
end
220+
end
221+
RUBY
222+
end
223+
224+
it 'does not consider non-RSpec blocks as outer scope' do
225+
expect_offense(<<~RUBY)
226+
describe User do
227+
[1, 2, 3].each do |i|
228+
let!(:user) { create(:user, id: i) }
229+
^^^^^^^^^^^ Do not use `let!` to setup objects not referenced in tests.
230+
231+
it 'creates user' do
232+
expect(User.count).to eq(3)
233+
end
234+
end
235+
end
236+
RUBY
237+
end
238+
239+
it 'correctly identifies override through mixed block types' do
240+
expect_no_offenses(<<~RUBY)
241+
describe User do
242+
let!(:user) { create(:user) }
243+
244+
it 'uses user' do
245+
expect(user).to be_valid
246+
end
247+
248+
[true, false].each do |flag|
249+
context "when flag is \#{flag}" do
250+
let!(:user) { create(:user, flag: flag) }
251+
252+
it 'setup only' do
253+
expect(User.count).to be > 0
254+
end
255+
end
256+
end
257+
end
258+
RUBY
259+
end
140260
end

0 commit comments

Comments
 (0)