rubocop-rspec icon indicating copy to clipboard operation
rubocop-rspec copied to clipboard

RSpec/ScatteredSetup: Incorrect autocorrect for `around` blocks.

Open zverok opened this issue 1 year ago • 1 comments

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.

zverok avatar Aug 03 '24 16:08 zverok

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.

pirj avatar Aug 04 '24 10:08 pirj