diff --git a/CHANGELOG.md b/CHANGELOG.md index 5951e5f..d10b979 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,16 @@ goes behind a new pin. - EFF004 fires when a callback's declared writes are not a subset of the outer fn's declared writes. +- **THR003 — `throws {}` on callback parameters.** + From `?bs 0.9`, when a function-typed parameter carries a `throws { X }` annotation, + the containing fn must declare at least those exception types in its own `throws {}` + clause. Calling the callback can surface X — the outer fn cannot advertise a narrower + throws surface than it can exercise. Closes the "callback throws-leak" vector, completing + the trilogy with EFF002 (`uses {}`) and EFF003/EFF004 (`reads {}`/`writes {}`). + - `throws {}` annotations on callback parameter types are stripped from emitted TypeScript. + - THR003 fires when any callback parameter's declared throws are not a subset of the + outer fn's declared throws. + - **CAP003 — capability asserted in unsafe fn (non-blocking warning).** From `?bs 0.9`, the compiler emits a `warning` (not an error) when a `uses {}` declaration appears on an `unsafe fn`. The capability inference diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index e1610df..1e15f65 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -336,6 +336,34 @@ const E: Record = { "fn updateMetrics(id: string) writes { metrics } -> void { }\n" + "fn recordEvent(id: string) writes { metrics } -> void { updateMetrics(id); }", }, + THR003: { + code: "THR003", + title: "outer fn declares narrower throws than a callback parameter", + rule: + "if a function-typed parameter declares `throws { X }`, the containing fn must declare at least those " + + "exception types — calling the callback can surface X, so the outer fn's throws surface must cover it", + idiom: + "a fn's throws surface is the union of its own declared throws and the throws its callback parameters may exercise", + rewrite: + "fn name(handler: () throws { X } -> T) throws { …existing, X } -> ...", + example: + "// before — accepts a throwing callback but outer fn declares no throws\n" + + "?bs 0.9\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: fn(string) throws { NetworkError } -> void\n" + + ") -> void { // THR003: missing throws { NetworkError }\n" + + " handler(items[0])\n" + + "}\n\n" + + "// after — outer fn declares the throws its callback may exercise\n" + + "?bs 0.9\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: fn(string) throws { NetworkError } -> void\n" + + ") throws { NetworkError } -> void {\n" + + " handler(items[0])\n" + + "}", + }, THR001: { code: "THR001", title: "fn transitively throws an exception type not declared in its header", diff --git a/packages/compiler/src/parser/parse-fn.ts b/packages/compiler/src/parser/parse-fn.ts index d8d91fb..0a3f5b4 100644 --- a/packages/compiler/src/parser/parse-fn.ts +++ b/packages/compiler/src/parser/parse-fn.ts @@ -73,6 +73,14 @@ export interface FnDecl { * Example: `(cb: () writes { metrics } -> void)` → `["metrics"]` */ paramWrites: string[]; + /** + * Union of all exception types declared in `throws { … }` annotations on + * function-typed parameters. Empty when no parameter carries a throws annotation. + * Used by `passEffCheck` (THR003). Gated on `?bs 0.9`. + * + * Example: `(handler: () throws { NetworkError } -> void)` → `["NetworkError"]` + */ + paramThrows: string[]; capabilities: string[]; /** * Optional declarative read-dependency list, e.g. `reads { cache, db }`. Each @@ -264,7 +272,7 @@ export function parseFn( if (!argsOpen || argsOpen.kind !== "open" || argsOpen.text !== "(" || argsOpen.matchedAt === undefined) return null; const argsClose = argsOpen.matchedAt; const args = sliceText(tokens, i, argsClose + 1); - const { text: argsTs, paramCaps, paramReads, paramWrites } = buildArgsTs(tokens, i, argsClose + 1, opts.src); + const { text: argsTs, paramCaps, paramReads, paramWrites, paramThrows } = buildArgsTs(tokens, i, argsClose + 1, opts.src); i = argsClose + 1; i = skipTrivia(tokens, i); @@ -521,6 +529,7 @@ export function parseFn( paramCaps, paramReads, paramWrites, + paramThrows, capabilities, reads, writes, @@ -694,27 +703,26 @@ function skipTrivia(tokens: Token[], i: number): number { * 1. Botscript `->` (arrow token) → TypeScript `=>` (function type arrow). * 2. `uses { cap, … }` annotations on function-typed parameters are stripped * from the emitted text and their capability names collected into `paramCaps`. - * 3. `reads { label, … }` and `writes { label, … }` annotations on - * function-typed parameters are stripped and collected into `paramReads` / - * `paramWrites`. + * 3. `reads { label, … }`, `writes { label, … }`, and `throws { label, … }` + * annotations on function-typed parameters are stripped and collected into + * `paramReads` / `paramWrites` / `paramThrows`. * * The stripping is position-independent: any effect annotation inside the args * list is treated as a parameter effect annotation. This is safe because the - * stripping only activates on the specific `uses { ... }` / `reads { ... }` / - * `writes { ... }` syntax pattern — a keyword/ident token immediately followed - * by a `{...}` block — not on bare `reads` or `writes` identifiers elsewhere in - * TypeScript type positions (e.g. `reads` as a field name in an object type). + * stripping only activates on the specific keyword/ident immediately followed + * by a `{...}` block — not on bare identifiers in TypeScript type positions. */ function buildArgsTs( tokens: Token[], from: number, to: number, src?: string, -): { text: string; paramCaps: string[]; paramReads: string[]; paramWrites: string[] } { +): { text: string; paramCaps: string[]; paramReads: string[]; paramWrites: string[]; paramThrows: string[] } { let out = ""; const paramCaps: string[] = []; const paramReads: string[] = []; const paramWrites: string[] = []; + const paramThrows: string[] = []; let i = from; while (i < to) { const t = tokens[i]!; @@ -724,13 +732,14 @@ function buildArgsTs( i++; continue; } - // Strip effect annotations (`uses`/`reads`/`writes`) and collect their labels. - // Note: `uses` is a lexer keyword (kind="keyword"), but `reads` and `writes` - // are treated as identifiers (kind="ident") by the lexer. + // Strip effect annotations (`uses`/`reads`/`writes`/`throws`) and collect labels. + // Note: `uses` is a lexer keyword (kind="keyword"); `reads`, `writes`, and + // `throws` are all treated as identifiers (kind="ident") by the lexer. const isUses = t.kind === "keyword" && t.keyword === "uses"; const isReads = t.kind === "ident" && t.text === "reads"; const isWrites = t.kind === "ident" && t.text === "writes"; - if (isUses || isReads || isWrites) { + const isThrows = t.kind === "ident" && t.text === "throws"; + if (isUses || isReads || isWrites || isThrows) { const j = skipTrivia(tokens, i + 1); const open = tokens[j]; if (open && open.kind === "open" && open.text === "{" && open.matchedAt !== undefined) { @@ -743,8 +752,10 @@ function buildArgsTs( const labels = parseLabelList(tokens, j + 1, open.matchedAt, src); if (isReads) { for (const l of labels) paramReads.push(l); - } else { + } else if (isWrites) { for (const l of labels) paramWrites.push(l); + } else { + for (const l of labels) paramThrows.push(l); } } i = open.matchedAt + 1; @@ -757,7 +768,7 @@ function buildArgsTs( out += t.text; i++; } - return { text: out, paramCaps, paramReads, paramWrites }; + return { text: out, paramCaps, paramReads, paramWrites, paramThrows }; } function sliceText(tokens: Token[], from: number, to: number): string { diff --git a/packages/compiler/src/passes/eff-check.ts b/packages/compiler/src/passes/eff-check.ts index df3ef34..049a0f2 100644 --- a/packages/compiler/src/passes/eff-check.ts +++ b/packages/compiler/src/passes/eff-check.ts @@ -14,8 +14,8 @@ * requires closure-level type inference and is out of scope for * this pass. It is reserved for a future version. * - * ?bs 0.9 EFF003 / EFF004 added. Same principle as EFF002 but for - * `reads {}` and `writes {}` annotations on callback parameters. + * ?bs 0.9 EFF003 / EFF004 / THR003 added. Same principle as EFF002 but for + * `reads {}`, `writes {}`, and `throws {}` annotations on callbacks. * * EFF003 A function-typed parameter carries `reads { labels }`, * but the containing fn does not declare those read labels. @@ -23,6 +23,11 @@ * EFF004 A function-typed parameter carries `writes { labels }`, * but the containing fn does not declare those write labels. * + * THR003 A function-typed parameter carries `throws { X }`, + * but the containing fn does not declare `throws { X }`. + * Calling the callback can surface X — the outer fn's + * throws surface must cover it. + * * Background (issue #56): * The pure-intent check (INT001/INT002) and the capability check (CAP001) * both miss the case where an effectful closure is passed to a combinator @@ -135,6 +140,34 @@ export function passEffCheck(src: string, version: VersionInfo): string { }); } } + + // THR003: throws {} on callback not propagated to outer fn. + if (decl.paramThrows.length > 0) { + const declaredThrows = new Set(decl.throws ?? []); + const uniqueMissing = [...new Set(decl.paramThrows.filter((t) => !declaredThrows.has(t)))]; + if (uniqueMissing.length > 0) { + const entry = getErrorCode("THR003")!; + const loc = locationOf(src, decl.fnKeywordStart); + const paramThrowsStr = [...new Set(decl.paramThrows)].join(", "); + diagnostics.push({ + code: "THR003", + severity: "error", + file: null, + line: loc.line, + column: loc.column, + start: decl.fnKeywordStart, + end: decl.fnKeywordStart + decl.name.length + 3, + message: + `fn '${decl.name}' accepts callback parameter(s) that declare throws { ${paramThrowsStr} } ` + + `but only declares throws ${declaredThrows.size > 0 ? `{ ${[...declaredThrows].join(", ")} }` : "{}"} — ` + + `missing: { ${uniqueMissing.join(", ")} }`, + rule: entry.rule, + idiom: entry.idiom, + rewrite: + `fn ${decl.name}(...) throws { ${[...declaredThrows, ...uniqueMissing].join(", ")} } -> ...`, + }); + } + } } } diff --git a/packages/compiler/tests/thr003-check.test.ts b/packages/compiler/tests/thr003-check.test.ts new file mode 100644 index 0000000..b6daf0e --- /dev/null +++ b/packages/compiler/tests/thr003-check.test.ts @@ -0,0 +1,152 @@ +/** + * Tests for THR003: callback throws annotation not propagated to outer fn (?bs 0.9+). + * + * Fires when a function-typed parameter declares `throws { X }` but the containing + * fn does not declare `throws { X }`. Calling the callback can surface X, so the + * outer fn's throws surface must cover it (same principle as EFF003/EFF004). + */ + +import { describe, expect, it } from "vitest"; +import { transform } from "../src/transform.js"; + +function compile(src: string): string { + return transform(src).code; +} + +// --------------------------------------------------------------------------- +// THR003: missing throws from callback parameter +// --------------------------------------------------------------------------- + +describe("THR003: callback throws not propagated", () => { + it("fires when callback declares throws and outer fn declares none", () => { + const src = + "?bs 0.9\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: fn(string) throws { NetworkError } -> void\n" + + ") -> void {\n" + + " handler(items[0])\n" + + "}\n"; + expect(() => compile(src)).toThrow("THR003"); + expect(() => compile(src)).toThrow(/NetworkError/); + }); + + it("fires when callback throws is a superset of outer fn throws", () => { + const src = + "?bs 0.9\n" + + "fn retry(\n" + + " action: fn() throws { NetworkError, TimeoutError } -> string\n" + + ") throws { NetworkError } -> string {\n" + + " action()\n" + + "}\n"; + expect(() => compile(src)).toThrow("THR003"); + expect(() => compile(src)).toThrow(/TimeoutError/); + }); + + it("fires with multiple callback params each declaring throws", () => { + const src = + "?bs 0.9\n" + + "fn run(\n" + + " a: fn() throws { AuthError } -> void,\n" + + " b: fn() throws { NetworkError } -> void\n" + + ") -> void {\n" + + " a()\n" + + " b()\n" + + "}\n"; + expect(() => compile(src)).toThrow("THR003"); + }); +}); + +// --------------------------------------------------------------------------- +// THR003: suppressed when throws surface is covered +// --------------------------------------------------------------------------- + +describe("THR003: suppressed when covered", () => { + it("does not fire when outer fn declares the callback throws", () => { + const src = + "?bs 0.9\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: fn(string) throws { NetworkError } -> void\n" + + ") throws { NetworkError } -> void {\n" + + " handler(items[0])\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire when outer fn over-declares throws", () => { + const src = + "?bs 0.9\n" + + "fn withRetry(\n" + + " action: fn() throws { NetworkError } -> string\n" + + ") throws { NetworkError, TimeoutError } -> string {\n" + + " action()\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire when callback has no throws annotation", () => { + const src = + "?bs 0.9\n" + + "fn withRetry(\n" + + " action: fn() -> string\n" + + ") -> string {\n" + + " action()\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire below ?bs 0.9", () => { + const src = + "?bs 0.8\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: fn(string) throws { NetworkError } -> void\n" + + ") -> void {\n" + + " handler(items[0])\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); +}); + +// --------------------------------------------------------------------------- +// THR003: throws annotation stripped from emitted TypeScript +// --------------------------------------------------------------------------- + +describe("THR003: throws stripped from emitted TS", () => { + it("strips throws from callback parameter type in emitted TypeScript", () => { + const src = + "?bs 0.9\n" + + "fn process(\n" + + " handler: fn(string) throws { NetworkError } -> void\n" + + ") throws { NetworkError } -> void {\n" + + " handler(\"x\")\n" + + "}\n"; + const out = compile(src); + expect(out).not.toContain("throws"); + expect(out).toContain("(string) =>"); + }); +}); + +// --------------------------------------------------------------------------- +// THR003: diagnostic fields +// --------------------------------------------------------------------------- + +describe("THR003: diagnostic code", () => { + it("throws with THR003 code in diagnostics", () => { + const src = + "?bs 0.9\n" + + "fn process(\n" + + " handler: fn(string) throws { NetworkError } -> void\n" + + ") -> void {\n" + + " handler(\"x\")\n" + + "}\n"; + try { + compile(src); + expect.fail("should have thrown"); + } catch (e) { + const err = e as { diagnostics?: Array<{ code: string }> }; + expect(err.diagnostics?.[0]?.code).toBe("THR003"); + } + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 100e08b..e20893a 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -463,6 +463,41 @@ export const EXPLANATIONS: Readonly> = { "fn recordEvent(id: string) writes { metrics } -> void { updateMetrics(id); }\n", }, }, + THR003: { + code: "THR003", + title: "outer fn declares narrower throws than a callback parameter", + body: + "A function-typed parameter can carry a `throws { X }` annotation declaring which " + + "exception types the callback may produce. The outer function that accepts that " + + "callback must declare at least those exception types in its own `throws {}` clause.\n\n" + + "Without this rule, a higher-order fn that accepts a throwing callback can advertise " + + "a narrower throws surface than it can actually exercise. A caller reading the outer " + + "fn's header sees no `throws {}` and concludes the call is infallible — but invoking " + + "the callback may produce NetworkError, ParseError, or any other declared type.\n\n" + + "THR003 is the `throws`-variant of EFF003/EFF004. It gates on `?bs 0.9`. The outer " + + "fn does not need to throw directly — it just needs to declare the exception types so " + + "callers have an accurate throws surface.\n\n" + + "Suppression: add the missing exception type(s) to the outer fn's `throws {}` clause, " + + "or remove the `throws {}` annotation from the callback parameter type if it is " + + "intentionally not propagated (e.g., the callback's exceptions are caught internally).", + example: { + fails: + "?bs 0.9\n" + + "// THR003: process accepts a callback that declares throws { NetworkError },\n" + + "// but process itself declares no throws\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: fn(string) throws { NetworkError } -> void\n" + + ") -> void { handler(items[0]) }\n", + passes: + "?bs 0.9\n" + + "// Fixed: outer fn declares the throws surface its callback may exercise\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: fn(string) throws { NetworkError } -> void\n" + + ") throws { NetworkError } -> void { handler(items[0]) }\n", + }, + }, }; export const KNOWN_CODES = Object.keys(EXPLANATIONS).sort(); diff --git a/packages/mcp/tests/server.test.ts b/packages/mcp/tests/server.test.ts index 5580da3..1dbf6ca 100644 --- a/packages/mcp/tests/server.test.ts +++ b/packages/mcp/tests/server.test.ts @@ -60,6 +60,7 @@ describe("botscript-mcp explanations", () => { "INT002", "RES001", "SYN001", + "THR003", "UNS001", "UNS002", "UNS003",