Skip to content

[bot] Fix RSpec/MultipleSubjects#133

Closed
6[bot] wants to merge 1 commit intomainfrom
fix/rspec-multiple_subjects-23409148441
Closed

[bot] Fix RSpec/MultipleSubjects#133
6[bot] wants to merge 1 commit intomainfrom
fix/rspec-multiple_subjects-23409148441

Conversation

@6
Copy link
Copy Markdown
Contributor

@6 6 bot commented Mar 22, 2026

Automated fix fix for RSpec/MultipleSubjects corpus conformance.

Details

Cop RSpec/MultipleSubjects
Backend codex
Mode fix
Turns 1
Run #59

Result

Implemented the `RSpec/MultipleSubjects` FN fix in [src/cop/rspec/multiple_subjects.rs](/home/runner/work/nitrocop/nitrocop/src/cop/rspec/multiple_subjects.rs) by teaching the cop to collect `subject` declarations from `if`/`unless` branches in source order, matching RuboCop for the version-gated/spec-branch patterns. I also added the required investigation `///` comment and three regression cases to [tests/fixtures/cops/rspec/multiple_subjects/offense.rb](/home/runner/work/nitrocop/nitrocop/tests/fixtures/cops/rspec/multiple_subjects/offense.rb).

Verified with `cargo test --lib -- cop::rspec::multiple_subjects` and committed only those files as `78ca41db` with message `Fix conditional subjects in RSpec/MultipleSubjects`.

Changes

  0 files changed
Agent conversation excerpt

I’m looking at the current cop and fixture setup first, then I’ll reproduce the targeted spec failure and inspect how Prism represents the if-wrapped subject cases so the fix stays narrow.

I’m looking at the current cop and fixture setup first, then I’ll reproduce the targeted spec failure and inspect how Prism represents the if-wrapped subject cases so the fix stays narrow.

exec_command: git status --short

exec_command: sed -n '1,220p' src/cop/rspec/multiple_subjects.rs

exec_command: sed -n '1,220p' tests/fixtures/cops/rspec/multiple_subjects/offense.rb

exec_command: sed -n '1,220p' tests/fixtures/cops/rspec/multiple_subjects/no_offense.rb

The worktree already had a local edit in the target fixture, and it’s malformed at the moment because the new subject lines are outside any example group. I’m checking how other RSpec cops traverse nested structure before I patch the fixture and cop together.

The worktree already had a local edit in the target fixture, and it’s malformed at the moment because the new subject lines are outside any example group. I’m checking how other RSpec cops traverse nested structure before I patch the fixture and cop together.

exec_command: rg -n "is_example_group|it_behaves_like|shared_examples|subject!|example_group" src/cop/rspec

exec_command: ruby -rprism -e 'puts Prism.parse(%q(describe Foo do\n if cond\n subject { 1 }\n else\n subject { 2 }\n end\nend\n)).value.inspect'

exec_command: cargo test --lib -- cop::rspec::multiple_subjects

The current failure path is confirming the same issue the corpus pointed to: the cop only inspects direct statements in the example-group body, so anything nested under control flow is invisible. I’m pulling one or two existing RSpec traversal patterns from the repo and then I’ll patch the fixture into a valid example that fails for the right reason.

The current failure path is confirming the same issue the corpus pointed to: the cop only inspects direct statements in the example-group body, so anything nested under control flow is invisible. I’m pulling one or two existing RSpec traversal patterns from the repo and then I’ll patch the fixture into a valid example that fails for the right reason.

exec_command: sed -n '1,260p' src/cop/rspec/leading_subject.rs

exec_command: sed -n '1,220p' src/cop/rspec/implicit_block_expectation.rs

write_stdin: ``

exec_command: ruby -e 'src = <<~RUBY describe Foo do if cond subject { 1 } else subject { 2 } end end RUBY require "prism" puts Prism.parse(src).value.inspect '

write_stdin: ``

write_stdin: ``

write_stdin: ``

The failing fixture is now reproducing the missed cases exactly: the added examples are not being counted because they sit under an IfNode, while the existing direct subject cases still work. I’m checking the RSpec utility helpers next so I can decide whether to keep this cop’s current example-group matching or fold in any receiver handling without drifting from RuboCop.

