Add ScatteredSetup cop

[backus]

You should only use max one hook type per example group.

# bad
describe Foo do
  before { setup1 }
  before { setup2 }
end

# good
describe Foo do
  before do
    setup1
    setup2
  end
end

[backus]

@andyw8 or @bquorning what do you think of this cop? If you agree with its intent can you review the code and merge if you approve?

[bquorning]

I like it!

How about checking eventual arguments? Calling before, before(:each) and before(:example) in the same context, we should recommend merging them. Not so if calling once with :each and once with :all.

[backus]

Good catch that is definitely necessary

[bquorning]

As I recall, valid arguments are :suite, :context/:all, and :example/:each/no argument.

And as you suggested in #252 (comment), adding these to the language definition sounds really useful.

[andyw8]

Nice idea. Just ran it over a large codebase with some very long tests and it seemed to be accurate. This currently generates warnings though:

describe do
  context do
    before(:all) do
    end

    before(:each) do
    end

    after(:each) do
    end

    after(:all) do
    end
  end
end

As @bquorning says, I think we should allow :each and :all in the same context.

[backus]

I've cleaned up this PR and addressed the functional behavior feedback here. I haven't extracted the different hook scope names to the language yet because I didn't want to further expand this PR. @bquorning @andyw8 lmk what you think

[andyw8]

Worth defining a rubocop-rspec error for this rather than a plain RuntimeError?

[backus]

Updated

[andyw8]

LGTM

[backus]

@andyw8 updated to extract that error class.

[backus]

Side note / question @andyw8 @bquorning. I've got these node wrapper that I've also refactored a bit in this PR and I'm struggling to name it well. Right now I've got RuboCop::RSpec::Interface which is an abstract class that wraps a RuboCop::Node. The concrete classes inheriting from Interface are RuboCop::RSpec::ExampleGroup, RuboCop::RSpec::Example, and RuboCop::RSpec::Hook. They're useful because they let me centralize destructuring of RSpec methods that are commonly worked with in these cops. For example, here I can do hook.scope # => :each and hook.name # => :before and for examples I can do things like example.docstring.

Anyways, what is a good name for RuboCop::RSpec::Interface? I tried "Construct" before but that doesn't feel right. Interface is better but its too loaded of a CS term IMO. Basically, I want a nice concise way of saying RSpecDSLNodeWrapper 😆

[backus]

Actually I removed the error. It is a slim chance anyone runs into it but if they do it shouldn't raise an error because they are doing something weird