RSpec/MultipleSubjects and RSpec/LeadingSubject autocorrect removes subject used in tests

[boardfish]

Given this code:

describe "Something" do
  let(:something_unrelated) { "i exist" }

  subject(:named_subject) { 1 }
  subject do
    2
  end

  it "should be 2" do
    expect(subject).to eq(2)
  end
end

And this reduced example command (assuming a config that requires rubocop-rspec):

bundle exec rubocop --only RSpec/MultipleSubjects,RSpec/LeadingSubject spec/example_spec.rb -a -d

This autocorrects to:

describe "Something" do
  subject(:named_subject) { 1 }
  let(:something_unrelated) { "i exist" }


  it "should be 2" do
    expect(subject).to eq(2)
  end
end

Output:

Inspecting 1 file
C

Offenses:

spec/example_spec.rb:2:3: C: [Corrected] RSpec/MultipleSubjects: Do not set more than one subject per example group
  subject do ...
  ^^^^^^^^^^
spec/example_spec.rb:4:3: C: [Corrected] RSpec/LeadingSubject: Declare subject above any other let declarations.
  subject(:named_subject) { 1 }
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/example_spec.rb:4:3: C: [Corrected] RSpec/MultipleSubjects: Do not set more than one subject per example group
  subject(:named_subject) { 1 }
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/example_spec.rb:5:3: C: [Corrected] RSpec/LeadingSubject: Declare subject above any other let declarations.
  subject do ...
  ^^^^^^^^^^

I'm guessing the sequence of events is something like this:

1. RSpec/MultipleSubjects no-ops. Ordinarily, if executed alone it would turn the named subject into a let.
2. RSpec/LeadingSubject moves the named subject to the start of the block.
3. RSpec/LeadingSubject moves the unnamed subject to the start of the block:

describe "Something" do
  subject do
    2
  end
  subject(:named_subject) { 1 }
  let(:something_unrelated) { "i exist" }


  it "should be 2" do
    expect(subject).to eq(2)
  end
end

4. MultipleSubjects removes the unnamed subject because it will be overridden by the declaration of the named_subject.

This makes the test fail.

---

This was based on some relatively recent code (albeit a misuse of subjects). I agree with the autocorrect behaviour of MultipleSubjects in concept, but I'm not sure what should be done to prevent the conflict of these two cops.

[pirj]

Thanks for reporting. Do you think we should avoid changing the order of subject definitions and move on?

[boardfish]

I think that's one possible option, and is probably the right one – I feel as though we should only avoid doing it if there are multiple subjects in the context, but I don't know how practical it is to suggest that. I've also considered:

1. Remove reordering of subject definitions entirely.
2. Get broader context of whether subject was called in the current context and use it to decide how to act here.
3. Mark LeadingSubject and/or MultipleSubjects as unsafe for autocorrect.
4. Have one cop act differently if another is being executed (unlikely to be possible).