Add Sdlc/Dry cop for repeated method skeleton #1321
Labels
No labels
P0
P1
P2
P3
bug
category/code-change
category/cross-repo
category/eval-run
category/investigation
category/no-test-target
category/owner-ruling
category/prose-artifact
category/recurring-chore
category/structure
category/umbrella
crc-refactor
create
delete
enhancement
filed-by/agent
filed-by/user
frozen
lint-rule
needs-info
needs-triage
next
plugin/cc-architect
plugin/os
plugin/os-adr
plugin/os-aidd-lint
plugin/os-backlog
plugin/os-context
plugin/os-doc-hygiene
plugin/os-sdlc
plugin/os-vault
project/cc-os
ready-for-agent
ready-for-human
recurring
review
rule-proposal
update
waiting
wayfinder:grilling
wayfinder:map
wayfinder:map
wayfinder:research
wayfinder:task
wayfinder:task
wontfix
worklist/deviations
worklist/lint-rule
worklist/new-implement-build
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
jared/cc-os#1321
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
Metrics/ClassLength fires on files whose real fault is the same method skeleton written many times with one literal changed. No Sdlc cop names that shape. The agent gets a generic length lesson and often splits the file the wrong way.
The pre-fix os-backlog file (
git show 7c228093:plugins/os-backlog/lib/backlog/cli_command_wiring.rb, 279 lines) has this shape. The layered config does not catch it. Sdlc/Dry/DuplicatedStatementSequence does not fire, because it compares 2-statement windows by exact AST equality and the repeated skeleton differs by one string literal. Also, a def whose body is a method-level rescue has a body of one rescue node, so it has no 2-statement window at all (smell analysis section 7).Jared's standard: "We want the solution to always be correct so we know the correction is warranted and we can reduce flags to zero. Otherwise we would always have unresolved violations. We need to know exactly what shape triggers that fix and that the fix is always correct for that shape, as well as exactly when the shape could trigger a false positive and why it is false so we can carve those out and avoid flagging them."
Reproduce
n/a: new cop. The red fixture is the pre-fix file above.
Approach
Build the cop from the smell analysis in docs/plans/class-length-family/smell-repeated-method-skeleton.md. The cop matches one shape. The shape, the fix, and each false-positive carve-out come from that file.
The cop name is
Sdlc/Dry/RepeatedRescueClause(smell analysis section 8), file plugins/os-sdlc/lib/os_sdlc/cops/repeated_rescue_clause.rb. The exact matcher is section 9 (SCOPE, MEMBER M1-M6, SKELETON, GROUP, FIRE, MESSAGE). The carve-outs are section 13 (C1-C13). The fixtures are section 16 (red R1-R14, green G1-G21).Carve-outs live in the cop matcher, by shape. Do not add config Exclude lists. Do not exclude test paths.
Jared reviewed the smell analysis on 2026-10-25 (section 17). Q1: accept that differing error lists become a wrapper argument. Q2: extend
Sdlc/Structural/RescueSiteto recordrescue *errorssites before this cop ships. The ticket is not blocked.Q2 reading: RescueSite records error constants (Detector::ErrorLineage::RescueIndex counts them). When a rescue list entry is
*nameandnameis the rest parameter of the enclosing def W, record each constant argument at or after the rest-parameter position of every receiverless call to W in the same file, with formrescueand the handler sends. When that call forwards a rest parameter of its own enclosing def V (W(name, *errors, &)), apply the same rule to the calls to V. Real case on main: plugins/os-backlog/lib/backlog/cli/issue_commands.rb (tracker_command:75,issue_command:70).The cop fires on main on day one: plugins/os-backlog/lib/backlog/cli/project_commands.rb has 4 members (
ensure_labels:31,validate_issue_create:95,create_issue:102,list_issues:114; 2 different error lists, so fix form B). Fix them in this ticket with the section 10 fix.Required work:
RepeatedRescueClauseinplugins/os-sdlc/lib/os_sdlc/cops/*.rbrescue *errorssites (Q2 reading above), with tests in plugins/os-sdlc/tests/cops/rescue_site_test.rb — check: presentrescue *errorsinplugins/os-sdlc/tests/cops/rescue_site_test.rbtracker_commandshares it (spec follow-up 1), because the revert of line-golf edits made Metrics/ClassLength fire on ProjectCommands — check: presentrescue *errorsinplugins/os-backlog/lib/backlog/cli/commands.rbAcceptance criteria
7c228093file shape).w(name, *errors)withrescue *errors => eand a callw("x", A, B::C)in the same file, when RescueSite runs, then it recordsAandB::C.Owner sketch
RepeatedRescueClause (plugins/os-sdlc/lib/os_sdlc/cops/repeated_rescue_clause.rb) walks every class, module,
class << self, and block body as one scope, so a per-nodeon_classhook is not enough. Register it in therequire:list and asSdlc/Dry/RepeatedRescueClause: Enabled: truein plugins/os-sdlc/.rubocop.yml. RescueSite (plugins/os-sdlc/lib/os_sdlc/cops/rescue_site.rb) gains therescue *errorsrecord inon_resbody. Commands (plugins/os-backlog/lib/backlog/cli/commands.rb) gains one private wrapper, shared by ProjectCommands andtracker_command.Origin
VERDICT: CREATE
Blocked by Jared's review of docs/plans/class-length-family/smell-repeated-method-skeleton.md (no ticket number): the cop is built from that analysis.
Rulings (Jared, 2026-10-25, accepted prediction): Q1 accept: differing error lists become a wrapper argument; constants are evaluated before the body runs. Q2 extend RescueSite to record
rescue *errorssites before this cop ships.Independent blind check (sonnet, own finder from section 9 only): 2 groups, 18 methods, all TRUE, 0 FALSE; matches the analysis.
Spec points to define before TDD (they change matches): (1) the key's 'reference kind' means empty versus listed error list, not the list itself; (2) how the key treats non-string literal arguments such as fail_soft(:a) vs fail_soft(:b); (3) public and private members form one group, the wrapper is private.
Add fixtures: early
returninside a wrapped body; public/private member mix.Spec ready for TDD. Section 9 now defines the three points (list kind only in the key; non-string literals stay in the key; public/private/protected form one group, wrapper private). Section 17 records Q1 (accept) and Q2 (extend RescueSite first). New fixtures R13, R14, G21. Commit
f81efc62.Work started on branch ticket-1321 (os-sdlc implement, AFK run approved 2026-10-25).
Resolution
Done: Cop Sdlc/Dry/RepeatedRescueClause (Member, Handler, Group) flags 3+ methods in one scope whose rescue skeleton differs only in literals or error lists and names the wrapper to extract; RescueSite records rescue *errors constants with one forwarding level (Q2). Fixed the 4 os-backlog hits with Commands#fail_soft_in shared with tracker_command (required-work item 7 updated on the ticket: wrapper on Commands, not ProjectCommands, after reverting line-golf edits made ClassLength fire). os-sdlc-lint exits 3 with usage when --root or --base is missing.
Evidence: main
cb92a1e5. os-sdlc suite 2592 runs, 0 failures; os-backlog 327 runs, 0 failures; cop over plugins/ 2510 files 0 offenses, 18 true hits on pre-fix files; rubocop clean on os-sdlc files. Implementation 162 escalated (75, unexpected_pass); hand-finished with one design audit, its 3 must-fix findings applied. Known lint residue: one Sdlc/Structural/UnusedPrivateMethod false positive on Commands#tracker_command (cop reads one file; same mechanism already on main in that file) recorded on #1325.Follow-ups: #1342 (verifier brief lacks cross-target diff, skeptic CREATE); UnusedPrivateMethod false positive recorded on #1325 (open, zero-FP audit). Dropped: itblock scope (no Ruby 3.4 it-blocks in repo); RescueSource vs CopHelpers duplication (test support, CRC pass will rule).