RSpec/ScatteredSetup: Incorrect autocorrect for around blocks.

[zverok]

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.

[pirj]

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.