jared/cc-os

Watch

Fork 0

Sdlc/Structural/UnreachableBranchAfterGuard: stop guards at receiver-mutating calls and handle unless #1361

New issue

Closed

opened 2026-10-25 21:23:19 +00:00 by jared · 4 comments

jared commented

2026-10-25 21:23:19 +00:00

Owner

Problem

Rewritten 2026-10-27 from docs/lint-lessons/analyses/sdlc-structural-unreachable-branch-after-guard.md.

Sdlc/Structural/UnreachableBranchAfterGuard keeps a guard active after the code changes the guarded variable through a send (q.clear, log(q.shift)) or passes it as an argument (drain(q)). The cop then calls a live branch unreachable. For unless, and for if/ternary with an else, the message says "unreachable" but does not name the dead side, so an author can delete the code that always runs.

Approach

Fix by shape, in the matcher. Use no Exclude list and no list of method names. Rule: docs/lint-lessons/analyses/sdlc-structural-unreachable-branch-after-guard.md step 6.

Required work:

End the guard at the first later statement that, at any depth, sends the variable a message whose name does not end in ?, or passes the variable as an argument to a send or yield.
Make the message depend on the form. For if with no else: "delete the conditional". For if/ternary with else: "the then side is dead; keep the else side". For unless: "the condition always passes; keep the body".
Add must-pass fixtures for clear, nested shift, and argument use, and must-fire fixtures for each message form.

Acceptance criteria

Given a guard on queue.empty?, then queue.clear, then if queue.empty?, when the cop runs, then it reports no offense.
Given a guard on list.empty?, then log(list.shift), then if list.empty?, when the cop runs, then it reports no offense.
Given a guard on list.empty?, then drain(list), then list.empty? ? a : b, when the cop runs, then it reports no offense.
Given a guard on list.empty?, then unless list.empty?, when the cop runs, then the message says the condition always passes and tells the author to keep the body.
Given a guard, then x.empty? ? a : b, when the cop runs, then the message says to keep the else side.
Given a guard and a later identical if with no use of the variable between them, when the cop runs, then it still reports an offense.

Owner sketch

class UnreachableBranchAfterGuard::GuardRelease   # replaces Reassignment
  def self.releases?(statement, receiver) = ...
end
class UnreachableBranchAfterGuard::Branch
  def message = ...                               # names the dead side by form
end

Origin

Trigger: #1325 zero-false-positive audit of every Sdlc cop.
Improvised this session: none
Chain: wrong "unreachable" claim ← reassignment tracking covers lvasgn only, and the message ignores unless polarity ← DESIGN (reassignment.rb:9)
Root candidate: none found; searched UnreachableBranchAfterGuard, unreachable
Where: plugins/os-sdlc/lib/os_sdlc/cops/unreachable_branch_after_guard.rb
Session: eb98cdcd-6783-46a0-b7df-cf18a8fe2a56
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os--claude-worktrees-ticket-1325/eb98cdcd-6783-46a0-b7df-cf18a8fe2a56.jsonl

## Problem Rewritten 2026-10-27 from docs/lint-lessons/analyses/sdlc-structural-unreachable-branch-after-guard.md. `Sdlc/Structural/UnreachableBranchAfterGuard` keeps a guard active after the code changes the guarded variable through a send (`q.clear`, `log(q.shift)`) or passes it as an argument (`drain(q)`). The cop then calls a live branch unreachable. For `unless`, and for `if`/ternary with an `else`, the message says "unreachable" but does not name the dead side, so an author can delete the code that always runs. ## Approach Fix by shape, in the matcher. Use no Exclude list and no list of method names. Rule: `docs/lint-lessons/analyses/sdlc-structural-unreachable-branch-after-guard.md` step 6. Required work: 1. End the guard at the first later statement that, at any depth, sends the variable a message whose name does not end in `?`, or passes the variable as an argument to a send or yield. 2. Make the message depend on the form. For `if` with no else: "delete the conditional". For `if`/ternary with else: "the then side is dead; keep the else side". For `unless`: "the condition always passes; keep the body". 3. Add must-pass fixtures for `clear`, nested `shift`, and argument use, and must-fire fixtures for each message form. ## Acceptance criteria 1. Given a guard on `queue.empty?`, then `queue.clear`, then `if queue.empty?`, when the cop runs, then it reports no offense. 2. Given a guard on `list.empty?`, then `log(list.shift)`, then `if list.empty?`, when the cop runs, then it reports no offense. 3. Given a guard on `list.empty?`, then `drain(list)`, then `list.empty? ? a : b`, when the cop runs, then it reports no offense. 4. Given a guard on `list.empty?`, then `unless list.empty?`, when the cop runs, then the message says the condition always passes and tells the author to keep the body. 5. Given a guard, then `x.empty? ? a : b`, when the cop runs, then the message says to keep the else side. 6. Given a guard and a later identical `if` with no use of the variable between them, when the cop runs, then it still reports an offense. ## Owner sketch ```ruby class UnreachableBranchAfterGuard::GuardRelease # replaces Reassignment def self.releases?(statement, receiver) = ... end class UnreachableBranchAfterGuard::Branch def message = ... # names the dead side by form end ``` ## Origin - Trigger: #1325 zero-false-positive audit of every Sdlc cop. - Improvised this session: none - Chain: wrong "unreachable" claim ← reassignment tracking covers `lvasgn` only, and the message ignores `unless` polarity ← DESIGN (`reassignment.rb:9`) - Root candidate: none found; searched UnreachableBranchAfterGuard, unreachable - Where: plugins/os-sdlc/lib/os_sdlc/cops/unreachable_branch_after_guard.rb - Session: eb98cdcd-6783-46a0-b7df-cf18a8fe2a56 - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os--claude-worktrees-ticket-1325/eb98cdcd-6783-46a0-b7df-cf18a8fe2a56.jsonl

