Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Master (Unreleased)

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

## 3.8.0 (2025-11-12)

Expand Down Expand Up @@ -984,6 +985,7 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
[@composerinteralia]: https://github.com/composerinteralia
[@corsonknowles]: https://github.com/corsonknowles
[@corydiamand]: https://github.com/corydiamand
[@d4rky-pl]: https://github.com/d4rky-pl
[@darhazer]: https://github.com/Darhazer
[@daveworth]: https://github.com/daveworth
[@dduugg]: https://github.com/dduugg
Expand Down
9 changes: 9 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -5747,6 +5747,7 @@ Checks for setup scattered across multiple hooks in an example group.
Unify `before` and `after` hooks when possible.
However, `around` hooks are allowed to be defined multiple times,
as unifying them would typically make the code harder to read.
Hooks defined in class methods are also ignored.

[#examples-rspecscatteredsetup]
=== Examples
Expand All @@ -5772,6 +5773,14 @@ describe Foo do
around { |example| before1; example.call; after1 }
around { |example| before2; example.call; after2 }
end

# good
describe Foo do
before { setup1 }
def self.setup
before { setup2 }
end
end
----

[#references-rspecscatteredsetup]
Expand Down
12 changes: 11 additions & 1 deletion lib/rubocop/cop/rspec/scattered_setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module RSpec
# Unify `before` and `after` hooks when possible.
# However, `around` hooks are allowed to be defined multiple times,
# as unifying them would typically make the code harder to read.
# Hooks defined in class methods are also ignored.
#
# @example
# # bad
Expand All @@ -30,6 +31,14 @@ module RSpec
# around { |example| before2; example.call; after2 }
# end
#
# # good
# describe Foo do
# before { setup1 }
# def self.setup
# before { setup2 }
# end
# end
#
class ScatteredSetup < Base
include FinalEndLocation
include RangeHelp
Expand All @@ -53,9 +62,10 @@ def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler

private

def repeated_hooks(node)
def repeated_hooks(node) # rubocop:disable Metrics/CyclomaticComplexity
hooks = RuboCop::RSpec::ExampleGroup.new(node)
.hooks
.reject(&:inside_class_method?)
.select { |hook| hook.knowable_scope? && hook.name != :around }
.group_by { |hook| [hook.name, hook.scope, hook.metadata] }
.values
Expand Down
13 changes: 13 additions & 0 deletions lib/rubocop/rspec/hook.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ def example?
scope.equal?(:each)
end

def inside_class_method? # rubocop:disable Metrics/CyclomaticComplexity
parent = node.parent
return false unless parent

parent.defs_type? || (parent.def_type? && inside_class_self?(parent))
end

def scope
return :each if scope_argument&.hash_type?

Expand Down Expand Up @@ -76,6 +83,12 @@ def scope_name
def scope_argument
node.send_node.first_argument
end

def inside_class_self?(node)
node.parent&.sclass_type? ||
(node.parent&.begin_type? && node.parent.parent&.sclass_type?) ||
false
end
end
end
end
25 changes: 25 additions & 0 deletions spec/rubocop/cop/rspec/scattered_setup_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,29 @@
end
RUBY
end

it 'ignores blocks defined inside class methods' do
expect_no_offenses(<<~RUBY)
describe Foo do
before { bar }
def self.setup
before { baz }
end
setup
end
RUBY
end

it 'ignores blocks defined in class << self' do
expect_no_offenses(<<~RUBY)
describe Foo do
before { bar }
class << self
def setup
before { baz }
end
end
end
RUBY
end
end
96 changes: 96 additions & 0 deletions spec/rubocop/rspec/hook_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,100 @@ def metadata(source)
).to eq("{#{expected_symbol}, #{expected_special}, #{expected_focus}}")
end
end

describe '#inside_class_method?' do
def example_group_hook(source)
RuboCop::RSpec::ExampleGroup.new(parse_source(source).ast).hooks.first
end

it 'returns true if the hook is in class method' do
expect(
example_group_hook(
'describe Foo { def self.setup; before { do_something }; end }'
).inside_class_method?
).to be(true)
end

it 'returns true if the hook is in class << self method' do
source = <<~RUBY
describe Foo do
class << self
def setup
before { do_something }
end
end
end
RUBY
expect(example_group_hook(source).inside_class_method?).to be(true)
end

it 'returns true if the hook is in method in multiline class << self' do
source = <<~RUBY
describe Foo do
class << self
other_code

def setup
before { do_something }
end
end
end
RUBY
expect(example_group_hook(source).inside_class_method?).to be(true)
end

it 'returns false if the hook is in instance method' do
expect(
example_group_hook(
'describe Foo { def setup; before { do_something }; end }'
).inside_class_method?
).to be(false)
end

it 'returns false if the hook is in instance method of multiline class' do
source = <<~RUBY
describe Foo do
other_code

def setup
before { do_something }
end
end
RUBY
expect(example_group_hook(source).inside_class_method?).to be(false)
end

it 'returns false if the hook is in in method in global scope' do
expect(
example_group_hook(
'def setup; before { do_something }; end'
).inside_class_method?
).to be(false)
end

it 'returns false if the hook is in multiline global scope' do
source = <<~RUBY
other_code

def setup
before { do_something }
end
RUBY
expect(example_group_hook(source).inside_class_method?).to be(false)
end

it 'returns false if the hook is in example group body' do
expect(
example_group_hook(
'describe Foo { before { do_something } }'
).inside_class_method?
).to be(false)
end

it 'returns false if the hook is in global scope' do
expect(
hook('before { do_something }').inside_class_method?
).to be(false)
end
end
end