Skip to content

[bot] Fix RSpec/MessageExpectation#150

Merged
6[bot] merged 2 commits intomainfrom
fix/rspec-message_expectation-23472704443
Mar 24, 2026
Merged

[bot] Fix RSpec/MessageExpectation#150
6[bot] merged 2 commits intomainfrom
fix/rspec-message_expectation-23472704443

Conversation

@6
Copy link
Copy Markdown
Contributor

@6 6 bot commented Mar 24, 2026

Automated fix fix for RSpec/MessageExpectation corpus conformance.

Details

Cop RSpec/MessageExpectation
Backend claude-oauth / normal
Model Claude Sonnet (OAuth)
Mode fix
Turns 16
Run #70

Result

Done. Here's a summary of the fix:

**Root cause:** `expect { ... }.to receive(:stop)` was falsely flagged. In Prism's AST, `expect { ... }` is a `CallNode` with a `block` field set, so `receiver.as_call_node()` succeeded and the cop flagged it. In RuboCop's Parser AST, `expect { ... }` becomes a `block` node (not a `send` node), so the pattern `$(send nil? {:expect :allow} ...)` never matches.

**Fix:** Added an early return when `recv_call.block().is_some()` — i.e., when `expect`/`allow` is called with a block rather than an argument.

Changes

  2 files changed
Agent conversation excerpt

ToolSearch

Read: /home/runner/work/nitrocop/nitrocop/docs/agent-ci.md

Now I understand the FP. The source context shows:

expect {
  ...
}.to receive(:stop)

Here expect { ... } is called with a block, not an argument. In RuboCop's Parser AST, expect { ... }.to receive(:stop) has a block node as the receiver of .to, so RuboCop's pattern $(send nil? {:expect :allow} ...) does NOT match — it requires a send node. But in Prism's AST, expect { ... } is a CallNode with a block attached, so nitrocop's receiver.as_call_node() succeeds and incorrectly flags it.

Fix: skip when the receiver call has a block attached.

Let me verify with RuboCop first, then add the test and fix.

Bash: echo 'expect { subject }.to receive(:stop)' > /tmp/test_fp.rb && rubocop --only RSpec/MessageExpectation /tmp/test_fp.rb 2>&1

RuboCop does NOT flag expect { ... }.to receive(:stop). Confirmed FP.

Now let me add the test case and fix:

Read: /home/runner/work/nitrocop/nitrocop/tests/fixtures/cops/rspec/message_expectation/no_offense.rb

ToolSearch

Edit: /home/runner/work/nitrocop/nitrocop/tests/fixtures/cops/rspec/message_expectation/no_offense.rb

Now verify the test fails before the fix:

Bash: cargo test --lib -- cop::rspec::message_expectation 2>&1 | tail -20

Test fails as expected. Now fix the cop — skip when the receiver call has a block attached:

Edit: /home/runner/work/nitrocop/nitrocop/src/cop/rspec/message_expectation.rs

Read: /home/runner/work/nitrocop/nitrocop/src/cop/rspec/message_expectation.rs

Edit: /home/runner/work/nitrocop/nitrocop/src/cop/rspec/message_expectation.rs

Now verify tests pass:

Bash: cargo test --lib -- cop::rspec::message_expectation 2>&1 | tail -15

All 4 tests pass. Now update the doc comment on the struct and run cargo fmt:

Read: /home/runner/work/nitrocop/nitrocop/src/cop/rspec/message_expectation.rs

Edit: /home/runner/work/nitrocop/nitrocop/src/cop/rspec/message_expectation.rs

Bash: cargo fmt -- src/cop/rspec/message_expectation.rs 2>&1

Now commit:

Bash: `git add src/cop/rspec/message_expectation.rs tests/fixtures/cops/rspec/message_expectation/no_offense.rb && git commit -m "$(cat <<'EOF'
Fix RSpec/MessageExpectation FP: expect { block }.to receive

I`

Done. Here's a summary of the fix:

Root cause: expect { ... }.to receive(:stop) was falsely flagged. In Prism's AST, expect { ... } is a CallNode with a block field set, so receiver.as_call_node() succeeded and the cop flagged it. In RuboCop's Parser AST, expect { ... } becomes a block node (not a send node), so the pattern $(send nil? {:expect :allow} ...) never matches.

Fix: Added an early return when recv_call.block().is_some() — i.e., when expect/allow is called with a block rather than an argument.

Task prompt (4360 tokens)

Fix RSpec/MessageExpectation — 1 FP, 0 FN

Cross-Corpus Risk

  • Standard corpus: 40,503 matches, 0 FP, 0 FN
  • Extended corpus: 77,088 matches, 1 FP, 0 FN
  • Extended divergence currently touches 1 repo

