rubocop-rspec
rubocop-rspec copied to clipboard
RSpec/ScatteredSetup: Incorrect autocorrect for `around` blocks.
Minimal reproducible example:
RSpec.describe "around blocs" do
around { |ex| puts "before 1"; ex.call; puts "after 1" }
around { |ex| puts "before 2"; ex.call; puts "after 2" }
it { puts "example" }
end
Running this prints:
before 1
before 2
example
after 2
after 1
Running rubocop-rspec on it complains (reasonably):
RSpec/ScatteredSetup: Do not define multiple around hooks in the same example group.
Autocorrect does this:
RSpec.describe "around blocs" do
around { |ex|
puts "before 1"; ex.call; puts "after 1"
puts "before 2"; ex.call; puts "after 2"
}
it { puts "example" }
end
...which obviously doesn’t have the same meaning. Running it with RSpec produces this:
before 1
example
after 1
before 2
example
after 2
So probably around blocks either shouldn’t be autocorrected, or there should be more sophisticated analysis. The proper correction for this case was something like
around { |ex|
puts "before 1"; puts "before 2"; ex.call; puts "after 2" ; puts "after 1"
}
(don’t mind the code layout; the essence is the order of operations).
A more realistic case (where it got me) was something along the lines:
around { |ex| DB.in_transaction(&ex) }
around { |ex| Time.in_zone('Europe/Kyiv', &ex) }
...where the proper correction would be
around { |ex| DB.in_transaction { Time.in_zone('Europe/Kyiv', &ex) } }
...though I am not sure that this autocorrect is easy to deduce (and that the result looks better than the original code).
So, IDK, maybe around should be excluded from ScatteredSetup check altogether.
Looks like a bug.
the result looks better than the original code
Indeed, it’s subjectively harder to read after correction.
around blocks either shouldn’t be autocorrected
Totally agree, especially taking into account that we don’t have a single spec on how after autocorrection should behave.