Skip to content

[bot] Fix Style/MixinUsage#129

Closed
6[bot] wants to merge 1 commit intomainfrom
fix/style-mixin_usage-23405918144
Closed

[bot] Fix Style/MixinUsage#129
6[bot] wants to merge 1 commit intomainfrom
fix/style-mixin_usage-23405918144

Conversation

@6
Copy link
Copy Markdown
Contributor

@6 6 bot commented Mar 22, 2026

Status: Agent is working on this fix...

Cop: Style/MixinUsage | Backend: codex | Mode: fix
Code bugs: 4 | Run: https://github.com/6/nitrocop/actions/runs/23405918144

Task prompt (5551 tokens)

Fix Style/MixinUsage — 3 FP, 0 FN

Instructions

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

Current state: 2,834 matches, 3 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 Style/MixinUsage /tmp/test.rb
    echo '<general pattern>' > /tmp/test.rb && rubocop --only Style/MixinUsage /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/style/mixin_usage/offense.rb with ^ annotation
    • FP fix: add the false-positive pattern to tests/fixtures/cops/style/mixin_usage/no_offense.rb
  4. Verify test fails: cargo test --lib -- cop::style::mixin_usage
  5. Fix src/cop/style/mixin_usage.rs
  6. Verify test passes: cargo test --lib -- cop::style::mixin_usage
  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
     ^^ Style/MixinUsage: Trailing whitespace detected.

The ^ characters must align with the offending columns. The message format is Style/MixinUsage: <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 Style/MixinUsage /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/style/mixin_usage.rs and tests/fixtures/cops/style/mixin_usage/
  • Run cargo test --lib -- cop::style::mixin_usage to verify your fix (do NOT run the full test suite)
  • Do NOT touch unrelated files
  • Do NOT use git stash

Prism Notes

  • const splits into ConstantReadNode (simple Foo) and ConstantPathNode (qualified Foo::Bar). If you handle one, check if you need the other.

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: 3 confirmed code bug(s), 0 context-dependent

FP #1: ged__linguistics__b0b119c: experiments/lafcadio_plural.rb:13

CONFIRMED false positive — CODE BUG
nitrocop incorrectly flags this pattern in isolation.
Fix the detection logic to not flag this.

