Skip to content

Commit 9e60861

Browse files
committed
Fix a false positive for RSpec/ScatteredSetup when the hook is defined inside a class method
Fixes #2146
1 parent 6a5bf09 commit 9e60861

File tree

6 files changed

+156
-1
lines changed

6 files changed

+156
-1
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Master (Unreleased)
44

55
- Fix a false positive for `RSpec/LeakyLocalVariable` when variables are used only in example metadata (e.g., skip messages). ([@ydah])
6+
- Fix a false positive for `RSpec/ScatteredSetup` when the hook is defined inside a class method. ([@d4rky-pl])
67

78
## 3.8.0 (2025-11-12)
89

@@ -984,6 +985,7 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
984985
[@composerinteralia]: https://github.com/composerinteralia
985986
[@corsonknowles]: https://github.com/corsonknowles
986987
[@corydiamand]: https://github.com/corydiamand
988+
[@d4rky-pl]: https://github.com/d4rky-pl
987989
[@darhazer]: https://github.com/Darhazer
988990
[@daveworth]: https://github.com/daveworth
989991
[@dduugg]: https://github.com/dduugg

docs/modules/ROOT/pages/cops_rspec.adoc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5747,6 +5747,7 @@ Checks for setup scattered across multiple hooks in an example group.
57475747
Unify `before` and `after` hooks when possible.
57485748
However, `around` hooks are allowed to be defined multiple times,
57495749
as unifying them would typically make the code harder to read.
5750+
Hooks defined in class methods are also ignored.
57505751
57515752
[#examples-rspecscatteredsetup]
57525753
=== Examples
@@ -5772,6 +5773,14 @@ describe Foo do
57725773
around { |example| before1; example.call; after1 }
57735774
around { |example| before2; example.call; after2 }
57745775
end
5776+
5777+
# good
5778+
describe Foo do
5779+
before { setup1 }
5780+
def self.setup
5781+
before { setup2 }
5782+
end
5783+
end
57755784
----
57765785
57775786
[#references-rspecscatteredsetup]

lib/rubocop/cop/rspec/scattered_setup.rb

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ module RSpec
88
# Unify `before` and `after` hooks when possible.
99
# However, `around` hooks are allowed to be defined multiple times,
1010
# as unifying them would typically make the code harder to read.
11+
# Hooks defined in class methods are also ignored.
1112
#
1213
# @example
1314
# # bad
@@ -30,6 +31,14 @@ module RSpec
3031
# around { |example| before2; example.call; after2 }
3132
# end
3233
#
34+
# # good
35+
# describe Foo do
36+
# before { setup1 }
37+
# def self.setup
38+
# before { setup2 }
39+
# end
40+
# end
41+
#
3342
class ScatteredSetup < Base
3443
include FinalEndLocation
3544
include RangeHelp
@@ -53,9 +62,10 @@ def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
5362

5463
private
5564

56-
def repeated_hooks(node)
65+
def repeated_hooks(node) # rubocop:disable Metrics/CyclomaticComplexity
5766
hooks = RuboCop::RSpec::ExampleGroup.new(node)
5867
.hooks
68+
.reject(&:inside_class_method?)
5969
.select { |hook| hook.knowable_scope? && hook.name != :around }
6070
.group_by { |hook| [hook.name, hook.scope, hook.metadata] }
6171
.values

lib/rubocop/rspec/hook.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ def example?
2525
scope.equal?(:each)
2626
end
2727

28+
def inside_class_method? # rubocop:disable Metrics/CyclomaticComplexity
29+
parent = node.parent
30+
return false unless parent
31+
32+
parent.defs_type? || (parent.def_type? && inside_class_self?(parent))
33+
end
34+
2835
def scope
2936
return :each if scope_argument&.hash_type?
3037

@@ -76,6 +83,12 @@ def scope_name
7683
def scope_argument
7784
node.send_node.first_argument
7885
end
86+
87+
def inside_class_self?(node)
88+
node.parent&.sclass_type? ||
89+
(node.parent&.begin_type? && node.parent.parent&.sclass_type?) ||
90+
false
91+
end
7992
end
8093
end
8194
end

spec/rubocop/cop/rspec/scattered_setup_spec.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,4 +202,29 @@
202202
end
203203
RUBY
204204
end
205+
206+
it 'ignores blocks defined inside class methods' do
207+
expect_no_offenses(<<~RUBY)
208+
describe Foo do
209+
before { bar }
210+
def self.setup
211+
before { baz }
212+
end
213+
setup
214+
end
215+
RUBY
216+
end
217+
218+
it 'ignores blocks defined in class << self' do
219+
expect_no_offenses(<<~RUBY)
220+
describe Foo do
221+
before { bar }
222+
class << self
223+
def setup
224+
before { baz }
225+
end
226+
end
227+
end
228+
RUBY
229+
end
205230
end

spec/rubocop/rspec/hook_spec.rb

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,100 @@ def metadata(source)
137137
).to eq("{#{expected_symbol}, #{expected_special}, #{expected_focus}}")
138138
end
139139
end
140+
141+
describe '#inside_class_method?' do
142+
def example_group_hook(source)
143+
RuboCop::RSpec::ExampleGroup.new(parse_source(source).ast).hooks.first
144+
end
145+
146+
it 'returns true if the hook is in class method' do
147+
expect(
148+
example_group_hook(
149+
'describe Foo { def self.setup; before { do_something }; end }'
150+
).inside_class_method?
151+
).to be(true)
152+
end
153+
154+
it 'returns true if the hook is in class << self method' do
155+
source = <<~RUBY
156+
describe Foo do
157+
class << self
158+
def setup
159+
before { do_something }
160+
end
161+
end
162+
end
163+
RUBY
164+
expect(example_group_hook(source).inside_class_method?).to be(true)
165+
end
166+
167+
it 'returns true if the hook is in method in multiline class << self' do
168+
source = <<~RUBY
169+
describe Foo do
170+
class << self
171+
other_code
172+
173+
def setup
174+
before { do_something }
175+
end
176+
end
177+
end
178+
RUBY
179+
expect(example_group_hook(source).inside_class_method?).to be(true)
180+
end
181+
182+
it 'returns false if the hook is in instance method' do
183+
expect(
184+
example_group_hook(
185+
'describe Foo { def setup; before { do_something }; end }'
186+
).inside_class_method?
187+
).to be(false)
188+
end
189+
190+
it 'returns false if the hook is in instance method of multiline class' do
191+
source = <<~RUBY
192+
describe Foo do
193+
other_code
194+
195+
def setup
196+
before { do_something }
197+
end
198+
end
199+
RUBY
200+
expect(example_group_hook(source).inside_class_method?).to be(false)
201+
end
202+
203+
it 'returns false if the hook is in in method in global scope' do
204+
expect(
205+
example_group_hook(
206+
'def setup; before { do_something }; end'
207+
).inside_class_method?
208+
).to be(false)
209+
end
210+
211+
it 'returns false if the hook is in multiline global scope' do
212+
source = <<~RUBY
213+
other_code
214+
215+
def setup
216+
before { do_something }
217+
end
218+
RUBY
219+
expect(example_group_hook(source).inside_class_method?).to be(false)
220+
end
221+
222+
it 'returns false if the hook is in example group body' do
223+
expect(
224+
example_group_hook(
225+
'describe Foo { before { do_something } }'
226+
).inside_class_method?
227+
).to be(false)
228+
end
229+
230+
it 'returns false if the hook is in global scope' do
231+
expect(
232+
hook('before { do_something }').inside_class_method?
233+
).to be(false)
234+
end
235+
end
140236
end

0 commit comments

Comments
 (0)