Skip to content

Commit

Permalink
Merge pull request #866 from Cofense/scattered-setup-message-block-name
Browse files Browse the repository at this point in the history
Add block name and other lines to `RSpec/ScatteredSetup` message
  • Loading branch information
bquorning authored Feb 3, 2020
2 parents 0e45744 + 318244a commit f39e531
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 21 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Add autocorrect support for `RSpec/ExpectActual` cop. ([@dduugg][], [@pirj][])
* Add `RSpec/RepeatedExampleGroupBody` cop. ([@lazycoder9][])
* Add `RSpec/RepeatedExampleGroupDescription` cop. ([@lazycoder9][])
* Add block name and other lines to `RSpec/ScatteredSetup` message. ([@elebow][])

## 1.37.1 (2019-12-16)

Expand Down Expand Up @@ -480,3 +481,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
[@ybiquitous]: https://github.com/ybiquitous
[@dduugg]: https://github.com/dduugg
[@lazycoder9]: https://github.com/lazycoder9
[@elebow]: https://github.com/elebow
32 changes: 25 additions & 7 deletions lib/rubocop/cop/rspec/scattered_setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,43 @@ module RSpec
# end
#
class ScatteredSetup < Cop
MSG = 'Do not define multiple hooks in the same example group.'
MSG = 'Do not define multiple `%<hook_name>s` hooks in the same '\
'example group (also defined on %<lines>s).'

def on_block(node)
return unless example_group?(node)

analyzable_hooks(node).each do |repeated_hook|
add_offense(repeated_hook)
repeated_hooks(node).each do |occurrences|
lines = occurrences.map(&:first_line)

occurrences.each do |occurrence|
lines_except_current = lines - [occurrence.first_line]
message = format(MSG, hook_name: occurrences.first.method_name,
lines: lines_msg(lines_except_current))
add_offense(occurrence, message: message)
end
end
end

def analyzable_hooks(node)
RuboCop::RSpec::ExampleGroup.new(node)
def repeated_hooks(node)
hooks = RuboCop::RSpec::ExampleGroup.new(node)
.hooks
.select(&:knowable_scope?)
.group_by { |hook| [hook.name, hook.scope, hook.metadata] }
.values
.reject(&:one?)
.flatten
.map(&:to_node)

hooks.map do |hook|
hook.map(&:to_node)
end
end

def lines_msg(numbers)
if numbers.size == 1
"line #{numbers.first}"
else
"lines #{numbers.join(', ')}"
end
end
end
end
Expand Down
28 changes: 14 additions & 14 deletions spec/rubocop/cop/rspec/scattered_setup_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,22 @@
expect_offense(<<-RUBY)
describe Foo do
before { bar }
^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
^^^^^^^^^^^^^^ Do not define multiple `before` hooks in the same example group (also defined on line 3).
before { baz }
^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
^^^^^^^^^^^^^^ Do not define multiple `before` hooks in the same example group (also defined on line 2).
end
RUBY
end

it 'flags multiple hooks of the same scope with different symbols' do
expect_offense(<<-RUBY)
describe Foo do
before { bar }
^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
before(:each) { baz }
^^^^^^^^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
before(:example) { baz }
^^^^^^^^^^^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
after { bar }
^^^^^^^^^^^^^ Do not define multiple `after` hooks in the same example group (also defined on lines 3, 4).
after(:each) { baz }
^^^^^^^^^^^^^^^^^^^^ Do not define multiple `after` hooks in the same example group (also defined on lines 2, 4).
after(:example) { baz }
^^^^^^^^^^^^^^^^^^^^^^^ Do not define multiple `after` hooks in the same example group (also defined on lines 2, 3).
end
RUBY
end
Expand All @@ -31,9 +31,9 @@
expect_offense(<<-RUBY)
describe Foo do
before(:all) { bar }
^^^^^^^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
^^^^^^^^^^^^^^^^^^^^ Do not define multiple `before` hooks in the same example group (also defined on line 3).
before(:all) { baz }
^^^^^^^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
^^^^^^^^^^^^^^^^^^^^ Do not define multiple `before` hooks in the same example group (also defined on line 2).
end
RUBY
end
Expand Down Expand Up @@ -109,13 +109,13 @@
expect_offense(<<-RUBY)
describe Foo do
before(:each, :special_case) { foo }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define multiple `before` hooks in the same example group (also defined on lines 3, 4, 5).
before(:example, :special_case) { bar }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define multiple `before` hooks in the same example group (also defined on lines 2, 4, 5).
before(:example, special_case: true) { bar }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define multiple `before` hooks in the same example group (also defined on lines 2, 3, 5).
before(special_case: true) { bar }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define multiple hooks in the same example group.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define multiple `before` hooks in the same example group (also defined on lines 2, 3, 4).
before(:example, special_case: false) { bar }
end
RUBY
Expand Down

0 comments on commit f39e531

Please sign in to comment.