AILang/docs/plans/0011-22-tidy-strict-clause-helper.md

# 22-tidy.7 strict-clause helper consolidation — Implementation Plan

> **Parent spec:** `docs/specs/0002-22-typeclasses.md` (milestone 22, closed 2026-05-09)
>
> **Trigger:** 22-tidy.5 JOURNAL entry observation that the duplicate-clause check pattern now appears at 17 sites across the parser (10 from 22b.4a + 8 from 22-tidy.5; the JOURNAL entry's "18 sites" was an off-by-one — re-grep confirms 17). 17 × ~12 LOC of identical structure is past the tipping point for the "three similar lines beats a premature abstraction" rule.
>
> **For agentic workers:** REQUIRED SUB-SKILL: use `skills/implement`
> to run this plan. Steps use `- [ ]` checkboxes for tracking.

**Goal:** consolidate the 17 inline `if X.is_some() { return Err(ParseError::Production { … }); }` blocks in `crates/ailang-surface/src/parse.rs` into one helper method `Parser::duplicate_clause_err(&self, production: &'static str, subject: &str, clause: &'static str) -> ParseError`, reducing ~150 LOC of duplication to ~17 one-liner call sites.

**Architecture:** add one method on `Parser` that captures `pos` from `self.peek()` and constructs the `ParseError::Production` shape. Three message templates exist in the current code:
- `"<production> \`<name>\` has duplicate \`(<clause> ...)\` clause"` (13 sites: data, fn, const, class, class method)
- `"instance has duplicate \`(<clause> ...)\` clause"` (3 sites)
- `"instance method \`<name>\` has duplicate \`(<clause> ...)\` clause"` (1 site)

All three are unified under one helper that takes the message *subject* (everything before `" has duplicate"`) as a `&str`, leaving construction at the call site. Caller passes `&format!("fn `{name}`")` for named productions and the literal `"instance"` for the nameless instance form.

**Tech Stack:** `crates/ailang-surface/src/parse.rs` only — one file.

**Files this plan creates or modifies:**

- Modify: `crates/ailang-surface/src/parse.rs` — adds `Parser::duplicate_clause_err` method; rewrites 17 inline blocks (lines 376-385, 497-506, 511-520, 524-533, 537-546, 699-708, 712-721, 725-734, 782-791, 798-807, 811-820, 880-890, 893-903, 947-955, 961-969, 972-980, 1025-1034) into one-line calls.
- Modify: `docs/JOURNAL.md` — append iteration entry.

**Helper signature and body:**

```rust
fn duplicate_clause_err(
    &self,
    production: &'static str,
    subject: &str,
    clause: &'static str,
) -> ParseError {
    let pos = self.peek().map(|t| t.span.start).unwrap_or(0);
    ParseError::Production {
        production,
        message: format!("{subject} has duplicate `({clause} ...)` clause"),
        pos,
    }
}
```

**Call-site shape (post-refactor):**

```rust
// Named production (fn / const / class / class method / data):
if X.is_some() {
    return Err(self.duplicate_clause_err("fn-def", &format!("fn `{name}`"), "doc"));
}

// Instance (no name):
if class.is_some() {
    return Err(self.duplicate_clause_err("instance-def", "instance", "class"));
}

// Instance method (with name):
if body.is_some() {
    return Err(self.duplicate_clause_err("instance.method", &format!("instance method `{name}`"), "body"));
}
```

---

## Task 1: introduce `duplicate_clause_err` and rewrite all 17 call sites

**Files:**
- Modify: `crates/ailang-surface/src/parse.rs` — add helper near other `Parser` methods (suggest: just below `expect_string` at lines 595-610 since both are small private helpers); rewrite 17 inline blocks.

This is a single mechanical refactor — bundling helper introduction with all call-site rewrites in one commit because:
1. The helper has no consumers without the rewrites; an interim commit with helper + zero callers fires `dead_code` warnings.
2. Splitting by parser (data / fn / const / class / instance) creates intermediate commits where the helper exists with partial callers — bisection still works (each call-site rewrite is independent), and the test suite stays green at every commit.
3. The 17 sites are textually parallel; reviewing them as one diff gives the reviewer the apples-to-apples context that a per-parser split would lose.

- [ ] **Step 1: add the helper method**

In `crates/ailang-surface/src/parse.rs`, locate `fn expect_string` (around line 595). Just below it (and above the next helper), add:

```rust
fn duplicate_clause_err(
    &self,
    production: &'static str,
    subject: &str,
    clause: &'static str,
) -> ParseError {
    let pos = self.peek().map(|t| t.span.start).unwrap_or(0);
    ParseError::Production {
        production,
        message: format!("{subject} has duplicate `({clause} ...)` clause"),
        pos,
    }
}
```

- [ ] **Step 2: replace the `parse_data` doc-clause block (line 376-385)**

Locate the block:

```rust
                Some("doc") => {
                    if doc.is_some() {
                        let pos = self.peek().map(|t| t.span.start).unwrap_or(0);
                        return Err(ParseError::Production {
                            production: "data-def",
                            message: format!(
                                "data `{name}` has duplicate `(doc ...)` clause"
                            ),
                            pos,
                        });
                    }
                    doc = Some(self.parse_doc()?);
                }
```

Replace with:

```rust
                Some("doc") => {
                    if doc.is_some() {
                        return Err(self.duplicate_clause_err("data-def", &format!("data `{name}`"), "doc"));
                    }
                    doc = Some(self.parse_doc()?);
                }
```

- [ ] **Step 3: replace `parse_fn`'s 4 clause blocks**

Locate `parse_fn` (around line 485). Replace its four duplicate-clause blocks (`doc`, `type`, `params`, `body`):

```rust
                Some("doc") => {
                    if doc.is_some() {
                        return Err(self.duplicate_clause_err("fn-def", &format!("fn `{name}`"), "doc"));
                    }
                    doc = Some(self.parse_doc()?);
                }
                Some("suppress") => suppress.push(self.parse_suppress_attr()?),
                Some("type") => {
                    if ty.is_some() {
                        return Err(self.duplicate_clause_err("fn-def", &format!("fn `{name}`"), "type"));
                    }
                    ty = Some(self.parse_type_attr()?);
                }
                Some("params") => {
                    if params.is_some() {
                        return Err(self.duplicate_clause_err("fn-def", &format!("fn `{name}`"), "params"));
                    }
                    params = Some(self.parse_params_attr()?);
                }
                Some("body") => {
                    if body.is_some() {
                        return Err(self.duplicate_clause_err("fn-def", &format!("fn `{name}`"), "body"));
                    }
                    body = Some(self.parse_body_attr()?);
                }
```

- [ ] **Step 4: replace `parse_const`'s 3 clause blocks**

Locate `parse_const` (around line 691). Replace its three duplicate-clause blocks (`doc`, `type`, `body`):

```rust
                Some("doc") => {
                    if doc.is_some() {
                        return Err(self.duplicate_clause_err("const-def", &format!("const `{name}`"), "doc"));
                    }
                    doc = Some(self.parse_doc()?);
                }
                Some("type") => {
                    if ty.is_some() {
                        return Err(self.duplicate_clause_err("const-def", &format!("const `{name}`"), "type"));
                    }
                    ty = Some(self.parse_type_attr()?);
                }
                Some("body") => {
                    if value.is_some() {
                        return Err(self.duplicate_clause_err("const-def", &format!("const `{name}`"), "body"));
                    }
                    value = Some(self.parse_body_attr()?);
                }
```

- [ ] **Step 5: replace `parse_class`'s 3 clause blocks**

Locate `parse_class` (around line 771). Replace its three duplicate-clause blocks (`param`, `superclass`, `doc`):

```rust
                Some("param") => {
                    if param.is_some() {
                        return Err(self.duplicate_clause_err("class-def", &format!("class `{name}`"), "param"));
                    }
                    self.expect_lparen("class.param")?;
                    self.expect_keyword("param")?;
                    param = Some(self.expect_ident("class param ident")?);
                    self.expect_rparen("class.param")?;
                }
                Some("superclass") => {
                    if superclass.is_some() {
                        return Err(self.duplicate_clause_err("class-def", &format!("class `{name}`"), "superclass"));
                    }
                    superclass = Some(self.parse_superclass()?);
                }
                Some("doc") => {
                    if doc.is_some() {
                        return Err(self.duplicate_clause_err("class-def", &format!("class `{name}`"), "doc"));
                    }
                    doc = Some(self.parse_doc()?);
                }
```

- [ ] **Step 6: replace `parse_class_method`'s 2 clause blocks**

Locate `parse_class_method` (around line 871). Replace its two duplicate-clause blocks (`type`, `default`):

```rust
                Some("type") => {
                    if ty.is_some() {
                        return Err(self.duplicate_clause_err("class.method", &format!("method `{name}`"), "type"));
                    }
                    ty = Some(self.parse_type_attr()?);
                }
                Some("default") => {
                    if default.is_some() {
                        return Err(self.duplicate_clause_err("class.method", &format!("method `{name}`"), "default"));
                    }
                    self.expect_lparen("class.default")?;
                    self.expect_keyword("default")?;
                    default = Some(self.parse_term()?);
                    self.expect_rparen("class.default")?;
                }
```

(Note: the exact body inside the default-block depends on the existing code. If the inner steps differ, preserve them — this Step 6's job is the `if X.is_some()` guard ONLY, not the rest of the arm. Read lines 880-908 in the source and preserve the non-guard logic verbatim.)

- [ ] **Step 7: replace `parse_instance`'s 3 clause blocks (no-name form)**

Locate `parse_instance` (around line 940). Replace its three duplicate-clause blocks (`class`, `type`, `doc`) — these use the no-name `"instance"` subject:

```rust
                Some("class") => {
                    if class.is_some() {
                        return Err(self.duplicate_clause_err("instance-def", "instance", "class"));
                    }
                    self.expect_lparen("instance.class")?;
                    self.expect_keyword("class")?;
                    class = Some(self.expect_ident("instance class name")?);
                    self.expect_rparen("instance.class")?;
                }
                Some("type") => {
                    if type_.is_some() {
                        return Err(self.duplicate_clause_err("instance-def", "instance", "type"));
                    }
                    self.expect_lparen("instance.type")?;
                    self.expect_keyword("type")?;
                    type_ = Some(self.parse_type()?);
                    self.expect_rparen("instance.type")?;
                }
                Some("doc") => {
                    if doc.is_some() {
                        return Err(self.duplicate_clause_err("instance-def", "instance", "doc"));
                    }
                    doc = Some(self.parse_doc()?);
                }
```

(Same caveat as Step 6: preserve the non-guard logic verbatim if the existing inner body differs.)

- [ ] **Step 8: replace `parse_instance_method`'s 1 clause block**

Locate `parse_instance_method` (around line 1015). Replace its single duplicate-clause block (`body`):

```rust
                Some("body") => {
                    if body.is_some() {
                        return Err(self.duplicate_clause_err("instance.method", &format!("instance method `{name}`"), "body"));
                    }
                    body = Some(self.parse_body_attr()?);
                }
```

(Preserve the non-guard logic if it differs.)

- [ ] **Step 9: build clean**

Run: `cargo build --workspace 2>&1 | tail -5`
Expected: builds clean. If a `dead_code` warning fires on `duplicate_clause_err`, verify all 17 sites were rewritten — the helper is private and needs a consumer.

- [ ] **Step 10: full ailang-surface test sweep stays green**

Run: `cargo test --workspace -p ailang-surface 2>&1 | grep -E "^test result" | tail -5`
Expected: every line `test result: ok.`; no FAILED. The 8 duplicate-clause unit tests added in 22-tidy.5 (4 fn + 3 const + 1 data) exercise the helper transitively; if any of them flips, the helper's message format diverges from what they assert.

- [ ] **Step 11: full workspace test sweep**

Run: `cargo test --workspace 2>&1 | grep -c "FAILED"`
Expected: `0`.

- [ ] **Step 12: confirm net LOC reduction**

Run: `git diff --stat HEAD` (after all replacements, before commit)
Expected: net negative line count on `crates/ailang-surface/src/parse.rs` — adding ~12 lines for the helper and removing ~150 lines (≈ 17 sites × ~9 lines avg saved each).

- [ ] **Step 13: commit**

```bash
git add crates/ailang-surface/src/parse.rs
git commit -m "iter 22-tidy.7: extract duplicate_clause_err helper, retire 17 inline blocks"
```

---

## Task 2: verify + JOURNAL

**Files:**
- Modify: `docs/JOURNAL.md` — append iteration entry.

- [ ] **Step 1: bench gates**

Run: `python3 bench/check.py 2>&1 | tail -1 && python3 bench/compile_check.py 2>&1 | tail -1 && python3 bench/cross_lang.py 2>&1 | tail -1`
Expected: each line ends `0 regressed`.

- [ ] **Step 2: confirm only 1 site retains the inline `ParseError::Production` + `format!("...has duplicate...")` shape, and that site is the helper itself**

Run: `grep -c 'has duplicate \`' crates/ailang-surface/src/parse.rs`
Expected: `1` (the helper's own format-string template) plus the 8 test-assertion `contains(...)` strings = 9 hits if grep counts contains too. Visually inspect: the inline `ParseError::Production { production: ..., message: format!("...has duplicate...") ... }` shape should appear at zero non-helper sites.

- [ ] **Step 3: append JOURNAL entry**

Append to `docs/JOURNAL.md`:

```markdown
## 2026-05-10 — Iteration 22-tidy.7: strict-clause helper consolidation

Closing the queued observation from 22-tidy.5: the
duplicate-clause check pattern had grown to 17 inline blocks
(10 from 22b.4a + 8 from 22-tidy.5) of identical 12-line shape
across `parse_data`, `parse_fn`, `parse_const`, `parse_class`,
`parse_class_method`, `parse_instance`, `parse_instance_method`.
17 × 12 LOC of textual duplication is past the tipping point;
this iter consolidates them.

Introduced one private method
`Parser::duplicate_clause_err(&self, production: &'static str,
subject: &str, clause: &'static str) -> ParseError` that captures
the `pos` lookup, formats the canonical
`"<subject> has duplicate \`(<clause> ...)\` clause"` message, and
returns the `ParseError::Production` shape. All 17 inline blocks
reduced to one-line calls; net ~150 LOC removed.

Three subject shapes are unified under the one helper by passing
the formatted subject as a parameter: `"<production> \`<name>\`"`
for named productions (fn, const, data, class, class method),
literal `"instance"` for the nameless instance form, and
`"instance method \`<name>\`"` for the instance method form. The
production tag stays separate (it's the diagnostic code, not the
human-readable text).

Tasks (commit subjects):

- 22-tidy.7: extract duplicate_clause_err helper, retire 17 inline blocks

Acceptance: `cargo test --workspace` 0 FAILED; the 8
duplicate-clause unit tests from 22-tidy.5 still pass (their
`contains(...)` assertions match the helper's message format
verbatim); bench gates 0/0/0; net LOC reduction on
`crates/ailang-surface/src/parse.rs`.

Observation (out-of-scope, queued): the 9 22b.4a-era duplicate-
clause sites (class × 3 + class method × 2 + instance × 3 +
instance method × 1) have no dedicated unit tests — they were
shipped without a regression pin. The 22-tidy.5 tests cover
fn/const/data only. Backfilling tests for the 22b.4a-era sites is
defensible but beyond the carried-debt scope; queue under
"parser-test backfill" if and when a 22b.4a-era diagnostic ever
needs to change.

Carried-debt status (after this iter):

- ✅ Primitive-name-set consolidation (22-tidy.4).
- ✅ Strict duplicate-clause detection in fn/const/data (22-tidy.5).
- ✅ Form-B (prose) printer arms for ClassDef / InstanceDef
  (22-tidy.6).
- ✅ Strict-clause helper consolidation (this iter).
- ✅ Lib.rs gap-related sites — audit confirmed defensive.

Milestone-22 carried debt + the queued tipping-point observation
are now both closed. Next milestone targets remain
orchestrator's call.
```

- [ ] **Step 4: commit**

```bash
git add docs/JOURNAL.md
git commit -m "iter 22-tidy.7: journal entry"
```