Open
Conversation
Add detection for the `include?` family of patterns that RuboCop flags
but nitrocop previously missed:
- `hash.reject { |k, _| COLLECTION.include?(k) }` → `except(*COLLECTION)`
- `hash.select { |k, _| !COLLECTION.include?(k) }` → `except(*COLLECTION)`
Handles array literal receivers (expanded args), constants, variables,
and method call receivers (splatted). Correctly skips Range receivers,
value-parameter usage, and inverted semantics (e.g. `reject` with
negated `include?`).
Corpus: 0 FP, 16 FN remaining (down from 189).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6
bot
force-pushed
the
fix/style-hash_except-23730408076
branch
from
af5577f to
20bc6b2
Compare
Contributor
|
✅ Cop-check results (8 shards)
All shards passed — ready to merge. |
Contributor
Author
Auto-repair Started
Task prompt (3683 tokens)PR Repair Task: PR #898Before making changes, read Repair the existing PR branch in place. Do not create a new branch or PR. Context
Required OutcomeFix the currently failing checks shown below. Required Local VerificationThe workflow will rerun these commands before pushing. Your changes should make them pass: cargo clippy --profile ci -- -D warnings
Current PR Diff StatCurrent PR Diff Excerptdiff --git a/src/cop/style/hash_except.rs b/src/cop/style/hash_except.rs
index 580432741..ac1350f92 100644
--- a/src/cop/style/hash_except.rs
+++ b/src/cop/style/hash_except.rs
@@ -6,6 +6,22 @@ use crate::cop::{Cop, CopConfig};
use crate::diagnostic::Diagnostic;
use crate::parse::source::SourceFile;
+/// Detects `Hash#reject`, `Hash#select`, and `Hash#filter` calls that can be
+/// replaced with `Hash#except`.
+///
+/// Handles two families of patterns:
+///
+/// 1. **Comparison patterns** (`==` / `!=`):
+/// - `hash.reject { |k, _| k == :sym }` → `hash.except(:sym)`
+/// - `hash.select { |k, _| k != :sym }` → `hash.except(:sym)`
+/// Only flags string/symbol comparands (mirrors RuboCop safety gate).
+///
+/// 2. **`include?` patterns**:
+/// - `hash.reject { |k, _| COLLECTION.include?(k) }` → `hash.except(*COLLECTION)`
+/// - `hash.select { |k, _| !COLLECTION.include?(k) }` → `hash.except(*COLLECTION)`
+/// Works with array literals (`[:a, :b]`), constants, and variables.
+/// Array literal receivers produce `except(:a, :b)` (expanded);
+/// all others produce `except(*name)` (splatted).
pub struct HashExcept;
impl Cop for HashExcept {
@@ -91,6 +107,13 @@ impl Cop for HashExcept {
};
let key_name = key_param.name().as_slice();
+ // Get the value parameter name (needed for include? checks)
+ let value_param = match requireds[1].as_required_parameter_node() {
+ Some(p) => p,
+ None => return,
+ };
+ let value_name = value_param.name().as_slice();
+
// Check the block body for a simple comparison pattern
let body = match block_node.body() {
Some(b) => b,
@@ -109,46 +132,84 @@ impl Cop for HashExcept {
let expr = &body_nodes[0];
- // Check for k == :sym pattern (reject) or k != :sym pattern (select/filter)
- if let Some(cmp_call) = expr.as_call_node() {
- let cmp_method = cmp_call.name().as_slice();
+ // Try to match the expression against known patterns
+ if let Some(outer_call) = expr.as_call_node() {
+ let outer_method = outer_call.name().as_slice();
- // For reject: k == :sym -> except(:sym)
- // For select/filter: k != :sym -> except(:sym)
- let is_matching = (method_bytes == b"reject" && cmp_method == b"==")
- || ((method_bytes == b"select" || method_bytes == b"filter")
- && cmp_method == b"!=");
+ // Pattern: !SOMETHING.include?(key) — negation wrapper
+ if outer_method == b"!" {
+ if let Some(inner) = outer_call.receiver() {
+ if let Some(inner_call) = inner.as_call_node() {
+ if inner_call.name().as_slice() == b"include?" {
+ // Negated include? is except-like for select/filter only
+ if method_bytes == b"select" || method_bytes == b"filter" {
+ self.check_include_pattern(
+ source,
+ &call,
+ &inner_call,
+ key_name,
+ value_name,
+ diagnostics,
+ );
+ }
+ }
+ }
+ }
+ return;
+ }
- if !is_matching {
+ // Pattern: SOMETHING.include?(key) — for reject only
+ if outer_method == b"include?" {
+ if method_bytes == b"reject" {
+ self.check_include_pattern(
+ source,
+ &call,
+ &outer_call,
+ key_name,
+ value_name,
+ diagnostics,
+ );
+ }
return;
}
- let cmp_recv = match cmp_call.receiver() {
- Some(r) => r,
- None => return,
- };
+ // Pattern: k == :sym / k != :sym (existing logic)
+ if outer_method == b"==" || outer_method == b"!=" {
+ // For reject: k == :sym -> except(:sym)
+ // For select/filter: k != :sym -> except(:sym)
+ let is_matching = (method_bytes == b"reject" && outer_method == b"==")
+ || ((method_bytes == b"select" || method_bytes == b"filter")
+ && outer_method == b"!=");
- let cmp_args = match cmp_call.arguments() {
- Some(a) => a,
- None => return,
- };
+ if !is_matching {
+ return;
+ }
- let cmp_arg_list: Vec<_> = cmp_args.arguments().iter().collect();
- if cmp_arg_list.len() != 1 {
- return;
- }
+ let cmp_recv = match outer_call.receiver() {
+ Some(r) => r,
+ None => return,
+ };
+
+ let cmp_args = match outer_call.arguments() {
+ Some(a) => a,
+ None => return,
+ };
- // One side must be the key param, other must be a literal
- let (is_key_left, value_node) =
- if let Some(lvar) = cmp_recv.as_local_variable_read_node() {
+ let cmp_arg_list: Vec<_> = cmp_args.arguments().iter().collect();
+ if cmp_arg_list.len() != 1 {
+ return;
+ }
+
+ // One side must be the key param, other must be a literal
+ let value_node = if let Some(lvar) = cmp_recv.as_local_variable_read_node() {
if lvar.name().as_slice() == key_name {
- (true, &cmp_arg_list[0])
+ &cmp_arg_list[0]
} else {
return;
}
} else if let Some(lvar) = cmp_arg_list[0].as_local_variable_read_node() {
if lvar.name().as_slice() == key_name {
- (false, &cmp_recv)
+ &cmp_recv
} else {
return;
}
@@ -156,29 +217,125 @@ impl Cop for HashExcept {
return;
};
- let _ = is_key_left;
+ // Value must be a symbol or string literal
+ let is_sym_or_str =
+ value_node.as_symbol_node().is_some() || value_node.as_string_node().is_some();
+
+ if !is_sym_or_str {
+ return;
+ }
- // Value must be a symbol or string literal
- let is_sym_or_str =
- value_node.as_symbol_node().is_some() || value_node.as_string_node().is_some();
+ let value_src = &source.as_bytes()
+ [value_node.location().start_offset()..value_node.location().end_offset()];
+ let value_str = String::from_utf8_lossy(value_src);
- if !is_sym_or_str {
- return;
+ let loc = call.message_loc().unwrap_or_else(|| call.location());
+ let (line, column) = source.offset_to_line_col(loc.start_offset());
+ diagnostics.push(self.diagnostic(
+ source,
+ line,
+ column,
+ format!("Use `except({})` instead.", value_str),
+ ));
}
+ }
+ }
+}
- let value_src = &source.as_bytes()
- [value_node.location().start_offset()..value_node.location().end_offset()];
- let value_str = String::from_utf8_lossy(value_src);
-
- let loc = call.message_loc().unwrap_or_else(|| call.location());
- let (line, column) = source.offset_to_line_col(loc.start_offset());
- diagnostics.push(self.diagnostic(
- source,
- line,
- column,
- format!("Use `except({})` instead.", value_str),
- ));
+impl HashExcept {
+ /// Check and emit an offense for the `include?` pattern.
+ ///
+ /// `include_call` is the `SOMETHING.include?(key)` CallNode.
+ /// `outer_call` is the top-level `reject`/`select`/`filter` CallNode.
+ fn check_include_pattern(
+ &self,
+ source: &SourceFile,
+ outer_call: &ruby_prism::CallNode<'_>,
+ include_call: &ruby_prism::CallNode<'_>,
+ key_name: &[u8],
+ value_name: &[u8],
+ diagnostics: &mut Vec<Diagnostic>,
+ ) {
+ // include? must have exactly one argument
+ let args = match include_call.arguments() {
+ Some(a) => a,
+ None => return,
+ };
+ let arg_list: Vec<_> = args.arguments().iter().collect();
+ if arg_list.len() != 1 {
... (111 more lines truncated)Failed Checks Packetbuild-and-test
Constraints
Available Local Helper ScriptsThese helper scripts are available in this CI checkout. Prefer the stable top-level CLI paths shown below over ad hoc commands when they directly help with diagnosis or validation.
Typical usage when present: python3 scripts/check_cop.py Department/CopName --verbose --rerun --clone
python3 scripts/dispatch_cops.py changed --base origin/main --head HEAD
python3 scripts/investigate_cop.py Department/CopName --context
python3 scripts/verify_cop_locations.py Department/CopName
python3 scripts/corpus_smoke_test.py --binary target/release/nitrocop |
Contributor
Author
Auto-repair Succeeded
Auto-repair succeeded with backend Validated locally before push using |
6
bot
force-pushed
the
fix/style-hash_except-23730408076
branch
from
2a25706 to
cc34c4d
Compare
Contributor
|
✅ Cop-check results (8 shards)
All shards passed — ready to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated fix fix for
Style/HashExceptcorpus conformance.Refs #368
Details
Style/HashExceptResult
Changes
Agent conversation excerpt
Now let me read the HashSubset mixin and the RuboCop spec to understand the full detection patterns.
Now let me understand how diagnostics work and look at how other cops handle span calculation.
Let me check how Prism array nodes work and look at the test utility for fixture formatting.
Now let me check the cop_fixture_tests macro to understand how the test uses fixtures, and verify how
as_range_nodeis available.Good. Now I have all the information needed. Let me implement the changes. First, I'll update the offense.rb fixture with correct test cases, then implement the
include?detection logic.Now let me write the updated offense.rb fixture with correct
^positions for theinclude?patterns.Tests fail as expected (4 detected, 6 new ones missing). Now let me implement the
include?pattern detection.Need to fix a borrow issue -
outer_callfrom pattern match needs&.Both tests pass. Now let me add no_offense cases for
include?edge cases, then validate against the corpus.All tests pass. Now let me validate against the corpus.
Excellent! The corpus check passes with zero regressions. Results:
The doc comment is already on the struct. Let me commit.
Everything looks clean. Let me commit.
Add detection for the
include?family of patterns that RuboCop flagsbut nitrocop previously missed:
-`
Done. Here's a summary of the changes:
Problem: Style/HashExcept had 189 false negatives — all caused by missing
include?pattern detection. The cop only handled==/!=comparisons.Fix: Added detection for two
include?patterns:hash.reject { |k, _| COLLECTION.include?(k) }→except(*COLLECTION)hash.select/filter { |k, _| !COLLECTION.include?(k) }→except(*COLLECTION)Key details:
[:foo, :bar]) produce expanded args:except(:foo, :bar)except(*KEYS){ }anddo...endblock syntaxResults: 0 FP, 16 FN remaining (down from 189) — 173 FN resolved with zero regressions.
Task prompt (7420 tokens)
Fix Style/HashExcept — 0 FP, 189 FN
Instructions
You are fixing ONE cop in nitrocop, a Rust Ruby linter that uses Prism for parsing.
Current state: 192 matches, 0 false positives, 189 false negatives.
Focus on: FN (RuboCop flags code nitrocop misses).
⚠ 192 existing matches must not regress. Validate with
check_cop.pybefore committing.Workflow
behavior on BOTH the specific FP case AND the general pattern:
tests/fixtures/cops/style/hash_except/offense.rbwith^annotationtests/fixtures/cops/style/hash_except/no_offense.rbcargo test --lib -- cop::style::hash_exceptsrc/cop/style/hash_except.rscargo test --lib -- cop::style::hash_except///doc comment on the cop struct documenting what you found and fixedFixture Format
Mark offenses with
^markers on the line AFTER the offending source line.The
^characters must align with the offending columns. The message format isStyle/HashExcept: <message text>.See the Current Fixture sections below for real examples from this cop.
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:
src/config/or the cop's config handling, not detection logica
///comment on the cop struct and commitCRITICAL: 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:
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
returnorcontinuethat skips a wholenode type, operator class, or naming pattern, it's probably too broad. Prefer adding a
condition that matches the SPECIFIC differentiating context.
Rules
src/cop/style/hash_except.rsandtests/fixtures/cops/style/hash_except/cargo test --lib -- cop::style::hash_exceptto verify your fix (do NOT run the full test suite)python3 scripts/check_cop.py Style/HashExcept --rerun --clone --sample 15before committing to catch regressionsgit stashCurrent Fixture: offense.rb
tests/fixtures/cops/style/hash_except/offense.rbCurrent Fixture: no_offense.rb
tests/fixtures/cops/style/hash_except/no_offense.rbKey Source Files
src/cop/style/hash_except.rsvendor/rubocop/lib/rubocop/cop/style/hash_except.rbvendor/rubocop/spec/rubocop/cop/style/hash_except_spec.rbRead these files before making changes.
Start Here
Use the existing corpus data to focus on the most concentrated regressions first.
Helpful local commands:
python3 scripts/investigate_cop.py Style/HashExcept --repos-onlypython3 scripts/investigate_cop.py Style/HashExcept --contextpython3 scripts/verify_cop_locations.py Style/HashExceptTop FN repos:
drhenner__ror_ecommerce__b7142f0(17 FN) — examplespec/controllers/admin/customer_service/comments_controller_spec.rb:34cenit-io__cenit__0106da3(8 FN) — exampleapp/controllers/api/v1/api_controller.rb:402DataDog__dd-trace-rb__e3406de(4 FN) — examplelib/datadog/appsec/contrib/rails/gateway/request.rb:58Representative FN examples:
AndyObtiva__glimmer-dsl-web__fd77e68: lib/rails/resource_service.rb:92— Useexcept(*TIMESTAMP_ATTRIBUTES)instead.Arachni__arachni__c1710d6: lib/arachni/element/capabilities/analyzable/signature.rb:211— Useexcept(*opts[:payload_platforms])instead.Arachni__arachni__c1710d6: lib/arachni/element/dom.rb:86— Useexcept(:@parent, :@page, :@browser, :@element)instead.Pre-diagnostic Results
Diagnosis Summary
Each example was tested by running nitrocop on the extracted source in isolation
with
--force-default-configto 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 #1:
AndyObtiva__glimmer-dsl-web__fd77e68: lib/rails/resource_service.rb:92NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 88:
def create_update_resource_params(resource: nil, resource_class: nil, resource_attributes: nil, singular_resource_name: nil, params: nil)) > class body (line 8:class << self) > class body (line 5:class ResourceService) > module body (line 1:module Rails)Prism AST at offense line:
LocalVariableOrWriteNode > CallNode > CallNode > LocalVariableReadNode > BlockNode > BlockParametersNode > ParametersNode > RequiredParameterNode > RequiredParameterNode > StatementsNode > CallNode > ConstantReadNode > ArgumentsNode > LocalVariableReadNodeMessage:
Useexcept(*TIMESTAMP_ATTRIBUTES)instead.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #2:
Arachni__arachni__c1710d6: lib/arachni/element/capabilities/analyzable/signature.rb:211NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: enclosing line 186:
when Hash> case expression (line 179:case signatures) > method body (line 175:def find_signatures( signatures, response, opts )) > module body (line 16:module Signature)Prism AST at offense line:
CallNode > CallNode > CallNode > BlockNode > BlockParametersNode > ParametersNode > RequiredParameterNode > RequiredParameterNode > StatementsNode > CallNode > CallNode > CallNode > CallNode > ArgumentsNode > SymbolNode > ArgumentsNode > LocalVariableReadNodeMessage:
Useexcept(*opts[:payload_platforms])instead.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #3:
Arachni__arachni__c1710d6: lib/arachni/element/dom.rb:86NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 85:
def marshal_dump) > class body (line 14:class DOM < Base)Message:
Useexcept(:@parent, :@page, :@browser, :@element)instead.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #4:
DataDog__datadog-ci-rb__cadff60: lib/datadog/ci/test_tracing/serializers/base.rb:32NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 27:
def initialize(trace, span, options: {})) > class body (line 11:class Base) > module body (line 10:module Serializers) > module body (line 9:module TestTracing) > module body (line 8:module CI) > module body (line 7:module Datadog)Message:
Useexcept(*Ext::Test::TRANSIENT_TAGS)instead.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #5:
DataDog__dd-trace-rb__e3406de: lib/datadog/appsec/contrib/rails/gateway/request.rb:58NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 55:
def route_params) > class body (line 11:class Request < Instrumentation::Gateway::Argument) > module body (line 9:module Gateway) > module body (line 8:module Rails) > module body (line 7:module Contrib) > module body (line 6:module AppSec) > module body (line 5:module Datadog)Prism AST at offense line:
CallNode > CallNode > CallNode > CallNode > ArgumentsNode > StringNode > BlockNode > BlockParametersNode > ParametersNode > RequiredParameterNode > RequiredParameterNodeMessage:
Useexcept(*excluded)instead.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #6:
DataDog__dd-trace-rb__e3406de: lib/datadog/appsec/contrib/rails/request.rb:28NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 25:
def self.route_params(request)) > module body (line 8:module Request) > module body (line 6:module Rails) > module body (line 5:module Contrib) > module body (line 4:module AppSec) > module body (line 3:module Datadog)Prism AST at offense line:
CallNode > CallNode > CallNode > LocalVariableReadNode > ArgumentsNode > StringNode > BlockNode > BlockParametersNode > ParametersNode > RequiredParameterNode > RequiredParameterNodeMessage:
Useexcept(*excluded)instead.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #7:
DataDog__dd-trace-rb__e3406de: lib/datadog/kit/appsec/events.rb:39NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: block (do..end) (line 32:
set_trace_and_span_context('track_login_success', trace, span) do |active_trace, active_span|) > method body (line 31:def track_login_success(trace = nil, span = nil, user:, **others)) > class body (line 16:class << self) > module body (line 10:module Events) > module body (line 8:module AppSec) > module body (line 7:module Kit) > module body (line 6:module Datadog)Message:
Useexcept(*USER_LOGIN_KEYS)instead.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #8:
DataDog__dd-trace-rb__e3406de: lib/datadog/kit/appsec/events.rb:94NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: block (do..end) (line 87:
set_trace_and_span_context('track_signup', trace, span) do |active_trace, active_span|) > method body (line 86:def track_signup(trace = nil, span = nil, user:, **others)) > class body (line 16:class << self) > module body (line 10:module Events) > module body (line 8:module AppSec) > module body (line 7:module Kit) > module body (line 6:module Datadog)Message:
Useexcept(*USER_LOGIN_KEYS)instead.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
Omitted 7 additional diagnosed FN example(s) for brevity.