convention: counter-prefix file naming across docs/specs/, docs/plans/, design/contracts/, design/models/
All 176 files in the four accumulating directories now use a zero-padded 4-digit counter prefix that reflects creation order (`NNNN-slug.md`). The counter is assigned per directory in strict git-log creation order; ties broken alphabetically by original name. The old `YYYY-MM-DD-` prefix on docs/specs/ and docs/plans/ files is dropped — the date is recoverable from git log and the counter carries the ordering. A file's counter is stable for the life of the file: never reassigned, never reused, never compacted. Deleted files retire their counter; subsequent files do not fill the gap. This is the property that lets cross-references stay literal — refs use the full filename including the counter (`design/contracts/0007-honesty-rule.md`) so they grep cleanly and resolve directly without a glob step. 313 cross-references updated across .md/.rs/.toml/.c/.json files (test pins, include_str! paths, design-INDEX entries, baseline notes, runtime C comments, inter-contract markdown links incl. bare basename and `../models/foo.md` forms). CLAUDE.md gets a new "File-naming convention" section spelling out the rule and rationale. skills/brainstorm/SKILL.md and skills/planner/SKILL.md updated so new spec/plan creation produces counter-prefixed names from the start. The full test suite (cargo test --workspace) passes.
This commit is contained in:
@@ -0,0 +1,396 @@
|
||||
# 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"
|
||||
```
|
||||
Reference in New Issue
Block a user