Risk class: extended-only edge case against a standard-perfect baseline.

Treat the extended examples as a narrow edge case, not proof that the broad pattern is safe.
Any carve-out that reduces the extended FP/FN count but regresses the standard corpus is a bad fix.
Prefer the smallest context-specific change you can justify from RuboCop behavior and the vendor spec.
This is concentrated in only a few repos, which increases the risk of overfitting to one local shape.

Instructions

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

Current state: 77,088 matches, 1 false positives, 0 false negatives.
Focus on: FP (nitrocop flags code RuboCop does not).

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/MessageExpectation /tmp/test.rb
    echo '<general pattern>' > /tmp/test.rb && rubocop --only RSpec/MessageExpectation /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/message_expectation/offense.rb with ^ annotation
    • FP fix: add the false-positive pattern to tests/fixtures/cops/rspec/message_expectation/no_offense.rb
  4. Verify test fails: cargo test --lib -- cop::rspec::message_expectation
  5. Fix src/cop/rspec/message_expectation.rs
  6. Verify test passes: cargo test --lib -- cop::rspec::message_expectation
  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/MessageExpectation: Trailing whitespace detected.

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

IMPORTANT: This is a config/context issue, NOT a detection bug

Pre-diagnostic shows nitrocop already detects all FP/FN patterns correctly in isolation.
The corpus mismatches are caused by configuration differences in target repos.

Do NOT loop trying to fix detection logic — the detection code is correct.

Instead:

  1. Investigate why the cop doesn't fire (FN) or fires incorrectly (FP) in the target
    repo's config context. Common causes:
    • Include/Exclude patterns in the cop's config not matching the file path
    • The cop being disabled by the target repo's .rubocop.yml
    • # rubocop:disable comments in the source file
    • File path patterns (e.g., spec files excluded by default)
  2. Look at src/config/ for how config affects this cop
  3. If you can fix the config resolution, do so. Otherwise document your findings as a
    /// comment on the cop struct and commit what you have.

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/MessageExpectation /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/message_expectation.rs and tests/fixtures/cops/rspec/message_expectation/
  • Run cargo test --lib -- cop::rspec::message_expectation to verify your fix (do NOT run the full test suite)
  • Do NOT touch unrelated files
  • Do NOT use git stash

Start Here

Use the existing corpus data to focus on the most concentrated regressions first.

Helpful local commands:

  • python3 scripts/investigate-cop.py RSpec/MessageExpectation --repos-only
  • python3 scripts/investigate-cop.py RSpec/MessageExpectation --context
  • python3 scripts/verify-cop-locations.py RSpec/MessageExpectation

Top FP repos:

  • nats-io__nats-pure.rb__b484a05 (1 FP) — example spec/service_spec.rb:146

Representative FP examples:

  • nats-io__nats-pure.rb__b484a05: spec/service_spec.rb:146 — Prefer allow for setting message expectations.

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.

  • FP: 0 confirmed code bug(s), 1 context-dependent

FP #1: nats-io__nats-pure.rb__b484a05: spec/service_spec.rb:146

NOT REPRODUCED in isolation — CONTEXT-DEPENDENT
nitrocop does not flag this in isolation. The FP is triggered
by surrounding code context or file-level state.
Investigate what full-file context causes the false detection.

Source context:

    xcontext "when an error occurs during setup" do
      before do
        allow(client).to receive(:subscribe).and_raise("Error during subscribe")
        allow(service).to receive(:stop)
      end

      it "stops the service" do
        expect {
          begin
            subject
          rescue
            nil
          end
        }.to receive(:stop)
      end

Message: Prefer allow for setting message expectations.

Current Rust Implementation

src/cop/rspec/message_expectation.rs

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

/// ## Corpus investigation (2026-03-14)
///
/// FP=1 (rpush/rpush): `expect(fake_http2_request).to receive(:on).with(:close), &on_close`
/// was flagged. RuboCop's NodePattern `(send ... :to #receive_message?)` only matches
/// when `.to` has exactly one argument (no block_pass). When `&proc` is passed as a
/// block argument, the Parser AST adds it as a `block_pass` child of the send node,
/// making the pattern not match. Fixed by checking if `.to` has a BlockArgumentNode.
///
/// ## Corpus investigation (2026-03-15)
///
/// FN=43: Missed `expect(...).to all(receive(...))` and similar patterns where
/// `receive` is nested inside matcher arguments (e.g., `all`, compound matchers)
/// rather than being at the root of the receiver chain. RuboCop uses
/// `def_node_search :receive_message?` which does a full subtree search, while
/// nitrocop only walked the receiver chain. Fixed by replacing
/// `call_chain_includes_receive` with `subtree_includes_receive` that recursively
/// searches receiver, arguments, and nested call nodes.
pub struct MessageExpectation;