jared added the

plugin/os-sdlc

filed-by/agent

category/code-change

labels

2026-10-25 21:23:19 +00:00

jared commented

2026-10-27 16:38:15 +00:00

Author

Owner

Body rewritten 2026-10-27 from the false-positive process; analysis and lint lesson merged to main (docs/lint-lessons/analyses/, plugins/os-sdlc/lint-lessons/).

jared referenced this issue from a commit

2026-10-27 17:10:02 +00:00

docs(os-sdlc): UnreachableBranchAfterGuard smell analysis and lint lesson (#1361)

jared commented

2026-10-27 22:05:52 +00:00

Author

Owner

Work started on branch ticket-1361 (os-sdlc implement pipeline).

jared referenced this issue from a commit

2026-10-27 22:15:56 +00:00

feat(os-sdlc): POODR audit passes: GuardRelease nested under the cop with one caller, Branch owns its form-based message, contracts tested through the cop public seam. (#1361)

jared referenced this issue from a commit

2026-10-27 22:15:56 +00:00

fix(os-sdlc): check the branch before its own statement releases the guard (#1361)

jared commented

2026-10-27 22:16:56 +00:00

Author

Owner

Resolution

Done: Sdlc/Structural/UnreachableBranchAfterGuard ends a guard at a later statement that sends the variable a non-predicate message (at any depth) or passes it as an argument to a send, yield, or super (GuardRelease composes Reassignment). Branch#message names the live side by form: if without else, if/ternary with else, unless. Orchestrator follow-up commit 5c000777: the branch is checked before its own statement releases the guard, so a use inside the dead branch body no longer hides the offense.

Evidence: Pipeline implementation 202, 8 dispatches all pass (bfe1cd4e); fix 5c000777 with must-fire test; ruby plugins/os-sdlc/tests/all.rb 2842 runs 0 failures (36 s); cop across plugins/: 0 offenses before and after (2751 files); merged to main and pushed (5c000777). Lint lesson YAML already describes the new forms; unchanged.

Follow-ups: Pipeline gate gap captured separately: no gate probed for a new false negative introduced by a cop release rule.

## Resolution **Done:** Sdlc/Structural/UnreachableBranchAfterGuard ends a guard at a later statement that sends the variable a non-predicate message (at any depth) or passes it as an argument to a send, yield, or super (GuardRelease composes Reassignment). Branch#message names the live side by form: if without else, if/ternary with else, unless. Orchestrator follow-up commit 5c000777: the branch is checked before its own statement releases the guard, so a use inside the dead branch body no longer hides the offense. **Evidence:** Pipeline implementation 202, 8 dispatches all pass (bfe1cd4e); fix 5c000777 with must-fire test; ruby plugins/os-sdlc/tests/all.rb 2842 runs 0 failures (36 s); cop across plugins/: 0 offenses before and after (2751 files); merged to main and pushed (5c000777). Lint lesson YAML already describes the new forms; unchanged. **Follow-ups:** Pipeline gate gap captured separately: no gate probed for a new false negative introduced by a cop release rule.

jared closed this issue

2026-10-27 22:16:56 +00:00

jared commented

2026-10-27 22:17:32 +00:00

Author

Owner

Gate-gap note (capture dropped by ticket-skeptic, depth-1): behavior-verifier checks only the listed acceptance criteria, so the new GuardRelease false negative (branch body use released its own guard) passed all 8 gates; fixed by the orchestrator in 5c000777. A cop ticket that adds a suppression rule should list must-fire cases in its acceptance criteria.

Gate-gap note (capture dropped by ticket-skeptic, depth-1): behavior-verifier checks only the listed acceptance criteria, so the new GuardRelease false negative (branch body use released its own guard) passed all 8 gates; fixed by the orchestrator in 5c000777. A cop ticket that adds a suppression rule should list must-fire cases in its acceptance criteria.

jared referenced this issue

2026-10-28 06:14:17 +00:00

Probe must-fire cases of a changed cop against main in the pipeline gates #1454