Fix Rule

Fix a Lint Rule

This skill takes a rule with known conformance deltas and fixes it. The core judgment call on every delta is: is ShellCheck right or is shuck right? When shuck is wrong, the fix belongs at whatever layer actually caused the problem — not hacked into the rule just to satisfy the oracle.

The hard architectural constraint

Before you change anything, internalize this: rule files in crates/shuck-linter/src/rules/ must not parse, scan, or walk anything. They are cheap filters over precomputed facts and semantic data. All structural discovery lives in lower layers:

• Tokenizing/parsing source → crates/shuck-parser
• Bindings, references, scopes, CFG, dataflow → crates/shuck-semantic
• Normalized commands, words, redirects, tests, pipelines, loops, surface fragments, etc. → crates/shuck-linter/src/facts.rs and src/facts/

Rule files must not:

• Walk or recurse through AST nodes (no walk_commands, iter_commands, manual child recursion).
• Re-parse or re-scan checker.source() to discover shell structure.
• Normalize commands, classify words/tests/redirects, parse command options, or otherwise recompute what the fact builder is responsible for.
• Import from crate::rules::common::* (rule-facing types come from the crate root or a rule-local helper module).
• Reference AST traversal types blocked by the architecture test in src/rules/mod.rs (WordPart, ConditionalExpr, iter_commands, query::, etc.).

If a fix tempts you to do any of those things in the rule file, stop and push the work down a layer. See crates/shuck-linter/AGENTS.md for the full contract.

Before you start

Read these files to understand the rule and its current state:

1. The rule's YAML definition: docs/rules/CXXX.yaml
2. The rule implementation: crates/shuck-linter/src/rules/{category}/{name}.rs
3. The rule's facts (if any): crates/shuck-linter/src/facts.rs — search for the rule name
4. The bug document (if one exists): docs/bugs/CXXX.md
5. Existing corpus-metadata (if any): crates/shuck/tests/testdata/corpus-metadata/cXXX.yaml
6. The linter architecture contract: crates/shuck-linter/AGENTS.md

Step 1: Set up the worktree

If you're in a git worktree (not the main worktree), the large corpus cache won't exist. Symlink it from the main worktree so you don't re-download everything:

make ensure-cache

This is idempotent — safe to run even if .cache already exists.

Step 2: Run the corpus test for this rule

Run a targeted comparison for just the rule in question:

make test-large-corpus SHUCK_LARGE_CORPUS_RULES=CXXX

The test will likely fail — the failure output IS the data you need. Capture and read the full output.

If the full run takes too long or produces overwhelming output, start with a sample:

make test-large-corpus SHUCK_LARGE_CORPUS_RULES=CXXX SHUCK_LARGE_CORPUS_SAMPLE_PERCENT=10

Reading the output

Each failure block shows a fixture where shuck and ShellCheck disagree:

compatibility diff (code + location):
  SCXXXX: shellcheck=N shuck=M
  labels:
    location-only | directive-handling | shellcheck-parse-abort | ...
  shellcheck diagnostics:
    SC1234 line:col-endline:endcol level message
  shuck diagnostics:
    CXXX=>SCXXXX line:col-endline:endcol severity message

Key signals:

• shellcheck=N shuck=0 — shuck is under-reporting (missing detections)
• shellcheck=0 shuck=M — shuck is over-reporting (false positives)
• shellcheck=N shuck=N but different lines — span anchoring issue
• shellcheck-parse-abort label — ShellCheck bailed on a parse error; delta is unreliable
• project-closure label — delta depends on source/dot resolution differences

Step 3: Investigate each delta pattern

For each failing fixture, read the actual script source to understand what the code is doing. Find the script in the corpus:

find .cache/large-corpus -name "fixture_name_here"

Then read the script around the lines mentioned in the diagnostics. Group failures by root cause — most deltas cluster into a few distinct patterns.

The core judgment: who is right?

For each pattern, decide:

ShellCheck is right, shuck is wrong — shuck has a bug. Proceed to Step 4.

ShellCheck is wrong, shuck is right — the oracle is imperfect. Common cases:

• ShellCheck warns on code that's actually safe (e.g., variables consumed by a sourced script, dynamic parameter expansion patterns)
• ShellCheck's parse aborted and it emitted partial/incorrect results
• ShellCheck misunderstands the script's intended shell dialect

For oracle issues, skip to Step 5 to record the divergence so it won't block the corpus test or waste future investigation time.

Genuine disagreement on policy — shuck intentionally diverges from ShellCheck (stricter or more lenient by design). Also goes to Step 5.

To understand ShellCheck's reasoning, run it directly:

nix --extra-experimental-features 'nix-command flakes' develop --command \
  shellcheck --format=json /path/to/script.sh 2>/dev/null | jq '.[] | select(.code == NNNN)'

Step 4: Fix shuck at the right layer

This is the most important part. Fixing at the wrong layer creates tech debt that compounds — a hack in the rule to work around a parser bug means every future rule hitting the same construct will need the same hack. And rules are not allowed to compensate for missing structural data by walking or parsing themselves (see "The hard architectural constraint" above).

Identify the layer

Work from the bottom up. The issue lives at the lowest layer that gets it wrong:

Layer 1 — Lexer (crates/shuck-parser/src/parser/lexer.rs) Symptoms: tokens are wrong, quoting boundaries are off, heredoc content is misattributed. Check by printing the token stream for a minimal reproducer.

Layer 2 — Parser/AST (crates/shuck-parser/src/parser/) Symptoms: the AST structure doesn't match what the script actually says. A command is parsed as the wrong type, redirections are attached to the wrong node, compound commands have wrong boundaries. Check by dumping the AST for a minimal reproducer.

Layer 3 — Semantic model (crates/shuck-semantic/) Symptoms: bindings/references are wrong, scope assignment is off, def-use chains miss a connection. The AST is correct but the semantic analysis misinterprets it. Check by querying the semantic model directly.

Layer 4 — Fact generation (crates/shuck-linter/src/facts.rs and src/facts/) Symptoms: the semantic model is correct but the linter-level facts derived from it are wrong. A test fact misclassifies an operand, a command fact normalizes incorrectly, a needed structural summary doesn't exist yet. Check by inspecting the facts for the failing fixture. If a rule needs structural data that no fact exposes, the fix is to add the fact here — not to walk the AST in the rule.

Layer 5 — Rule implementation (crates/shuck-linter/src/rules/) Symptoms: the facts are correct but the rule's filtering predicate has a gap. It reports a violation where it shouldn't (missing filter) or misses one it should catch (incomplete matching over correct facts). This is the only layer where the fix belongs in the rule file itself, and the fix should still be a filter over existing facts — not new traversal or parsing.