Enclosing structure: BEGIN {} block (Prism: PreExecutionNode) (line: BEGIN {)
The offense is inside this structure — this is likely WHY
RuboCop does not flag it. Your fix should detect this context.

Full source context (add relevant parts to no_offense.rb):

#

BEGIN {
	base = File::dirname( File::dirname(File::expand_path(__FILE__)) )
	$LOAD_PATH.unshift "#{base}/lib"

	require "#{base}/utils.rb"
	include UtilityFunctions

	require 'linguistics'
}

$yaml = false
Linguistics::use( :en )

Message: ``includeis used at the top level. Use insideclass` or `module`.`

FP #2: ged__linguistics__b0b119c: experiments/lprintf.rb:17

CONFIRMED false positive — CODE BUG
nitrocop incorrectly flags this pattern in isolation.
Fix the detection logic to not flag this.

Enclosing structure: BEGIN {} block (Prism: PreExecutionNode) (line: BEGIN {)
The offense is inside this structure — this is likely WHY
RuboCop does not flag it. Your fix should detect this context.

Full source context (add relevant parts to no_offense.rb):

#

BEGIN {
	base = File::dirname( File::dirname(File::expand_path(__FILE__)) )
	$LOAD_PATH.unshift "#{base}/lib"

	require "#{base}/utils.rb"
	include UtilityFunctions
}

require 'linguistics'

Linguistics::use( :en, :classes => [String,Array] )

module Linguistics::EN

Message: ``includeis used at the top level. Use insideclass` or `module`.`

FP #3: ged__linguistics__b0b119c: experiments/conjunct-with-block.rb:13

CONFIRMED false positive — CODE BUG
nitrocop incorrectly flags this pattern in isolation.
Fix the detection logic to not flag this.

Enclosing structure: BEGIN {} block (Prism: PreExecutionNode) (line: BEGIN {)
The offense is inside this structure — this is likely WHY
RuboCop does not flag it. Your fix should detect this context.

Full source context (add relevant parts to no_offense.rb):

#

BEGIN {
	base = File::dirname( File::dirname(File::expand_path(__FILE__)) )
	$LOAD_PATH.unshift "#{base}/lib"

	require "#{base}/utils.rb"
	include UtilityFunctions
}

require 'linguistics'

Linguistics::use( :en, :installProxy => true )
array = %w{sheep shrew goose bear penguin barnacle sheep goose goose}

Message: ``includeis used at the top level. Use insideclass` or `module`.`

Current Rust Implementation

src/cop/style/mixin_usage.rs

use ruby_prism::Visit;

use crate::cop::{Cop, CopConfig};
use crate::diagnostic::Diagnostic;
use crate::parse::source::SourceFile;

/// Corpus investigation (round 1): 14+ FPs from `include T('default/layout/html')` in YARD
/// templates. Root cause: we checked `node.arguments().is_some()` which matches any argument
/// including method calls. RuboCop's node pattern requires arguments to be `const` nodes.
/// Fixed by verifying all arguments are ConstantReadNode or ConstantPathNode before flagging.
///
/// Corpus investigation (round 2): 6 FPs from `include M` inside `while`, `until`, `for`,
/// `case`, and lambda/proc blocks at the top level. Root cause: nitrocop only tracked
/// `in_class_or_module` and `in_block` (BlockNode) as scope barriers, but missed other
/// constructs. RuboCop's `in_top_level_scope?` pattern only considers `begin`, `kwbegin`,
/// `if`, and `def` as transparent wrappers — everything else (while, until, for, case,
/// lambda, etc.) creates an opaque scope. Fixed by replacing the opt-out approach with an
/// opt-in approach: only transparent nodes (if, def, begin) pass through the top-level flag.
///
/// Corpus investigation (round 3): 6 FPs from `include`/`extend`/`prepend` inside
/// `begin...rescue` or `begin...ensure` blocks at the top level. In RuboCop's Parser AST,
/// `begin...rescue...end` wraps the body in a `rescue` node, making it opaque (not in the
/// transparent `{kwbegin begin if def}` list). In Prism, statements are direct children
/// of `BeginNode`. Fixed by overriding `visit_begin_node` to mark the scope as opaque when
/// `rescue_clause` or `ensure_clause` is present. Plain `begin...end` remains transparent.
///
/// Corpus investigation (round 4): 2 FPs from `include GravatarHelper, GravatarHelper::PublicMethods, ERB::Util`
/// in redmine forks. Root cause: RuboCop's node pattern `(send nil? ${:include :extend :prepend} const)`
/// matches exactly ONE `const` argument. Multi-argument mixin calls like `include A, B, C`
/// don't match the pattern and are not flagged. nitrocop was incorrectly accepting any number
/// of const arguments. Fixed by requiring exactly one argument in the const check.
pub struct MixinUsage;

const MIXIN_METHODS: &[&[u8]] = &[b"include", b"extend", b"prepend"];

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

    fn check_source(
        &self,
        source: &SourceFile,
        parse_result: &ruby_prism::ParseResult<'_>,
        _code_map: &crate::parse::codemap::CodeMap,
        _config: &CopConfig,
        diagnostics: &mut Vec<Diagnostic>,
        _corrections: Option<&mut Vec<crate::correction::Correction>>,
    ) {
        let mut visitor = MixinUsageVisitor {
            cop: self,
            source,
            diagnostics: Vec::new(),
            in_opaque_scope: false,
        };
        visitor.visit(&parse_result.node());
        diagnostics.extend(visitor.diagnostics);
    }
}

struct MixinUsageVisitor<'a> {
    cop: &'a MixinUsage,
    source: &'a SourceFile,
    diagnostics: Vec<Diagnostic>,
    /// True when we're inside a scope that is NOT considered "top level" by RuboCop.
    /// RuboCop's `in_top_level_scope?` only treats `begin`, `kwbegin`, `if`, and `def`
    /// as transparent wrappers. Everything else (class, module, block, while, until,
    /// for, case, lambda, etc.) creates an opaque scope where mixin calls are allowed.
    in_opaque_scope: bool,
}

impl<'pr> Visit<'pr> for MixinUsageVisitor<'_> {
    fn visit_call_node(&mut self, node: &ruby_prism::CallNode<'pr>) {
        let method_bytes = node.name().as_slice();

        if MIXIN_METHODS.contains(&method_bytes)
            && node.receiver().is_none()
            && !self.in_opaque_scope
        {
            // RuboCop's node pattern `(send nil? ${:include :extend :prepend} const)`
            // matches exactly ONE `const` argument. Multi-argument calls like
            // `include A, B, C` don't match, nor do method call arguments like
            // `include T('...')`.
            let is_single_const_mixin = node.arguments().is_some_and(|args| {
                let arguments: Vec<_> = args.arguments().iter().collect();
                arguments.len() == 1
                    && (arguments[0].as_constant_read_node().is_some()
                        || arguments[0].as_constant_path_node().is_some())
            });

            if is_single_const_mixin {
                let method_str = std::str::from_utf8(method_bytes).unwrap_or("include");
                let loc = node.location();
                let (line, column) = self.source.offset_to_line_col(loc.start_offset());
                self.diagnostics.push(self.cop.diagnostic(
                    self.source,
                    line,
                    column,
                    format!(
                        "`{method_str}` is used at the top level. Use inside `class` or `module`."
                    ),
                ));
            }
        }

        // Visit children
        if let Some(recv) = node.receiver() {
            self.visit(&recv);
        }
        if let Some(args) = node.arguments() {
            for arg in args.arguments().iter() {
                self.visit(&arg);
            }
        }
        if let Some(block) = node.block() {
            self.visit(&block);
        }
    }

    // === Transparent wrappers (RuboCop considers these still "top level") ===
    // `begin`/`kwbegin`, `if`, and `def` are transparent.
    // No need to override visit_if_node or visit_def_node —
    // the default traversal descends into children without changing in_opaque_scope.
    //
    // However, `begin...rescue...end` and `begin...ensure...end` are special:
    // In RuboCop's Parser AST, the `rescue`/`ensure` node becomes the parent of
    // the body statements, and `rescue`/`ensure` is NOT in the transparent list.
    // So we must treat BeginNode with rescue/ensure as opaque.
    fn visit_begin_node(&mut self, node: &ruby_prism::BeginNode<'pr>) {
        let has_rescue_or_ensure = node.rescue_clause().is_some() || node.ensure_clause().is_some();
        if has_rescue_or_ensure {
            let prev = self.in_opaque_scope;
            self.in_opaque_scope = true;
            ruby_prism::visit_begin_node(self, node);
            self.in_opaque_scope = prev;
        } else {
            // Plain `begin...end` without rescue/ensure is transparent
            ruby_prism::visit_begin_node(self, node);
        }
    }

    // === Opaque scopes (mixin calls inside these are NOT top-level) ===

    fn visit_class_node(&mut self, node: &ruby_prism::ClassNode<'pr>) {
        let prev = self.in_opaque_scope;
        self.in_opaque_scope = true;
        if let Some(body) = node.body() {
            self.visit(&body);
        }
        self.in_opaque_scope = prev;
    }

    fn visit_module_node(&mut self, node: &ruby_prism::ModuleNode<'pr>) {
        let prev = self.in_opaque_scope;
        self.in_opaque_scope = true;
        if let Some(body) = node.body() {
            self.visit(&body);
        }
        self.in_opaque_scope = prev;
    }

    fn visit_singleton_class_node(&mut self, node: &ruby_prism::SingletonClassNode<'pr>) {
        let prev = self.in_opaque_scope;
        self.in_opaque_scope = true;
        if let Some(body) = node.body() {
            self.visit(&body);
        }
        self.in_opaque_scope = prev;
    }

    fn visit_block_node(&mut self, node: &ruby_prism::BlockNode<'pr>) {
        let prev = self.in_opaque_scope;
        self.in_opaque_scope = true;
        if let Some(body) = node.body() {
            self.visit(&body);
        }
        self.in_opaque_scope = prev;
    }

    fn visit_lambda_node(&mut self, node: &ruby_prism::LambdaNode<'pr>) {
        let prev = self.in_opaque_scope;
        self.in_opaque_scope = true;
        if let Some(body) = node.body() {
            self.visit(&body);
        }
        self.in_opaque_scope = prev;
    }

    fn visit_while_node(&mut self, node: &ruby_prism::WhileNode<'pr>) {
        let prev = self.in_opaque_scope;
        self.in_opaque_scope = true;
        ruby_prism::visit_while_node(self, node);
        self.in_opaque_scope = prev;
    }

    fn visit_until_node(&mut self, node: &ruby_prism::UntilNode<'pr>) {
        let prev = self.in_opaque_scope;
        self.in_opaque_scope = true;
        ruby_prism::visit_until_node(self, node);
        self.in_opaque_scope = prev;
    }

    fn visit_for_node(&mut self, node: &ruby_prism::ForNode<'pr>) {
        let prev = self.in_opaque_scope;
        self.in_opaque_scope = true;
        ruby_prism::visit_for_node(self, node);
        self.in_opaque_scope = prev;
    }

    fn visit_case_node(&mut self, node: &ruby_prism::CaseNode<'pr>) {
        let prev = self.in_opaque_scope;
        self.in_opaque_scope = true;
        ruby_prism::visit_case_node(self, node);
        self.in_opaque_scope = prev;
    }

    fn visit_case_match_node(&mut self, node: &ruby_prism::CaseMatchNode<'pr>) {
        let prev = self.in_opaque_scope;
        self.in_opaque_scope = true;
        ruby_prism::visit_case_match_node(self, node);
        self.in_opaque_scope = prev;
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    crate::cop_fixture_tests!(MixinUsage, "cops/style/mixin_usage");
}

RuboCop Ruby Implementation (ground truth)

vendor/rubocop/lib/rubocop/cop/style/mixin_usage.rb

# frozen_string_literal: true

module RuboCop
  module Cop
    module Style
      # Checks that `include`, `extend` and `prepend` statements appear
      # inside classes and modules, not at the top level, so as to not affect
      # the behavior of `Object`.
      #
      # @example
      #   # bad
      #   include M
      #
      #   class C
      #   end
      #
      #   # bad
      #   extend M
      #
      #   class C
      #   end
      #
      #   # bad
      #   prepend M
      #
      #   class C
      #   end
      #
      #   # good
      #   class C
      #     include M
      #   end
      #
      #   # good
      #   class C
      #     extend M
      #   end
      #
      #   # good
      #   class C
      #     prepend M
      #   end
      class MixinUsage < Base
        MSG = '`%<statement>s` is used at the top level. Use inside `class` or `module`.'
        RESTRICT_ON_SEND = %i[include extend prepend].freeze

        # @!method include_statement(node)
        def_node_matcher :include_statement, <<~PATTERN
          (send nil? ${:include :extend :prepend}
            const)
        PATTERN

        # @!method in_top_level_scope?(node)
        def_node_matcher :in_top_level_scope?, <<~PATTERN
          {
            root?                        # either at the top level
            ^[  {kwbegin begin if def}   # or wrapped within one of these
                #in_top_level_scope? ]   # that is in top level scope
          }
        PATTERN

        def on_send(node)
          include_statement(node) do |statement|
            return unless in_top_level_scope?(node)

            add_offense(node, message: format(MSG, statement: statement))
          end
        end
      end
    end
  end
end

RuboCop Test Excerpts

vendor/rubocop/spec/rubocop/cop/style/mixin_usage_spec.rb

  context 'include' do

    it 'registers an offense when using outside class (used above)' do

      expect_offense(<<~RUBY)
        include M
        ^^^^^^^^^ `include` is used at the top level. Use inside `class` or `module`.
        class C
        end
      RUBY

    it 'registers an offense when using outside class (used below)' do

      expect_offense(<<~RUBY)
        class C
        end
        include M
        ^^^^^^^^^ `include` is used at the top level. Use inside `class` or `module`.
      RUBY

    it 'registers an offense when using only `include` statement' do

      expect_offense(<<~RUBY)
        include M
        ^^^^^^^^^ `include` is used at the top level. Use inside `class` or `module`.
      RUBY

    it 'registers an offense when using `include` in method definition outside class or module' do

      expect_offense(<<~RUBY)
        def foo
          include M
          ^^^^^^^^^ `include` is used at the top level. Use inside `class` or `module`.
        end
      RUBY

    it 'does not register an offense when using outside class' do

      expect_no_offenses(<<~RUBY)
        Foo.include M
        class C; end
      RUBY

    it 'does not register an offense when using inside class' do

      expect_no_offenses(<<~RUBY)
        class C
          include M
        end
      RUBY

    it 'does not register an offense when using inside block' do

      expect_no_offenses(<<~RUBY)
        Class.new do
          include M
        end
      RUBY

    it 'does not register an offense when using inside block ' \

      expect_no_offenses(<<~RUBY)
        klass.class_eval do
          include M1
          include M2 if defined?(M)
        end
      RUBY

    it "doesn't register an offense when `include` call is a method argument" do

      expect_no_offenses(<<~RUBY)
        do_something(include(M))
      RUBY

    it 'does not register an offense when using `include` in method definition inside class' do

Current Fixture: offense.rb

tests/fixtures/cops/style/mixin_usage/offense.rb

include M
^^^^^^^^^ Style/MixinUsage: `include` is used at the top level. Use inside `class` or `module`.

extend N
^^^^^^^^ Style/MixinUsage: `extend` is used at the top level. Use inside `class` or `module`.

prepend O
^^^^^^^^^ Style/MixinUsage: `prepend` is used at the top level. Use inside `class` or `module`.

# Transparent wrappers: include inside def/if/begin at top level is still flagged
def foo
  include M
  ^^^^^^^^^ Style/MixinUsage: `include` is used at the top level. Use inside `class` or `module`.
end

if condition
  extend N
  ^^^^^^^^ Style/MixinUsage: `extend` is used at the top level. Use inside `class` or `module`.
end

begin
  prepend O
  ^^^^^^^^^ Style/MixinUsage: `prepend` is used at the top level. Use inside `class` or `module`.
end

include M1::M2::M3
^^^^^^^^^^^^^^^^^^ Style/MixinUsage: `include` is used at the top level. Use inside `class` or `module`.

Current Fixture: no_offense.rb

tests/fixtures/cops/style/mixin_usage/no_offense.rb

class C
  include M
end
module N
  extend O
end
Foo.include M
Class.new do
  include M
end
obj.include(M)
# Method call arguments should not be flagged (only constants)
include T('default/layout/html')
extend some_method
prepend build_module(:foo)
# include inside while/until/for/case/lambda at top level is NOT flagged by RuboCop
while condition
  include M
end
until done
  extend N
end
for x in items
  prepend O
end
case foo
when :bar
  include M
end
-> { include M }
proc { include M }
# include inside begin/rescue at top level is NOT flagged by RuboCop
begin
  include M
rescue LoadError
  nil
end
begin
  require 'something'
  include M
rescue LoadError => e
  puts e
end
# include inside if inside begin/ensure at top level
begin
  if condition
    include M
  end
ensure
  cleanup
end
# Multiple constant arguments: RuboCop's pattern matches only a single const
include GravatarHelper, GravatarHelper::PublicMethods, ERB::Util
extend A, B
prepend X, Y, Z

@6
Copy link
Copy Markdown
Contributor Author

6 bot commented Mar 22, 2026

Agent failed. See run: https://github.com/6/nitrocop/actions/runs/23405918144

@6 6 bot closed this Mar 22, 2026
@6 6 bot deleted the fix/style-mixin_usage-23405918144 branch March 22, 2026 15:21
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.

0 participants