Skip to content

Commit ce8a208

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 ce8a208

File tree

6 files changed

+82
-1
lines changed

6 files changed

+82
-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: 16 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
@@ -56,7 +65,7 @@ def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
5665
def repeated_hooks(node)
5766
hooks = RuboCop::RSpec::ExampleGroup.new(node)
5867
.hooks
59-
.select { |hook| hook.knowable_scope? && hook.name != :around }
68+
.select { |hook| repeated_hook?(hook) }
6069
.group_by { |hook| [hook.name, hook.scope, hook.metadata] }
6170
.values
6271
.reject(&:one?)
@@ -94,6 +103,12 @@ def autocorrect(corrector, first_occurrence, occurrence)
94103
corrector.remove(range_by_whole_lines(occurrence.source_range,
95104
include_final_newline: true))
96105
end
106+
107+
def repeated_hook?(hook)
108+
hook.knowable_scope? &&
109+
hook.name != :around &&
110+
!hook.inside_class_method?
111+
end
97112
end
98113
end
99114
end

lib/rubocop/rspec/hook.rb

Lines changed: 7 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?
29+
parent = node.parent
30+
return false unless parent
31+
32+
parent.defs_type?
33+
end
34+
2835
def scope
2936
return :each if scope_argument&.hash_type?
3037

spec/rubocop/cop/rspec/scattered_setup_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,4 +202,16 @@
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
205217
end

spec/rubocop/rspec/hook_spec.rb

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,40 @@ 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 inside 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 false if the hook is inside instance method' do
155+
expect(
156+
example_group_hook(
157+
'describe Foo { def setup; before { do_something }; end }'
158+
).inside_class_method?
159+
).to be(false)
160+
end
161+
162+
it 'returns false if the hook is in example group body' do
163+
expect(
164+
example_group_hook(
165+
'describe Foo { before { do_something } }'
166+
).inside_class_method?
167+
).to be(false)
168+
end
169+
170+
it 'returns false if the hook is in global scope' do
171+
expect(
172+
hook('before { do_something }').inside_class_method?
173+
).to be(false)
174+
end
175+
end
140176
end

0 commit comments

Comments
 (0)