/// Default style is `allow` — flags `expect(...).to receive` in favor of `allow`.
impl Cop for MessageExpectation {
    fn name(&self) -> &'static str {
        "RSpec/MessageExpectation"
    }

    fn default_enabled(&self) -> bool {
        false
    }

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

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

    fn interested_node_types(&self) -> &'static [u8] {
        &[CALL_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>>,
    ) {
        // Config: EnforcedStyle — "allow" (default) or "expect"
        let enforced_style = config.get_str("EnforcedStyle", "allow");

        // Look for: expect(foo).to receive(:bar)
        // The pattern is a call chain: expect(foo).to(receive(:bar))
        // We flag the `expect(...)` part.
        let call = match node.as_call_node() {
            Some(c) => c,
            None => return,
        };

        let method_name = call.name().as_slice();
        if method_name != b"to" {
            return;
        }

        // If .to has a &proc block argument, skip — RuboCop's NodePattern
        // (send ... :to #receive_message?) requires exactly one argument with no
        // block_pass. A block_pass adds an extra child to the send node, preventing
        // the pattern from matching.
        if call
            .block()
            .is_some_and(|b| b.as_block_argument_node().is_some())
        {
            return;
        }

        // Check the argument is `receive` or similar
        let args = match call.arguments() {
            Some(a) => a,
            None => return,
        };
        let arg_list: Vec<_> = args.arguments().iter().collect();
        if arg_list.is_empty() {
            return;
        }

        let first_arg = &arg_list[0];
        if !subtree_includes_receive(first_arg) {
            return;
        }

        // Check that the receiver of `.to` is `expect(...)` (not `allow(...)`)
        let receiver = match call.receiver() {
            Some(r) => r,
            None => return,
        };
        let recv_call = match receiver.as_call_node() {
            Some(c) => c,
            None => return,
        };

        let recv_name = recv_call.name().as_slice();
        if recv_call.receiver().is_some() {
            return;
        }

        if enforced_style == "expect" {
            // "expect" style: flag `allow(...).to receive(...)`, prefer `expect`
            if recv_name != b"allow" {
                return;
            }
            let loc = recv_call.location();
            let (line, column) = source.offset_to_line_col(loc.start_offset());
            diagnostics.push(self.diagnostic(
                source,
                line,
                column,
                "Prefer `expect` for setting message expectations.".to_string(),
            ));
        } else {
            // Default "allow" style: flag `expect(...).to receive(...)`, prefer `allow`
            if recv_name != b"expect" {
                return;
            }
            let loc = recv_call.location();
            let (line, column) = source.offset_to_line_col(loc.start_offset());
            diagnostics.push(self.diagnostic(
                source,
                line,
                column,
                "Prefer `allow` for setting message expectations.".to_string(),
            ));
        }
    }
}

/// Deep-search a node subtree for `receive(...)` (a bare `receive` call with no
/// receiver). This mirrors RuboCop's `def_node_search :receive_message?` which
/// searches the entire subtree, not just the receiver chain. This matters for
/// patterns like `expect(foo).to all(receive(:bar))` where `receive` is nested
/// inside the argument of `all`, not in the receiver chain.
fn subtree_includes_receive(node: &ruby_prism::Node<'_>) -> bool {
    if let Some(call) = node.as_call_node() {
        if call.name().as_slice() == b"receive" && call.receiver().is_none() {
            return true;
        }
        // Recurse into receiver chain
        if let Some(recv) = call.receiver() {
            if subtree_includes_receive(&recv) {
                return true;
            }
        }
        // Recurse into arguments (handles `all(receive(...))` etc.)
        if let Some(args) = call.arguments() {
            for arg in args.arguments().iter() {
                if subtree_includes_receive(&arg) {
                    return true;
                }
            }
        }
    }
    false
}

#[cfg(test)]
mod tests {
    use super::*;
    crate::cop_fixture_tests!(MessageExpectation, "cops/rspec/message_expectation");

    #[test]
    fn expect_style_flags_allow_receive() {
        use crate::cop::CopConfig;
        use std::collections::HashMap;

        let config = CopConfig {
            options: HashMap::from([(
                "EnforcedStyle".into(),
                serde_yml::Value::String("expect".into()),
            )]),
            ..CopConfig::default()
        };
        let source = b"allow(foo).to receive(:bar)\n";
        let diags = crate::testutil::run_cop_full_with_config(&MessageExpectation, source, config);
        assert_eq!(diags.len(), 1);
        assert!(diags[0].message.contains("expect"));
    }