The failing fixture is now reproducing the missed cases exactly: the added examples are not being counted because they sit under an IfNode, while the existing direct subject cases still work. I’m checking the RSpec utility helpers next so I can decide whether to keep this cop’s current example-group matching or fold in any receiver handling without drifting from RuboCop.

exec_command: rg -n "fn is_rspec_example_group|fn is_rspec_shared_group|fn is_rspec_subject" src/cop/util.rs src/cop/util/*.rs src/cop -g '!src/cop/rspec/multiple_subjects.rs'

exec_command: sed -n '1,220p' src/cop/rspec/overwriting_setup.rs

exec_command: sed -n '1,220p' src/cop/rspec/multiple_describes.rs

exec_command: ruby -e 'src = <<~RUBY describe Foo do if cond subject { 1 } subject { 2 } else subject { 3 } end end RUBY File.write("/tmp/test_multiple_subjects_if.rb", src) ' && rubocop --only RS

The plain rubocop binary isn’t loading the RSpec plugin by default in this environment, so I’m switching to an explicit --plugin rubocop-rspec check where I need ground-truth behavior.

The plain rubocop binary isn’t loading the RSpec plugin by default in this environment, so I’m switching to an explicit --plugin rubocop-rspec check where I need ground-truth behavior.

exec_command: rubocop --plugin rubocop-rspec --only RSpec/MultipleSubjects /tmp/test_multiple_subjects_if.rb

exec_command: sed -n '1220,1285p' src/cop/util.rs

write_stdin: ``

exec_command: ruby -e 'src = <<~RUBY describe Foo do if cond subject { 1 } subject { 2 } else subject { 3 } end end RUBY File.write("/tmp/test_multiple_subjects_if_spec.rb", src) ' && rubocop --pl

exec_command: ruby -e 'src = <<~RUBY describe Foo do subject { 1 } subject { 2 } end RUBY File.write("/tmp/test_multiple_subjects_direct_spec.rb", src) ' && rubocop --plugin rubocop-rspec --only RSpec/MultipleS

write_stdin: ``

write_stdin: ``

RuboCop confirms the control-flow case is a real offense once the file is treated as a spec. I’m copying the smallest viable traversal pattern now: recurse through conditional branches inside the example-group body, keep nested example groups isolated, and leave shared-example/include blocks untouched so the existing zero-FP behavior stays intact.

RuboCop confirms the control-flow case is a real offense once the file is treated as a spec. I’m copying the smallest viable traversal pattern now: recurse through conditional branches inside the example-group body, keep nested example groups isolated, and leave shared-example/include blocks untouched so the existing zero-FP behavior stays intact.

exec_command: rg -n "recurse_into_conditional|as_if_node\(|as_unless_node\(|subsequent\(" src/cop/rspec src/cop -g '!src/cop/rspec/multiple_subjects.rs'

exec_command: sed -n '220,360p' src/cop/rspec/leading_subject.rs

exec_command: sed -n '360,430p' src/testutil.rs

Plan:

  1. Repair the fixture into valid example-group code that reproduces the three if-wrapped missed offenses and confirm the targeted test still fails.
  2. Update MultipleSubjects to collect subject declarations through conditional branches while keeping nested example groups and include/shared-example blocks isolated.
  3. Re-run cargo test --lib -- cop::rspec::multiple_subjects, then commit only the cop file and its fixture changes.

Plan:

  1. Repair the fixture into valid example-group code that reproduces the three if-wrapped missed offenses and confirm the targeted test still fails.
  2. Update MultipleSubjects to collect subject declarations through conditional branches while keeping nested example groups and include/shared-example blocks isolated.
  3. Re-run cargo test --lib -- cop::rspec::multiple_subjects, then commit only the cop file and its fixture changes.

update_plan

I’m rewriting the offense fixture into three valid describe blocks with if/else branches so the failing expectations reflect RuboCop’s actual control-flow behavior instead of orphaned subject calls.

I’m rewriting the offense fixture into three valid describe blocks with if/else branches so the failing expectations reflect RuboCop’s actual control-flow behavior instead of orphaned subject calls.

apply_patch

exec_command: cargo test --lib -- cop::rspec::multiple_subjects

write_stdin: ``

write_stdin: ``

write_stdin: ``

write_stdin: ``

write_stdin: ``

write_stdin: ``

I’ve got the failure pinned down cleanly. The code change is next: keep the current top-level example-group entrypoint, but replace the flat direct-statement scan with a conditional-aware collector that walks if/unless branches in source order and nothing broader.

I’ve got the failure pinned down cleanly. The code change is next: keep the current top-level example-group entrypoint, but replace the flat direct-statement scan with a conditional-aware collector that walks if/unless branches in source order and nothing broader.

apply_patch

exec_command: cargo test --lib -- cop::rspec::multiple_subjects

Task prompt (3988 tokens)

Fix RSpec/MultipleSubjects — 0 FP, 3 FN

Instructions

You are fixing ONE cop in nitrocop, a Rust Ruby linter that uses Prism for parsing.

Current state: 229 matches, 0 false positives, 3 false negatives.
Focus on: FN (RuboCop flags code nitrocop misses).

Workflow

  1. Read the Pre-diagnostic Results and Corpus FP/FN Examples sections below first
  2. Verify with RuboCop first (for FP fixes): before writing any code, confirm RuboCop's
    behavior on BOTH the specific FP case AND the general pattern:
    echo '<specific FP case>' > /tmp/test.rb && rubocop --only RSpec/MultipleSubjects /tmp/test.rb
    echo '<general pattern>' > /tmp/test.rb && rubocop --only RSpec/MultipleSubjects /tmp/test.rb
    If RuboCop flags the general pattern, your fix must be narrow enough to not suppress it.
  3. Add a test case FIRST:
    • FN fix: add the missed pattern to tests/fixtures/cops/rspec/multiple_subjects/offense.rb with ^ annotation
    • FP fix: add the false-positive pattern to tests/fixtures/cops/rspec/multiple_subjects/no_offense.rb
  4. Verify test fails: cargo test --lib -- cop::rspec::multiple_subjects
  5. Fix src/cop/rspec/multiple_subjects.rs
  6. Verify test passes: cargo test --lib -- cop::rspec::multiple_subjects
  7. Add a /// doc comment on the cop struct documenting what you found and fixed
  8. Commit only your cop's files

Fixture Format

Mark offenses with ^ markers on the line AFTER the offending source line:

x = 1
     ^^ RSpec/MultipleSubjects: Trailing whitespace detected.

The ^ characters must align with the offending columns. The message format is RSpec/MultipleSubjects: <message text>.

If your test passes immediately

If you add a test case and it passes without code changes, the corpus mismatch is
caused by config/context differences, not a detection bug.
Do NOT loop trying to make the test fail. Instead:

  1. Investigate config resolution (Include/Exclude, cop enablement, disable comments)
  2. The fix is likely in src/config/ or the cop's config handling, not detection logic
  3. If you cannot determine the root cause within 5 minutes, document your findings as
    a /// comment on the cop struct and commit

CRITICAL: Avoid regressions in the opposite direction

When fixing FPs, your change MUST NOT suppress legitimate detections. When fixing FNs,
your change MUST NOT flag code that RuboCop accepts. A fix that eliminates a few issues
in one direction but introduces hundreds in the other is a catastrophic regression.

Before exempting a category of patterns, verify with RuboCop that the general case
is still an offense:

rubocop --only RSpec/MultipleSubjects /tmp/test.rb

If RuboCop flags the general pattern but not your specific case, the difference is in
a narrow context (e.g., enclosing structure, receiver type, argument count) — your fix
must target that specific context, not the broad category.

Rule of thumb: if your fix adds an early return or continue that skips a whole
node type, operator class, or naming pattern, it's probably too broad. Prefer adding a
condition that matches the SPECIFIC differentiating context.

Rules

  • Only modify src/cop/rspec/multiple_subjects.rs and tests/fixtures/cops/rspec/multiple_subjects/
  • Run cargo test --lib -- cop::rspec::multiple_subjects to verify your fix (do NOT run the full test suite)
  • Do NOT touch unrelated files
  • Do NOT use git stash

Pre-diagnostic Results

Diagnosis Summary

Each example was tested by running nitrocop on the extracted source in isolation
with --force-default-config to determine if the issue is a code bug or config issue.
Note: source context is truncated and may not parse perfectly. If a diagnosis
seems wrong (e.g., your test passes immediately for a 'CODE BUG'), treat it as
a config/context issue instead.

  • FN: 3 code bug(s), 0 config/context issue(s)

FN #1: doorkeeper-gem__doorkeeper-openid_connect__225e319: spec/lib/oauth/password_access_token_request_spec.rb:7

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Enclosing structure: if branch (line: if Gem.loaded_specs['doorkeeper'].version >= Gem::Version.create('5.5.1'))
The offense is inside this structure — the cop may need
to handle this context to detect the pattern.

Message: Do not set more than one subject per example group

Ready-made test snippet (add to offense.rb, adjust ^ count):

    subject { Doorkeeper::OAuth::PasswordAccessTokenRequest.new server, client, credentials, resource_owner, { nonce: '123456' } }
^ RSpec/MultipleSubjects: Do not set more than one subject per example group

Full source context:

# frozen_string_literal: true

require 'rails_helper'

describe Doorkeeper::OpenidConnect::OAuth::PasswordAccessTokenRequest do
  if Gem.loaded_specs['doorkeeper'].version >= Gem::Version.create('5.5.1')
    subject { Doorkeeper::OAuth::PasswordAccessTokenRequest.new server, client, credentials, resource_owner, { nonce: '123456' } }
  else
    subject { Doorkeeper::OAuth::PasswordAccessTokenRequest.new server, client, resource_owner, { nonce: '123456' } }
  end

  let(:server) { double }
  let(:client) { double }
  let(:credentials) { }

FN #2: madeintandem__hstore_accessor__b527f0f: spec/hstore_accessor_spec.rb:299

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Enclosing structure: if branch (line: if ::ActiveRecord::VERSION::STRING.to_f >= 4.2)
The offense is inside this structure — the cop may need
to handle this context to detect the pattern.

Message: Do not set more than one subject per example group

Ready-made test snippet (add to offense.rb, adjust ^ count):

      subject { SuperProduct }
^ RSpec/MultipleSubjects: Do not set more than one subject per example group

Full source context:

        expect(Product.not_popular).to eq [product_b]
      end
    end
  end

  describe "#type_for_attribute" do
    if ::ActiveRecord::VERSION::STRING.to_f >= 4.2
      subject { SuperProduct }

      def self.it_returns_the_type_for_the_attribute(type, attribute_name, active_record_type)
        context "#{type}" do
          it "returns the type for the column" do
            expect(subject.type_for_attribute(attribute_name.to_s)).to eq(active_record_type.new)
          end
        end

FN #3: ruby-grape__grape-active_model_serializers__c985c86: spec/grape-active_model_serializers/endpoint_extension_spec.rb:5

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Enclosing structure: if branch (line: if Grape::Util.const_defined?('InheritableSetting'))
The offense is inside this structure — the cop may need
to handle this context to detect the pattern.

Message: Do not set more than one subject per example group

Ready-made test snippet (add to offense.rb, adjust ^ count):

    subject do
^ RSpec/MultipleSubjects: Do not set more than one subject per example group

Full source context:

require 'spec_helper'

describe 'Grape::EndpointExtension' do
  if Grape::Util.const_defined?('InheritableSetting')
    subject do
      Grape::Endpoint.new(
        Grape::Util::InheritableSetting.new,
        path: '/',
        method: 'foo'
      )
    end
  else

Current Rust Implementation

src/cop/rspec/multiple_subjects.rs

use crate::cop::node_type::{BLOCK_NODE, CALL_NODE, STATEMENTS_NODE};
use crate::cop::util::RSPEC_DEFAULT_INCLUDE;
use crate::cop::{Cop, CopConfig};
use crate::diagnostic::{Diagnostic, Severity};
use crate::parse::source::SourceFile;

/// RSpec/MultipleSubjects: Flag multiple `subject` declarations in the same example group.
pub struct MultipleSubjects;

impl Cop for MultipleSubjects {
    fn name(&self) -> &'static str {
        "RSpec/MultipleSubjects"
    }

    fn default_severity(&self) -> Severity {
        Severity::Convention
    }

    fn default_include(&self) -> &'static [&'static str] {
        RSPEC_DEFAULT_INCLUDE
    }

    fn interested_node_types(&self) -> &'static [u8] {
        &[BLOCK_NODE, CALL_NODE, STATEMENTS_NODE]
    }

    fn check_node(
        &self,
        source: &SourceFile,
        node: &ruby_prism::Node<'_>,
        _parse_result: &ruby_prism::ParseResult<'_>,
        _config: &CopConfig,
        diagnostics: &mut Vec<Diagnostic>,
        _corrections: Option<&mut Vec<crate::correction::Correction>>,
    ) {
        // Look for call nodes that are example groups (describe/context/etc.)
        let call = match node.as_call_node() {
            Some(c) => c,
            None => return,
        };

        let name = call.name().as_slice();
        if !is_example_group(name) {
            return;
        }

        let block = match call.block() {
            Some(b) => b,
            None => return,
        };
        let block_node = match block.as_block_node() {
            Some(b) => b,
            None => return,
        };
        let body = match block_node.body() {
            Some(b) => b,
            None => return,
        };
        let stmts = match body.as_statements_node() {
            Some(s) => s,
            None => return,
        };

        // Collect subject declarations in this group's direct body
        let mut subject_calls: Vec<(usize, usize, usize)> = Vec::new(); // (line, col, end_offset)

        for stmt in stmts.body().iter() {
            if let Some(c) = stmt.as_call_node() {
                let m = c.name().as_slice();
                if (m == b"subject" || m == b"subject!") && c.receiver().is_none() {
                    let loc = c.location();
                    let (line, col) = source.offset_to_line_col(loc.start_offset());
                    let end_off = loc.end_offset();
                    subject_calls.push((line, col, end_off));
                }
            }
        }

        if subject_calls.len() <= 1 {
            return;
        }

        // Flag all except the last one
        for &(line, col, _end_off) in &subject_calls[..subject_calls.len() - 1] {
            diagnostics.push(self.diagnostic(
                source,
                line,
                col,
                "Do not set more than one subject per example group".to_string(),
            ));
        }
    }
}

fn is_example_group(name: &[u8]) -> bool {
    matches!(
        name,
        b"describe"
            | b"context"
            | b"feature"
            | b"example_group"
            | b"xdescribe"
            | b"xcontext"
            | b"xfeature"
            | b"fdescribe"
            | b"fcontext"
            | b"ffeature"
    )
}

#[cfg(test)]
mod tests {
    use super::*;

    crate::cop_fixture_tests!(MultipleSubjects, "cops/rspec/multiple_subjects");
}

RuboCop Ruby Implementation (ground truth)

vendor/rubocop-rspec/lib/rubocop/cop/rspec/multiple_subjects.rb

# frozen_string_literal: true

module RuboCop
  module Cop
    module RSpec
      # Checks if an example group defines `subject` multiple times.
      #
      # @example
      #   # bad
      #   describe Foo do
      #     subject(:user) { User.new }
      #     subject(:post) { Post.new }
      #   end
      #
      #   # good
      #   describe Foo do
      #     let(:user) { User.new }
      #     subject(:post) { Post.new }
      #   end
      #
      #   # bad (does not support autocorrection)
      #   describe Foo do
      #     subject!(:user) { User.new }
      #     subject!(:post) { Post.new }
      #   end
      #
      #   # good
      #   describe Foo do
      #     before do
      #       User.new
      #       Post.new
      #     end
      #   end
      #
      # This cop does not support autocorrection in some cases.
      # The autocorrect behavior for this cop depends on the type of
      # duplication:
      #
      #   - If multiple named subjects are defined then this probably indicates
      #     that the overwritten subjects (all subjects except the last
      #     definition) are effectively being used to define helpers. In this
      #     case they are replaced with `let`.
      #
      #   - If multiple unnamed subjects are defined though then this can *only*
      #     be dead code and we remove the overwritten subject definitions.
      #
      #   - If subjects are defined with `subject!` then we don't autocorrect.
      #     This is enough of an edge case that people can just move this to
      #     a `before` hook on their own
      #
      class MultipleSubjects < Base
        extend AutoCorrector
        include RangeHelp

        MSG = 'Do not set more than one subject per example group'

        def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
          return unless example_group?(node)

          subjects = RuboCop::RSpec::ExampleGroup.new(node).subjects

          subjects[0...-1].each do |subject|
            add_offense(subject) do |corrector|
              autocorrect(corrector, subject)
            end
          end
        end

        private

        def autocorrect(corrector, subject)
          return unless subject.method_name.equal?(:subject) # Ignore `subject!`

          if named_subject?(subject)
            rename_autocorrect(corrector, subject)
          else
            remove_autocorrect(corrector, subject)
          end
        end

        def named_subject?(node)
          node.send_node.arguments?
        end

        def rename_autocorrect(corrector, node)
          corrector.replace(node.send_node.loc.selector, 'let')
        end

        def remove_autocorrect(corrector, node)
          range = range_by_whole_lines(node.source_range,
                                       include_final_newline: true)
          corrector.remove(range)
        end
      end
    end
  end
end

RuboCop Test Excerpts

vendor/rubocop-rspec/spec/rubocop/cop/rspec/multiple_subjects_spec.rb

  it 'registers an offense for every overwritten subject' do

    expect_offense(<<~RUBY)
      describe 'hello there' do
        subject(:foo) { 1 }
        ^^^^^^^^^^^^^^^^^^^ Do not set more than one subject per example group
        subject(:bar) { 2 }
        ^^^^^^^^^^^^^^^^^^^ Do not set more than one subject per example group
        subject { 3 }
        ^^^^^^^^^^^^^ Do not set more than one subject per example group
        subject(:baz) { 4 }

        describe 'baz' do
          subject(:norf) { 1 }
        end
      end
    RUBY

      describe 'hello there' do

        describe 'baz' do

  it 'does not try to autocorrect subject!' do

  it 'does not flag shared example groups' do

    expect_no_offenses(<<~RUBY)
      describe Foo do
        it_behaves_like 'user' do
          subject { described_class.new(user, described_class) }

          it { expect(subject).not_to be_accessible }
        end

        it_behaves_like 'admin' do
          subject { described_class.new(user, described_class) }

          it { expect(subject).to be_accessible }
        end
      end
    RUBY

  it 'autocorrects' do

    expect_offense(<<~RUBY)
      describe 'hello there' do
        subject { 1 }
        ^^^^^^^^^^^^^ Do not set more than one subject per example group
        subject { 2 }
        ^^^^^^^^^^^^^ Do not set more than one subject per example group
        subject { 3 }
      end
    RUBY

      describe 'hello there' do

Current Fixture: offense.rb

tests/fixtures/cops/rspec/multiple_subjects/offense.rb

describe 'hello there' do
  subject(:foo) { 1 }
  ^^^^^^^^^^^^^^^^^^^ RSpec/MultipleSubjects: Do not set more than one subject per example group
  subject(:bar) { 2 }
  ^^^^^^^^^^^^^^^^^^^ RSpec/MultipleSubjects: Do not set more than one subject per example group
  subject { 3 }
  ^^^^^^^^^^^^^ RSpec/MultipleSubjects: Do not set more than one subject per example group
  subject(:baz) { 4 }

  describe 'baz' do
    subject(:norf) { 1 }
  end
end

Current Fixture: no_offense.rb

tests/fixtures/cops/rspec/multiple_subjects/no_offense.rb

describe Foo do
  it_behaves_like 'user' do
    subject { described_class.new(user, described_class) }

    it { expect(subject).not_to be_accessible }
  end

  it_behaves_like 'admin' do
    subject { described_class.new(user, described_class) }

    it { expect(subject).to be_accessible }
  end
end

@6 6 bot force-pushed the fix/rspec-multiple_subjects-23409148441 branch from e256ecc to 843668b Compare March 22, 2026 18:16
@6 6 bot marked this pull request as ready for review March 22, 2026 18:16
@6 6 bot enabled auto-merge (rebase) March 22, 2026 18:16
@6 6 closed this Mar 22, 2026
auto-merge was automatically disabled March 22, 2026 21:22

Pull request was closed

@6 6 deleted the fix/rspec-multiple_subjects-23409148441 branch March 24, 2026 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant