Open
Conversation
94adfe2 to
d925dc8
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/SafeNavigationChainLengthcorpus conformance.Refs #448
Details
Style/SafeNavigationChainLengthResult
Changes
Agent conversation excerpt
Reading
docs/agent-ci.mdfirst, then I’ll inspect the current cop, fixtures, and RuboCop’s reference implementation to isolate the FN pattern before touching code.Reading
docs/agent-ci.mdfirst, then I’ll inspect the current cop, fixtures, and RuboCop’s reference implementation to isolate the FN pattern before touching code.The fixture already contains the representative FN cases, so I’m checking whether the current implementation actually misses them and whether the bug is in receiver-only counting versus Prism parent traversal.
The fixture already contains the representative FN cases, so I’m checking whether the current implementation actually misses them and whether the bug is in receiver-only counting versus Prism parent traversal.
There’s already an uncommitted fixture edit in
offense.rb. I’m leaving it intact and working with it rather than rewriting around it.There’s already an uncommitted fixture edit in
offense.rb. I’m leaving it intact and working with it rather than rewriting around it.I’ve confirmed the failing fixture cases were pre-added. Next I’m waiting on the targeted test run, then I’ll inspect the AST shape for the multiline and argument-contained chains that the current receiver walk likely misses.
I’ve confirmed the failing fixture cases were pre-added. Next I’m waiting on the targeted test run, then I’ll inspect the AST shape for the multiline and argument-contained chains that the current receiver walk likely misses.
I’m checking how parent traversal is exposed in the Prism bindings. If generic parent links are available, the fix should match RuboCop much more closely than the current receiver-only walk.
I’m checking how parent traversal is exposed in the Prism bindings. If generic parent links are available, the fix should match RuboCop much more closely than the current receiver-only walk.
The likely fix is to switch from receiver-only counting to ancestor-based counting that mirrors RuboCop, while still treating Prism block-bearing calls as chain boundaries. I’m verifying exact duplicate/offense behavior with RuboCop before I edit the cop.
The likely fix is to switch from receiver-only counting to ancestor-based counting that mirrors RuboCop, while still treating Prism block-bearing calls as chain boundaries. I’m verifying exact duplicate/offense behavior with RuboCop before I edit the cop.
The missed cases split into two categories now: nested chains across transparent Prism wrappers like
ArgumentsNode, and chains that currently get cut off by block-pass syntax like&:inspect, which RuboCop does not treat as a boundary. I’m confirming the exact Prism node types before patching.The missed cases split into two categories now: nested chains across transparent Prism wrappers like
ArgumentsNode, and chains that currently get cut off by block-pass syntax like&:inspect, which RuboCop does not treat as a boundary. I’m confirming the exact Prism node types before patching.Plan:
BlockNodes as chain boundaries.Plan:
BlockNodes as chain boundaries.Task prompt (6190 tokens)
Fix Style/SafeNavigationChainLength — 0 FP, 40 FN
Instructions
You are fixing ONE cop in nitrocop, a Rust Ruby linter that uses Prism for parsing.
Current state: 960 matches, 0 false positives, 40 false negatives.
Focus on: FN (RuboCop flags code nitrocop misses).
⚠ 960 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/safe_navigation_chain_length/offense.rbwith^annotationtests/fixtures/cops/style/safe_navigation_chain_length/no_offense.rbcargo test --lib -- cop::style::safe_navigation_chain_lengthsrc/cop/style/safe_navigation_chain_length.rscargo test --lib -- cop::style::safe_navigation_chain_length///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/SafeNavigationChainLength: <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/safe_navigation_chain_length.rsandtests/fixtures/cops/style/safe_navigation_chain_length/cargo test --lib -- cop::style::safe_navigation_chain_lengthto verify your fix (do NOT run the full test suite)python3 scripts/check_cop.py Style/SafeNavigationChainLength --rerun --clone --sample 15before committing to catch regressionsgit stashCurrent Fixture: offense.rb
tests/fixtures/cops/style/safe_navigation_chain_length/offense.rbCurrent Fixture: no_offense.rb
tests/fixtures/cops/style/safe_navigation_chain_length/no_offense.rbKey Source Files
src/cop/style/safe_navigation_chain_length.rsvendor/rubocop/lib/rubocop/cop/style/safe_navigation_chain_length.rbvendor/rubocop/spec/rubocop/cop/style/safe_navigation_chain_length_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/SafeNavigationChainLength --repos-onlypython3 scripts/investigate_cop.py Style/SafeNavigationChainLength --contextpython3 scripts/verify_cop_locations.py Style/SafeNavigationChainLengthTop FN repos:
inspec__inspec-azure__4d8eaea(4 FN) — examplelibraries/azure_role_definition.rb:34activemerchant__active_merchant__ca45b1b(3 FN) — examplelib/active_merchant/billing/gateways/cecabank/cecabank_json.rb:105seek4science__seek__573bc1a(3 FN) — exampleapp/models/concerns/workflow_extraction.rb:151Representative FN examples:
Betterment__delayed__00d575c: lib/delayed/priority.rb:76— Avoid safe navigation chains longer than 2 calls.MarkUsProject__Markus__41d3df9: app/controllers/results_controller.rb:706— Avoid safe navigation chains longer than 2 calls.MarkUsProject__Markus__41d3df9: app/controllers/submissions_controller.rb:882— Avoid safe navigation chains longer than 2 calls.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:
Betterment__delayed__00d575c: lib/delayed/priority.rb:76NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 70:
def names=(names)) > class body (line 59:class << self) > class body (line 2:class Priority < Numeric) > module body (line 1:module Delayed)Message:
Avoid safe navigation chains longer than 2 calls.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #2:
MarkUsProject__Markus__41d3df9: app/controllers/results_controller.rb:706NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 705:
def view_token_param) > class body (line 1:class ResultsController < ApplicationController)Prism AST at offense line:
StatementsNode > OrNode > CallNode > CallNode > ArgumentsNode > SymbolNode > CallNode > CallNode > CallNode > ArgumentsNode > StringNode > ArgumentsNode > CallNode > CallNode > CallNodeMessage:
Avoid safe navigation chains longer than 2 calls.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #3:
MarkUsProject__Markus__41d3df9: app/controllers/submissions_controller.rb:882NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: if branch (line 881:
if !record.nil?) > method body (line 880:def view_token_param) > class body (line 3:class SubmissionsController < ApplicationController)Prism AST at offense line:
StatementsNode > OrNode > CallNode > CallNode > ArgumentsNode > SymbolNode > CallNode > CallNode > CallNode > ArgumentsNode > StringNode > ArgumentsNode > CallNode > CallNode > CallNode > CallNodeMessage:
Avoid safe navigation chains longer than 2 calls.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #4:
Pistos__diakonos__e284f5d: lib/diakonos/version.rb:6NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 5:
def self.parse_version(s)) > module body (line 1:module Diakonos)Prism AST at offense line:
StatementsNode > CallNode > CallNode > CallNode > LocalVariableReadNodeMessage:
Avoid safe navigation chains longer than 2 calls.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #5:
Shopify__shipit-engine__ff8282b: test/helpers/hooks_helper.rb:28NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 20:
def expect_no_hook(*args)) > module body (line 3:module HooksHelper)Message:
Avoid safe navigation chains longer than 2 calls.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #6:
Shopify__spoom__3d124a0: test/spoom/cli/srb/sigs_test.rb:502NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 473:
def test_export_check_sync_raises_if_rbi_is_not_up_to_date) > class body (line 9:class SigsTest < TestWithProject) > module body (line 8:module Srb) > module body (line 7:module Cli) > module body (line 6:module Spoom)Message:
Avoid safe navigation chains longer than 2 calls.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #7:
TelosLabs__rails-world__44c01d6: app/builders/application_form_builder.rb:71NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 70:
def attribute_has_error?(attribute)) > class body (line 1:class ApplicationFormBuilder < ActionView::Helpers::FormBuilder)Message:
Avoid safe navigation chains longer than 2 calls.Ready-made test snippet (add to offense.rb, adjust
^count):Full file context (30 lines before offense):
FN #8:
activemerchant__active_merchant__ca45b1b: lib/active_merchant/billing/gateways/cecabank/cecabank_json.rb:105NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.
Full-file enclosing chain: method body (line 102:
def decrypt_sensitive_fields(data)) > class body (line 5:class CecabankJsonGateway < Gateway) > module body (line 4:module Billing) > module body (line 3:module ActiveMerchant)Message:
Avoid safe navigation chains longer than 2 calls.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.