    #[test]
    fn expect_style_does_not_flag_expect_receive() {
        use crate::cop::CopConfig;
        use std::collections::HashMap;

        let config = CopConfig {
            options: HashMap::from([(
                "EnforcedStyle".into(),
                serde_yml::Value::String("expect".into()),
            )]),
            ..CopConfig::default()
        };
        let source = b"expect(foo).to receive(:bar)\n";
        let diags = crate::testutil::run_cop_full_with_config(&MessageExpectation, source, config);
        assert!(diags.is_empty());
    }
}

RuboCop Ruby Implementation (ground truth)

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

# frozen_string_literal: true

module RuboCop
  module Cop
    module RSpec
      # Checks for consistent message expectation style.
      #
      # This cop can be configured in your configuration using the
      # `EnforcedStyle` option and supports `--auto-gen-config`.
      #
      # @example `EnforcedStyle: allow` (default)
      #
      #   # bad
      #   expect(foo).to receive(:bar)
      #
      #   # good
      #   allow(foo).to receive(:bar)
      #
      # @example `EnforcedStyle: expect`
      #
      #   # bad
      #   allow(foo).to receive(:bar)
      #
      #   # good
      #   expect(foo).to receive(:bar)
      #
      class MessageExpectation < Base
        include ConfigurableEnforcedStyle

        MSG = 'Prefer `%<style>s` for setting message expectations.'

        RESTRICT_ON_SEND = %i[to].freeze

        # @!method message_expectation(node)
        def_node_matcher :message_expectation, <<~PATTERN
          (send $(send nil? {:expect :allow} ...) :to #receive_message?)
        PATTERN

        # @!method receive_message?(node)
        def_node_search :receive_message?, '(send nil? :receive ...)'

        def on_send(node)
          message_expectation(node) do |match|
            return correct_style_detected if preferred_style?(match)

            message = format(MSG, style: style)
            add_offense(match.loc.selector, message: message) do
              opposite_style_detected
            end
          end
        end

        private

        def preferred_style?(expectation)
          expectation.method_name.equal?(style)
        end
      end
    end
  end
end

RuboCop Test Excerpts

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

  context 'when EnforcedStyle is allow' do

    it 'flags expect(...).to receive' do

      expect_offense(<<~RUBY)
        expect(foo).to receive(:bar)
        ^^^^^^ Prefer `allow` for setting message expectations.
      RUBY

    it 'approves of allow(...).to receive' do

  context 'when EnforcedStyle is expect' do

    it 'flags allow(...).to receive' do

      expect_offense(<<~RUBY)
        allow(foo).to receive(:bar)
        ^^^^^ Prefer `expect` for setting message expectations.
      RUBY

    it 'approves of expect(...).to receive' do

Current Fixture: offense.rb

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

expect(foo).to receive(:bar)
^^^^^^ RSpec/MessageExpectation: Prefer `allow` for setting message expectations.
expect(foo).to receive(:baz).with(1)
^^^^^^ RSpec/MessageExpectation: Prefer `allow` for setting message expectations.
expect(obj).to receive(:qux).and_return(true).at_least(:once)
^^^^^^ RSpec/MessageExpectation: Prefer `allow` for setting message expectations.
expect(items).to all receive(:process)
^^^^^^ RSpec/MessageExpectation: Prefer `allow` for setting message expectations.
expect(items).to all(receive(:process).with(:arg))
^^^^^^ RSpec/MessageExpectation: Prefer `allow` for setting message expectations.

Current Fixture: no_offense.rb

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

allow(foo).to receive(:bar)
allow(foo).to receive(:baz).with(1)
allow(obj).to receive(:qux).and_return(true)
expect(foo).to eq(bar)
expect(foo).to have_received(:bar)
allow(foo).to receive(:bar).and_return(42)
expect(obj).not_to receive(:qux)
expect(obj).to_not receive(:qux)
# &proc block argument: RuboCop's NodePattern (send ... :to receive?) doesn't match
# when there's a block_pass child (extra argument in Parser AST)
expect(fake_request).to receive(:on).with(:close), &on_close
allow(items).to all receive(:process)
allow(items).to all(receive(:process).with(:arg))

@6 6 bot force-pushed the fix/rspec-message_expectation-23472704443 branch from 7491c3f to 2ccf9c7 Compare March 24, 2026 04:23
@6 6 bot marked this pull request as ready for review March 24, 2026 04:26
@6 6 bot enabled auto-merge (squash) March 24, 2026 04:26
@6 6 bot merged commit d29b46f into main Mar 24, 2026
4 checks passed
@6 6 bot deleted the fix/rspec-message_expectation-23472704443 branch March 24, 2026 04:31
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