plan: 22-tidy.5 strict duplicate-clause detection

This commit is contained in:
2026-05-10 04:25:11 +02:00
parent e37d099edb
commit 2506180023
@@ -0,0 +1,544 @@
# 22-tidy.5 strict duplicate-clause detection — Implementation Plan
> **Parent spec:** `docs/specs/2026-05-09-22-typeclasses.md` (milestone 22, closed 2026-05-09)
>
> **Carried-debt anchor:** `docs/JOURNAL.md` 2026-05-09 entry "Iteration 22-tidy" → "Strict duplicate-clause detection in `parse_fn` / `parse_data` / `parse_const` (sibling parsers to the 22b.4a additions). The asymmetry is documented; promoting siblings to the strict pattern is queued."
>
> **For agentic workers:** REQUIRED SUB-SKILL: use `skills/implement`
> to run this plan. Steps use `- [ ]` checkboxes for tracking.
**Goal:** retire the strict-vs-loose asymmetry between the class/instance parsers (22b.4a — 10 strict sites) and the fn/const/data parsers (currently silent last-wins on duplicate clauses). Add 8 new strict checks (4 in `parse_fn`, 3 in `parse_const`, 1 in `parse_data`) using the same `if X.is_some() { return ParseError::Production }` pattern, each pinned by an RED-first unit test in `parse.rs`'s `mod tests`.
**Architecture:** the established 22b.4a strict-clause pattern is `if X.is_some() { return Err(ParseError::Production { production, message: "<prod> has duplicate `(<clause> ...)` clause", pos }) }`. This iteration lifts that pattern to the three sibling parsers, mirroring the message form (`"<prod> ...has duplicate ..."`). No new error variants; reuses `ParseError::Production`.
**Tech Stack:** `crates/ailang-surface/src/parse.rs` — three parsers (`parse_fn`, `parse_const`, `parse_data`) and the in-file `mod tests`.
**Files this plan creates or modifies:**
- Modify: `crates/ailang-surface/src/parse.rs` — adds 8 strict checks (4 in `parse_fn` lines 487-499, 3 in `parse_const` lines 650-652, 1 in `parse_data` line 375); adds 8 RED-first unit tests in the `mod tests` block at line 1683.
- Modify: `docs/JOURNAL.md` — append iteration entry.
**Error-message form:** mirror the 22b.4a pattern. For named productions (fn, const, data), use `"<prod> `<name>` has duplicate `(<clause> ...)` clause"`. Verbatim per parser:
| Parser | Clause | Message |
|--------|--------|---------|
| `parse_fn` | `doc` | `` "fn `<name>` has duplicate `(doc ...)` clause" `` |
| `parse_fn` | `type` | `` "fn `<name>` has duplicate `(type ...)` clause" `` |
| `parse_fn` | `params` | `` "fn `<name>` has duplicate `(params ...)` clause" `` |
| `parse_fn` | `body` | `` "fn `<name>` has duplicate `(body ...)` clause" `` |
| `parse_const` | `doc` | `` "const `<name>` has duplicate `(doc ...)` clause" `` |
| `parse_const` | `type` | `` "const `<name>` has duplicate `(type ...)` clause" `` |
| `parse_const` | `body` | `` "const `<name>` has duplicate `(body ...)` clause" `` |
| `parse_data` | `doc` | `` "data `<name>` has duplicate `(doc ...)` clause" `` |
---
## Task 1: `parse_fn` strict duplicate-clause detection (4 sites)
**Files:**
- Modify: `crates/ailang-surface/src/parse.rs` — `parse_fn` body at lines 485-513; `mod tests` at line 1683 (append four tests).
- [ ] **Step 1: write 4 RED unit tests at the END of `mod tests` (just before the closing `}` of that module)**
```rust
#[test]
fn parse_fn_rejects_duplicate_doc_clause() {
let err = parse(
r#"
(module m
(fn f
(doc "first")
(doc "second")
(type (fn-type (params) (ret (con Unit))))
(params)
(body (do io/print_str "x"))))
"#,
)
.unwrap_err();
let msg = format!("{err:?}");
assert!(
msg.contains("fn `f` has duplicate `(doc ...)` clause"),
"expected duplicate-doc diagnostic, got: {msg}"
);
}
#[test]
fn parse_fn_rejects_duplicate_type_clause() {
let err = parse(
r#"
(module m
(fn f
(type (fn-type (params) (ret (con Unit))))
(type (fn-type (params) (ret (con Unit))))
(params)
(body (do io/print_str "x"))))
"#,
)
.unwrap_err();
let msg = format!("{err:?}");
assert!(
msg.contains("fn `f` has duplicate `(type ...)` clause"),
"expected duplicate-type diagnostic, got: {msg}"
);
}
#[test]
fn parse_fn_rejects_duplicate_params_clause() {
let err = parse(
r#"
(module m
(fn f
(type (fn-type (params) (ret (con Unit))))
(params)
(params)
(body (do io/print_str "x"))))
"#,
)
.unwrap_err();
let msg = format!("{err:?}");
assert!(
msg.contains("fn `f` has duplicate `(params ...)` clause"),
"expected duplicate-params diagnostic, got: {msg}"
);
}
#[test]
fn parse_fn_rejects_duplicate_body_clause() {
let err = parse(
r#"
(module m
(fn f
(type (fn-type (params) (ret (con Unit))))
(params)
(body (do io/print_str "x"))
(body (do io/print_str "y"))))
"#,
)
.unwrap_err();
let msg = format!("{err:?}");
assert!(
msg.contains("fn `f` has duplicate `(body ...)` clause"),
"expected duplicate-body diagnostic, got: {msg}"
);
}
```
- [ ] **Step 2: run RED — confirm all 4 fail**
Run: `cargo test --workspace -p ailang-surface parse_fn_rejects_duplicate 2>&1 | tail -25`
Expected: FAIL — all 4 tests panic with `expected duplicate-<clause> diagnostic, got: …` (the parser currently silent-overwrites and parse succeeds, so `unwrap_err()` panics; OR it succeeds in the second `(body)` case and `unwrap_err` panics; either way the assertion fails).
- [ ] **Step 3: replace each loose clause-handler in `parse_fn` (lines 487-499) with the strict form**
In `crates/ailang-surface/src/parse.rs` `parse_fn` (around line 485-513), replace:
```rust
match self.peek_head_ident() {
Some("doc") => doc = Some(self.parse_doc()?),
Some("suppress") => suppress.push(self.parse_suppress_attr()?),
Some("type") => {
let t = self.parse_type_attr()?;
ty = Some(t);
}
Some("params") => {
let p = self.parse_params_attr()?;
params = Some(p);
}
Some("body") => {
let b = self.parse_body_attr()?;
body = Some(b);
}
```
with:
```rust
match self.peek_head_ident() {
Some("doc") => {
if doc.is_some() {
let pos = self.peek().map(|t| t.span.start).unwrap_or(0);
return Err(ParseError::Production {
production: "fn-def",
message: format!(
"fn `{name}` has duplicate `(doc ...)` clause"
),
pos,
});
}
doc = Some(self.parse_doc()?);
}
Some("suppress") => suppress.push(self.parse_suppress_attr()?),
Some("type") => {
if ty.is_some() {
let pos = self.peek().map(|t| t.span.start).unwrap_or(0);
return Err(ParseError::Production {
production: "fn-def",
message: format!(
"fn `{name}` has duplicate `(type ...)` clause"
),
pos,
});
}
ty = Some(self.parse_type_attr()?);
}
Some("params") => {
if params.is_some() {
let pos = self.peek().map(|t| t.span.start).unwrap_or(0);
return Err(ParseError::Production {
production: "fn-def",
message: format!(
"fn `{name}` has duplicate `(params ...)` clause"
),
pos,
});
}
params = Some(self.parse_params_attr()?);
}
Some("body") => {
if body.is_some() {
let pos = self.peek().map(|t| t.span.start).unwrap_or(0);
return Err(ParseError::Production {
production: "fn-def",
message: format!(
"fn `{name}` has duplicate `(body ...)` clause"
),
pos,
});
}
body = Some(self.parse_body_attr()?);
}
```
- [ ] **Step 4: run GREEN — all 4 pass**
Run: `cargo test --workspace -p ailang-surface parse_fn_rejects_duplicate 2>&1 | tail -15`
Expected: PASS — `4 passed; 0 failed`.
- [ ] **Step 5: full ailang-surface 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 100+ existing fixtures must still parse — strict-only-on-duplicate is a no-op for valid inputs.)
- [ ] **Step 6: commit**
```bash
git add crates/ailang-surface/src/parse.rs
git commit -m "iter 22-tidy.5.1: parse_fn rejects duplicate doc/type/params/body clauses"
```
---
## Task 2: `parse_const` strict duplicate-clause detection (3 sites)
**Files:**
- Modify: `crates/ailang-surface/src/parse.rs` — `parse_const` body at lines 648-665; `mod tests` (append three tests).
- [ ] **Step 1: write 3 RED unit tests at the END of `mod tests` (after Task 1's tests)**
```rust
#[test]
fn parse_const_rejects_duplicate_doc_clause() {
let err = parse(
r#"
(module m
(const c
(doc "first")
(doc "second")
(type (con Int))
(body (lit (int 42)))))
"#,
)
.unwrap_err();
let msg = format!("{err:?}");
assert!(
msg.contains("const `c` has duplicate `(doc ...)` clause"),
"expected duplicate-doc diagnostic, got: {msg}"
);
}
#[test]
fn parse_const_rejects_duplicate_type_clause() {
let err = parse(
r#"
(module m
(const c
(type (con Int))
(type (con Int))
(body (lit (int 42)))))
"#,
)
.unwrap_err();
let msg = format!("{err:?}");
assert!(
msg.contains("const `c` has duplicate `(type ...)` clause"),
"expected duplicate-type diagnostic, got: {msg}"
);
}
#[test]
fn parse_const_rejects_duplicate_body_clause() {
let err = parse(
r#"
(module m
(const c
(type (con Int))
(body (lit (int 1)))
(body (lit (int 2)))))
"#,
)
.unwrap_err();
let msg = format!("{err:?}");
assert!(
msg.contains("const `c` has duplicate `(body ...)` clause"),
"expected duplicate-body diagnostic, got: {msg}"
);
}
```
- [ ] **Step 2: run RED — confirm all 3 fail**
Run: `cargo test --workspace -p ailang-surface parse_const_rejects_duplicate 2>&1 | tail -20`
Expected: FAIL — all 3 tests panic; the parser currently silent-overwrites.
- [ ] **Step 3: replace each loose clause-handler in `parse_const` (lines 648-665) with the strict form**
In `crates/ailang-surface/src/parse.rs` `parse_const`, replace:
```rust
loop {
match self.peek_head_ident() {
Some("doc") => doc = Some(self.parse_doc()?),
Some("type") => ty = Some(self.parse_type_attr()?),
Some("body") => value = Some(self.parse_body_attr()?),
Some(other) => {
```
with:
```rust
loop {
match self.peek_head_ident() {
Some("doc") => {
if doc.is_some() {
let pos = self.peek().map(|t| t.span.start).unwrap_or(0);
return Err(ParseError::Production {
production: "const-def",
message: format!(
"const `{name}` has duplicate `(doc ...)` clause"
),
pos,
});
}
doc = Some(self.parse_doc()?);
}
Some("type") => {
if ty.is_some() {
let pos = self.peek().map(|t| t.span.start).unwrap_or(0);
return Err(ParseError::Production {
production: "const-def",
message: format!(
"const `{name}` has duplicate `(type ...)` clause"
),
pos,
});
}
ty = Some(self.parse_type_attr()?);
}
Some("body") => {
if value.is_some() {
let pos = self.peek().map(|t| t.span.start).unwrap_or(0);
return Err(ParseError::Production {
production: "const-def",
message: format!(
"const `{name}` has duplicate `(body ...)` clause"
),
pos,
});
}
value = Some(self.parse_body_attr()?);
}
Some(other) => {
```
- [ ] **Step 4: run GREEN — all 3 pass**
Run: `cargo test --workspace -p ailang-surface parse_const_rejects_duplicate 2>&1 | tail -15`
Expected: PASS — `3 passed; 0 failed`.
- [ ] **Step 5: full ailang-surface 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.
- [ ] **Step 6: commit**
```bash
git add crates/ailang-surface/src/parse.rs
git commit -m "iter 22-tidy.5.2: parse_const rejects duplicate doc/type/body clauses"
```
---
## Task 3: `parse_data` strict duplicate-clause detection (1 site — `doc`)
`parse_data` already has strict detection for `drop-iterative` (lines 401-409); `ctor` is multi-instance and stays loose. Only `doc` is loose-and-shouldn't-be.
**Files:**
- Modify: `crates/ailang-surface/src/parse.rs` — `parse_data` body at lines 374-378; `mod tests` (append one test).
- [ ] **Step 1: write 1 RED unit test at the END of `mod tests` (after Task 2's tests)**
```rust
#[test]
fn parse_data_rejects_duplicate_doc_clause() {
let err = parse(
r#"
(module m
(data D
(doc "first")
(doc "second")
(ctor C)))
"#,
)
.unwrap_err();
let msg = format!("{err:?}");
assert!(
msg.contains("data `D` has duplicate `(doc ...)` clause"),
"expected duplicate-doc diagnostic, got: {msg}"
);
}
```
- [ ] **Step 2: run RED — confirm fail**
Run: `cargo test --workspace -p ailang-surface parse_data_rejects_duplicate_doc 2>&1 | tail -10`
Expected: FAIL — the parser silently keeps the second `doc` value (last-wins).
- [ ] **Step 3: replace the loose `doc` handler in `parse_data` (lines 374-378) with the strict form**
In `crates/ailang-surface/src/parse.rs` `parse_data`, replace:
```rust
Some("doc") => {
let s = self.parse_doc()?;
doc = Some(s);
}
```
with:
```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()?);
}
```
- [ ] **Step 4: run GREEN — pass**
Run: `cargo test --workspace -p ailang-surface parse_data_rejects_duplicate_doc 2>&1 | tail -10`
Expected: PASS — `1 passed; 0 failed`.
- [ ] **Step 5: full ailang-surface 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.
- [ ] **Step 6: commit**
```bash
git add crates/ailang-surface/src/parse.rs
git commit -m "iter 22-tidy.5.3: parse_data rejects duplicate doc clause"
```
---
## Task 4: verify + JOURNAL
**Files:**
- Modify: `docs/JOURNAL.md` — append iteration entry.
- [ ] **Step 1: full workspace test sweep**
Run: `cargo test --workspace 2>&1 | grep -c "FAILED"`
Expected: `0`.
Run: `cargo test --workspace 2>&1 | grep -E "^test result" | wc -l`
Expected: `≥ 23` (test-binary count unchanged; the 8 new tests are inside `ailang-surface`'s in-file `mod tests`).
- [ ] **Step 2: 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 3: confirm strict-asymmetry retired**
Run: `grep -nE "Some\(\"(doc|type|params|body)\"\) =>" crates/ailang-surface/src/parse.rs | grep -v "if .*\.is_some\(\)"`
Expected: zero hits inside `parse_fn` / `parse_const` / `parse_data` (the surviving non-`is_some`-guarded `Some(...) =>` lines should only be inside `parse_class`, `parse_instance`, sub-clause parsers like `parse_class_method`, or the `Some("suppress") => suppress.push(...)` line which is a multi-instance Vec append and intentionally loose). Visually verify.
- [ ] **Step 4: append JOURNAL entry**
Append to `docs/JOURNAL.md`:
```markdown
## 2026-05-10 — Iteration 22-tidy.5: strict duplicate-clause detection in fn/const/data parsers
Closing the second milestone-22 carried-debt item flagged in
`2026-05-09 — Iteration 22-tidy`: the strict duplicate-clause
detection that 22b.4a introduced for `parse_class` /
`parse_instance` / their method sub-parsers (10 sites) was missing
from the sibling parsers `parse_fn`, `parse_const`, `parse_data` —
those silently last-wins on duplicate clauses. This iteration
lifts the strict pattern to those three siblings.
Sites promoted to strict (8 total):
- `parse_fn`: `doc`, `type`, `params`, `body`
- `parse_const`: `doc`, `type`, `body`
- `parse_data`: `doc` (only — `ctor` is multi-instance,
`drop-iterative` was already strict)
Each new strict check uses the established 22b.4a pattern
(`if X.is_some() { return ParseError::Production { … } }`) and the
established message form (`` "<prod> `<name>` has duplicate `(<clause> ...)` clause" ``).
Eight new RED-first unit tests in `parse.rs::mod tests` pin each
diagnostic.
Tasks (commit subjects):
- 22-tidy.5.1: parse_fn rejects duplicate doc/type/params/body clauses
- 22-tidy.5.2: parse_const rejects duplicate doc/type/body clauses
- 22-tidy.5.3: parse_data rejects duplicate doc clause
Acceptance: 8 new tests RED-first then GREEN; full workspace test
sweep green; bench gates 0/0/0; no behaviour change for valid
fixtures (the ~106 round-trip fixtures stay green). The
strict-vs-loose asymmetry between class/instance and fn/const/data
parsers is retired.
Carried-debt status (after this iter):
- ✅ Primitive-name-set consolidation (closed 22-tidy.4).
- ✅ Strict duplicate-clause detection in fn/const/data (this iter).
- ⏳ Form-B (prose) printer arms for ClassDef / InstanceDef — still
queued.
- ✅ Lib.rs gap-related sites at lib.rs:1266 / 1853-1856 / 1978 /
2314-2316 — audit confirmed defensive (no action needed; see
`2026-05-10 — Audit: mono-pass env-seeding gaps`).
```
- [ ] **Step 5: commit**
```bash
git add docs/JOURNAL.md
git commit -m "iter 22-tidy.5: journal entry"
```