diff --git a/AGENTS.md b/AGENTS.md index cc1a83f..ba0d01d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -194,6 +194,8 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | RES001 | (0.3+) `Result.try` / `Result.tryAsync` with no body. | `Result.try { }`. | | INT001 | (0.7+) A fn declares `intent: "pure"` but also has `uses { … }`. (0.8+) Also fires when `intent: "pure"` is combined with `reads { … }` or `writes { … }`. Pure functions may not declare capabilities or resource dependencies. | Either drop the conflicting header clause(s) or change the intent to reflect the actual behaviour. | | SYN001 | Duplicate fn header clause (e.g. two `reads { }` on the same fn, or two `intent:`), or a label inside `reads {}` / `writes {}` that is not a plain identifier. `parseFn` is version-agnostic, so SYN001 fires whenever a duplicate clause is written regardless of the `?bs` pin. | Declare each header clause once; merge label lists rather than repeating the clause; use bare identifiers (not quoted strings) as labels. | +| THR001 | (0.9+) A fn (or a same-file callee, transitively) throws an exception type not declared in the fn's `throws {}`. If `loadUser` calls `fetchRow throws { NetworkError }`, `loadUser` must also declare `throws { NetworkError }`. | Add the missing type(s) to `throws {}` at each level of the call chain. | +| THR002 | (0.9+) A fn body directly constructs `err(TypeName(...))`, `err(new TypeName(...))`, or `err(TypeName)` where `TypeName` (CapCase) is not declared in the fn's own `throws {}` clause. Producer-side complement to THR001. | Add `TypeName` to the fn's `throws {}`, or change the error construction to use a declared type. | When you add a new compiler error, allocate the next free code in the same range (`BSnnn` for general parse errors, `CAPnnn` for capability checks, diff --git a/CHANGELOG.md b/CHANGELOG.md index d10b979..8a3bd03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ goes behind a new pin. ## ?bs 0.9 — unreleased ### Added +- **THR001 — `throws {}` transitivity enforcement.** + From `?bs 0.9`, the compiler enforces that if fn A calls fn B (in the same + file) and B declares `throws { X }`, then A must also declare `throws { X }`. + The rule applies transitively to any call depth. Reading A's header now tells + you the complete exception surface without tracing the call graph manually. + Over-declaration is always allowed (conservative headers are harmless). + +- **THR002 — undeclared error type construction.** + From `?bs 0.9`, the compiler fires when a fn body contains + `err(TypeName(...))`, `err(new TypeName(...))`, or bare `err(TypeName)` + where TypeName (CapCase ident) is absent from the fn's own `throws { }` + clause. Catches the case where a fn produces an error type its callers + cannot match. Indirect patterns (`err(e)` where `e`'s type is inferred) + are out of scope. + - **DEP001 / DEP002 — `reads {}` / `writes {}` transitivity enforcement.** From `?bs 0.9`, the compiler enforces that if fn A calls fn B (in the same file) and B declares `reads { x }` (or `writes { x }`), then A must also diff --git a/README.md b/README.md index 386f21b..3914489 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,7 @@ claude mcp add botscript -- npx -y @mbfarias/botscript-mcp | ----------- | -------------------------------------- | --------------------------------------------------------------------------------------------------- | | `primer` | (no args) | The canonical language primer (same text the `?primer` directive emits). | | `transform` | `{ source: string, filename?: string }` | `{ ok: true, code, forms, version }` on success, or `{ ok: false, diagnostics: [...] }` on failure. | -| `explain` | `{ code: string }` | Long-form explanation for a stable diagnostic code (`BS001`, `BS002`, `CAP001`, `CAP002`, `UNS001`–`UNS004`, `RES001`, `FMT001`, `INT001`, `SYN001`) plus a fails/passes example pair. | +| `explain` | `{ code: string }` | Long-form explanation for any stable diagnostic code (`BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`, `DEP002`, `EFF002`–`EFF004`, `FMT001`, `INT001`, `INT002`, `RES001`, `SYN001`, `THR001`–`THR003`, `UNS001`–`UNS004`) plus a fails/passes example pair. | A bot's loop becomes deterministic: `transform` → if `ok=false`, read `diagnostics[0].code` → `explain(code)` → apply `rewrite` → `transform` again. diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index 1e15f65..38c84ff 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -385,6 +385,33 @@ const E: Record = { "fn fetchRemote(id: string) throws { HttpError } -> string = id\n" + "fn loadUser(id: string) throws { HttpError } -> string = fetchRemote(id)", }, + THR002: { + code: "THR002", + title: "fn body constructs an error type not present in its throws declaration", + rule: + "if a fn body contains `err(TypeName(...))`, `err(new TypeName(...))`, or bare `err(TypeName)` " + + "where TypeName (CapCase ident) is not in the fn's own `throws { }` set, the fn is producing an " + + "error callers cannot match — they will never see a TypeName arm", + idiom: + "add the constructed error type to the fn's `throws { }` clause so callers can exhaustively match it; " + + "indirect patterns like `err(e)` (where e's type is inferred) are out of scope — only direct " + + "constructor calls and bare CapCase references are checked", + rewrite: + "fn name(...) throws { …existing, UndeclaredError } -> ...", + example: + "// before — parseConfig constructs NetworkError but declares throws { ParseError }\n" + + "?bs 0.9\n" + + "fn parseConfig(s: string) throws { ParseError } -> Result {\n" + + " if (bad) err(NetworkError(\"timed out\")) // THR002: NetworkError not declared\n" + + " else ok(s)\n" + + "}\n\n" + + "// after\n" + + "?bs 0.9\n" + + "fn parseConfig(s: string) throws { ParseError, NetworkError } -> Result {\n" + + " if (bad) err(NetworkError(\"timed out\"))\n" + + " else ok(s)\n" + + "}", + }, }; export function getErrorCode(code: string): ErrorCodeEntry | undefined { diff --git a/packages/compiler/src/passes/thr-check.ts b/packages/compiler/src/passes/thr-check.ts index 0b7d4ea..a6a66b2 100644 --- a/packages/compiler/src/passes/thr-check.ts +++ b/packages/compiler/src/passes/thr-check.ts @@ -2,27 +2,25 @@ * Throws declaration check (?bs 0.9+). * * Enforces transitivity of `throws { ... }` annotations across same-file - * function calls. - * - * Rule: if fn A calls fn B (defined in the same file) and B declares - * `throws { X }`, then A must also declare `throws { X }`. - * - * This makes the failure surface of each fn complete from a caller's - * perspective — reading A's header tells you every exception type A (or - * anything it calls) may throw, without tracing through the call graph. + * function calls, and checks that a fn's body does not directly construct + * error types absent from its own `throws {}` declaration. * * THR001 throws under-declared: fn A calls fn B which (transitively) * declares `throws { X }` that A does not declare. For a direct * call the diagnostic says "'B' which throws { X }"; for a multi-hop * chain it names the path, e.g. "B -> C — 'C' throws { X }". * - * Only same-file call resolution is performed (same as cap-check / dep-check). - * Over-declaration is intentionally NOT checked — a caller may conservatively - * declare more exception types than it strictly needs. + * THR002 undeclared error construction: fn body contains `err(TypeName(...))`, + * `err(new TypeName(...))`, or bare `err(TypeName)` where TypeName + * (CapCase ident) is not in the fn's own `throws {}` set. Catches the + * case where a fn returns an error type it never declared, leaving + * callers' exhaustive match arms permanently dead. Indirect patterns + * (`err(e)` where e's type is inferred) are out of scope — token-based + * detection only. * - * NOTE: This pass enforces transitivity only — it does NOT verify that a fn's - * body actually throws the types it declares (a leaf fn can lie). Body-level - * soundness requires the effect inference pass; see issue #14. + * Only same-file call resolution is performed for THR001 (same as cap-check / + * dep-check). Over-declaration is intentionally NOT checked — a caller may + * conservatively declare more exception types than it strictly needs. */ import { BotscriptError } from "../diagnostics.js"; @@ -31,7 +29,8 @@ import { parseProgram } from "../parser/parse.js"; import type { FnDecl } from "../parser/parse-fn.js"; import { atLeast, type VersionInfo } from "./version.js"; import { locationOf } from "./_location.js"; -import { computeNesting, collectCallees } from "./_callgraph.js"; +import type { Token } from "../parser/lex.js"; +import { computeNesting, collectCallees, nextSignificant, prevSignificant } from "./_callgraph.js"; // --------------------------------------------------------------------------- // Types @@ -115,16 +114,23 @@ export function passThrCheck(src: string, version: VersionInfo): string { } } - // Validate: declared throws must cover transitive throws. + // THR001: declared throws must cover transitive throws. for (const rec of records.values()) { const missing = [...rec.transitiveThrows.keys()] .filter((l) => !rec.declaredThrows.has(l)) .sort(); if (missing.length > 0) { - throw mkError(src, rec, missing); + throw mkThr001Error(src, rec, missing); } } + // THR002: fn body must not directly construct undeclared error types. + for (const rec of records.values()) { + const inner = innerByDecl.get(rec.decl) ?? []; + const err = checkBodyErrors(tokens, rec.decl, inner, rec.declaredThrows, src); + if (err) throw err; + } + return src; } @@ -143,7 +149,7 @@ function formatPath(path: ThrPath): string { return segments.join(" -> "); } -function mkError(src: string, rec: FnRecord, missingLabels: string[]): BotscriptError { +function mkThr001Error(src: string, rec: FnRecord, missingLabels: string[]): BotscriptError { const entry = getErrorCode("THR001")!; const { line, column } = locationOf(src, rec.decl.fnKeywordStart); @@ -160,11 +166,6 @@ function mkError(src: string, rec: FnRecord, missingLabels: string[]): Botscript ? formatPath(firstPath.next) : pathStr; - const currentDeclStr = - rec.declaredThrows.size === 0 - ? "no throws clause" - : `throws { ${[...rec.declaredThrows].sort().join(", ")} }`; - const proposed = [...new Set([...rec.declaredThrows, ...missingLabels])].sort().join(", "); const otherMissing = missingLabels.slice(1); @@ -177,9 +178,13 @@ function mkError(src: string, rec: FnRecord, missingLabels: string[]): Botscript ? `'${leaf}' which throws { ${firstLabel} }` : `${displayPath} — '${leaf}' throws { ${firstLabel} }`; + const declSuffix = + rec.declaredThrows.size === 0 + ? `has no throws clause` + : `has throws { ${[...rec.declaredThrows].sort().join(", ")} } but not { ${missingLabels.join(", ")} }`; const message = `fn '${rec.decl.name}'${transitively} calls ${callDescription}, ` + - `but '${rec.decl.name}' declares ${currentDeclStr}${otherTail}`; + `but '${rec.decl.name}' ${declSuffix}${otherTail}`; const callPath = `call path: ${pathStr}`; const nameEnd = rec.decl.nameStart + rec.decl.name.length; @@ -200,3 +205,93 @@ function mkError(src: string, rec: FnRecord, missingLabels: string[]): Botscript return new BotscriptError([diagnostic]); } + +/** + * THR002: scan fn body for `err(TypeName(...))`, `err(new TypeName(...))`, or + * bare `err(TypeName)` where TypeName (CapCase ident) is not in the fn's own + * `throws {}` set. Returns a BotscriptError on the first violation found, or null. + */ +function checkBodyErrors( + tokens: Token[], + fn: FnDecl, + inner: FnDecl[], + declaredThrows: Set, + src: string, +): BotscriptError | null { + const entry = getErrorCode("THR002")!; + + // Cursor-based inner-fn exclusion. + const open: FnDecl[] = []; + let nextInner = 0; + + for (let i = fn.bodyTokenStart ?? fn.tokenStart; i < fn.tokenEnd; i++) { + while (open.length > 0 && open[open.length - 1]!.tokenEnd <= i) open.pop(); + while (nextInner < inner.length && inner[nextInner]!.tokenStart <= i) { + open.push(inner[nextInner]!); + nextInner++; + } + if (open.length > 0) continue; + + const tok = tokens[i]; + // Look for `err` ident — must not be a property access. + if (!tok || tok.kind !== "ident" || tok.text !== "err") continue; + + const prevIdx = prevSignificant(tokens, i - 1); + const prev = tokens[prevIdx]; + if (prev && ((prev.kind === "punct" && prev.text === ".") || prev.kind === "questionDot")) continue; + + // Next must be `(` + const parenIdx = nextSignificant(tokens, i + 1); + const parenTok = tokens[parenIdx]; + if (!parenTok || parenTok.kind !== "open" || parenTok.text !== "(") continue; + + // Look at the first argument. + let argIdx = nextSignificant(tokens, parenIdx + 1); + let argTok = tokens[argIdx]; + + // Handle `err(new TypeName(...))` — skip `new` ident. + if (argTok && argTok.kind === "ident" && argTok.text === "new") { + argIdx = nextSignificant(tokens, argIdx + 1); + argTok = tokens[argIdx]; + } + + if (!argTok || argTok.kind !== "ident") continue; + const typeName = argTok.text; + + // CapCase: first character is an uppercase letter. + if (!/^[A-Z]/.test(typeName)) continue; + + // The token after the type name must be `(` (constructor call) or `)` (bare ref). + const afterIdx = nextSignificant(tokens, argIdx + 1); + const after = tokens[afterIdx]; + if (!after) continue; + const isCtor = after.kind === "open" && after.text === "("; + const isRef = after.kind === "close" && after.text === ")"; + if (!isCtor && !isRef) continue; + + // Already declared — fine. + if (declaredThrows.has(typeName)) continue; + + const { line, column } = locationOf(src, tok.start); + const proposed = [...new Set([...declaredThrows, typeName])].sort().join(", "); + + return new BotscriptError([{ + code: "THR002", + severity: "error" as const, + file: null, + line, + column, + start: tok.start, + end: tok.end, + message: + declaredThrows.size === 0 + ? `fn '${fn.name}' constructs err(${typeName}...) but has no throws clause` + : `fn '${fn.name}' constructs err(${typeName}...) but '${typeName}' is not declared in throws { ${[...declaredThrows].sort().join(", ")} }`, + rule: entry.rule, + idiom: entry.idiom, + rewrite: `fn ${fn.name}(...) throws { ${proposed} } -> ...`, + }]); + } + + return null; +} diff --git a/packages/compiler/tests/thr-check.test.ts b/packages/compiler/tests/thr-check.test.ts index 50c9470..99fe9ba 100644 --- a/packages/compiler/tests/thr-check.test.ts +++ b/packages/compiler/tests/thr-check.test.ts @@ -1,7 +1,9 @@ /** - * Tests for throws {} transitivity enforcement (?bs 0.9+). + * Tests for throws {} enforcement (?bs 0.9+). * * THR001: fn A calls fn B which throws { X }, but A doesn't declare throws { X }. + * THR002: fn body constructs err(TypeName(...)), err(new TypeName(...)), or bare + * err(TypeName) where TypeName (CapCase) is not in throws {}. */ import { describe, expect, it } from "vitest"; @@ -174,3 +176,137 @@ describe("THR001: throws under-declared (0.9+)", () => { expect(() => compile(src)).not.toThrow(); }); }); + +// --------------------------------------------------------------------------- +// THR002: undeclared error construction +// --------------------------------------------------------------------------- + +describe("THR002: body constructs undeclared error type (0.9+)", () => { + it("fires when body calls err(CapCase(...)) not in throws {}", () => { + const src = + "?bs 0.9\n" + + "fn parseConfig(s: string) throws { ParseError } -> Result {\n" + + " if (bad) err(NetworkError(\"timed out\"))\n" + + " else ok(s)\n" + + "}\n"; + expect(() => compile(src)).toThrow("THR002"); + expect(() => compile(src)).toThrow(/NetworkError/); + }); + + it("fires when body calls err(CapCase) bare ref not in throws {}", () => { + const src = + "?bs 0.9\n" + + "fn build(s: string) -> Result {\n" + + " if (bad) err(BuildError)\n" + + " else ok(s)\n" + + "}\n"; + expect(() => compile(src)).toThrow("THR002"); + expect(() => compile(src)).toThrow(/BuildError/); + }); + + it("fires when body calls err(new CapCase(...)) not in throws {}", () => { + const src = + "?bs 0.9\n" + + "fn connect(url: string) throws { TimeoutError } -> Result {\n" + + " if (bad) err(new NetworkError(\"conn refused\"))\n" + + " else ok(url)\n" + + "}\n"; + expect(() => compile(src)).toThrow("THR002"); + expect(() => compile(src)).toThrow(/NetworkError/); + }); + + it("does not fire when the error type is declared in throws {}", () => { + const src = + "?bs 0.9\n" + + "fn parseConfig(s: string) throws { ParseError } -> Result {\n" + + " if (bad) err(ParseError(\"invalid\"))\n" + + " else ok(s)\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire for lowercase err(e) patterns (indirect, out of scope)", () => { + const src = + "?bs 0.9\n" + + "fn wrap(s: string) -> Result {\n" + + " const e = \"something failed\"\n" + + " err(e)\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire when the fn has no throws {} but body uses err(lowercase)", () => { + const src = + "?bs 0.9\n" + + "fn fail(s: string) -> Result = err(s)\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire below ?bs 0.9", () => { + const src = + "?bs 0.8\n" + + "fn parseConfig(s: string) throws { ParseError } -> Result {\n" + + " if (bad) err(NetworkError(\"timed out\"))\n" + + " else ok(s)\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire for err as a property access (obj.err(...))", () => { + const src = + "?bs 0.9\n" + + "fn handle(logger: { err: (x: string) => void }, s: string) -> string {\n" + + " logger.err(BadInput(\"oops\"))\n" + + " s\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire when declared throws covers multiple error types including the constructed one", () => { + const src = + "?bs 0.9\n" + + "fn fetch(url: string) throws { HttpError, NetworkError } -> Result {\n" + + " if (slow) err(NetworkError(\"timeout\"))\n" + + " else err(HttpError(\"404\"))\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("includes the undeclared type name in the error message", () => { + const src = + "?bs 0.9\n" + + "fn parse(s: string) -> Result {\n" + + " err(SyntaxError(\"bad input\"))\n" + + "}\n"; + expect(() => compile(src)).toThrow(/SyntaxError/); + expect(() => compile(src)).toThrow(/THR002/); + }); + + it("fires with 'has no throws clause' message when fn has no throws annotation", () => { + const src = + "?bs 0.9\n" + + "fn parse(s: string) -> Result {\n" + + " err(ParseError(\"bad\"))\n" + + "}\n"; + try { + compile(src); + expect.fail("should have thrown"); + } catch (e) { + const err = e as { diagnostics?: Array<{ code: string; message: string }> }; + expect(err.diagnostics?.[0]?.code).toBe("THR002"); + expect(err.diagnostics?.[0]?.message).toMatch(/has no throws clause/); + } + }); + + it("does not fire for err call inside an inner fn that itself declares the type", () => { + const src = + "?bs 0.9\n" + + "fn outer(s: string) -> string {\n" + + " fn inner(x: string) throws { ParseError } -> Result {\n" + + " err(ParseError(\"bad\"))\n" + + " }\n" + + " s\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index e20893a..c1ab83e 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -463,6 +463,64 @@ export const EXPLANATIONS: Readonly> = { "fn recordEvent(id: string) writes { metrics } -> void { updateMetrics(id); }\n", }, }, + THR001: { + code: "THR001", + title: "fn transitively throws an exception type not declared in its header", + body: + "THR001 fires from `?bs 0.9` when fn A calls fn B (directly or transitively, same file) " + + "and B declares `throws { X }` that A's own `throws { }` clause does not include.\n\n" + + "The throws declaration is the caller's contract: reading A's header should tell you every " + + "exception type A (or anything it calls) may produce. Without the transitivity rule, callers " + + "of A see an incomplete failure surface — they match on A's declared throws and miss the " + + "types that bubble up from deeper in the call graph.\n\n" + + "Over-declaration is intentionally allowed: a fn may declare `throws { X, Y }` even if it " + + "only calls fns that throw `{ X }`. Conservative declarations are safe; under-declarations " + + "are not.\n\n" + + "THR001 is gated on `?bs 0.9`. Files pinned to earlier versions are unaffected.", + example: { + fails: + "?bs 0.9\n" + + "fn fetchRemote(id: string) throws { HttpError } -> string = id\n" + + "fn loadUser(id: string) -> string = fetchRemote(id)\n", + passes: + "?bs 0.9\n" + + "fn fetchRemote(id: string) throws { HttpError } -> string = id\n" + + "fn loadUser(id: string) throws { HttpError } -> string = fetchRemote(id)\n", + }, + }, + THR002: { + code: "THR002", + title: "fn body constructs an error type absent from its throws declaration", + body: + "THR002 fires from `?bs 0.9` when a fn body contains `err(TypeName(...))`, " + + "`err(new TypeName(...))`, or bare `err(TypeName)` where TypeName (a CapCase " + + "identifier) is not present in the fn's own `throws { }` clause.\n\n" + + "This is the producer-side complement to THR001. THR001 ensures callers propagate the " + + "throws surface; THR002 ensures the fn actually declares what it produces. Without it, " + + "a fn can silently return an error type its callers cannot match — exhaustive match arms " + + "for the undeclared type will be permanently dead code.\n\n" + + "**Scope:** token-based detection only. Direct construction patterns are caught:\n" + + "- `err(HttpError(msg))` → detects `HttpError`\n" + + "- `err(new ParseError(...))` → detects `ParseError`\n" + + "- `err(BuildError)` → detects `BuildError` (bare ref, not a call)\n\n" + + "Indirect patterns (`err(e)` where `e` carries a type) require inference and are " + + "intentionally out of scope.\n\n" + + "THR002 is gated on `?bs 0.9`. Files pinned to earlier versions are unaffected.", + example: { + fails: + "?bs 0.9\n" + + "fn parseConfig(s: string) throws { ParseError } -> Result {\n" + + " if (bad) err(NetworkError(\"timed out\"))\n" + + " else ok(s)\n" + + "}\n", + passes: + "?bs 0.9\n" + + "fn parseConfig(s: string) throws { ParseError, NetworkError } -> Result {\n" + + " if (bad) err(NetworkError(\"timed out\"))\n" + + " else ok(s)\n" + + "}\n", + }, + }, THR003: { code: "THR003", title: "outer fn declares narrower throws than a callback parameter", diff --git a/packages/mcp/tests/server.test.ts b/packages/mcp/tests/server.test.ts index 1dbf6ca..36a8a44 100644 --- a/packages/mcp/tests/server.test.ts +++ b/packages/mcp/tests/server.test.ts @@ -60,6 +60,8 @@ describe("botscript-mcp explanations", () => { "INT002", "RES001", "SYN001", + "THR001", + "THR002", "THR003", "UNS001", "UNS002",