diff --git a/CHANGELOG.md b/CHANGELOG.md index cc4f9f9..5d1802a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,14 @@ goes behind a new pin. cannot match. Indirect patterns (`err(e)` where `e`'s type is inferred) are out of scope. +- **THR003 — under-declared throws from callback parameter throws annotations.** + From `?bs 0.9`, the compiler fires when a function-typed parameter carries + `throws { X }` but the containing fn does not declare `throws { X }` in + its own header. Calling the callback exercises that throw, so the outer + fn's throws surface must cover it. Direct analogue of EFF003/EFF004 for + the throws dimension. `throws {}` is stripped from emitted TypeScript + (same as `uses {}`, `reads {}`, `writes {}`). Over-declaration is always allowed. + - **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/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index ffe7ddd..822ea97 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -384,6 +384,37 @@ const E: Record = { " else ok(s)\n" + "}", }, + THR003: { + code: "THR003", + title: "fn under-declares throws implied by callback parameter throws annotations", + rule: + "if a function-typed parameter carries `throws { X }`, the containing fn can exercise that " + + "exception through any call to the callback — so the fn's own `throws {}` must cover it; " + + "a fn's throws surface is the union of its own declared throws and the throws its callback " + + "parameters may exercise", + idiom: + "add the callback parameter's throws labels to the containing fn's own `throws { }` clause; " + + "this is the direct analogue of EFF003/EFF004 for the throws surface", + rewrite: + "fn name(...) throws { …existing, CallbackThrown } -> ...", + example: + "// before — process accepts a handler that throws { NetworkError } but doesn't declare it\n" + + "?bs 0.9\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: (s: string) throws { NetworkError } -> void\n" + + ") -> void { // THR003: missing throws { NetworkError }\n" + + " handler(items[0])\n" + + "}\n\n" + + "// after\n" + + "?bs 0.9\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: (s: string) throws { NetworkError } -> void\n" + + ") throws { NetworkError } -> void {\n" + + " handler(items[0])\n" + + "}", + }, }; export function getErrorCode(code: string): ErrorCodeEntry | undefined { diff --git a/packages/compiler/src/parser/parse-fn.ts b/packages/compiler/src/parser/parse-fn.ts index d8d91fb..6e7cf67 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 type names declared in `throws { … }` annotations on + * function-typed parameters. Empty when no parameter carries a throws annotation. + * Used by `passThrCheck` (THR003). Gated on `?bs 0.9`. + * + * Example: `(handler: (s: string) 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, @@ -690,31 +699,34 @@ function skipTrivia(tokens: Token[], i: number): number { /** * Build the TypeScript-compatible args string from the args token range. * - * Three transformations applied: + * Four transformations applied: * 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`. + * 4. `throws { Type, … }` annotations on function-typed parameters are stripped + * from the emitted text and their type names collected into `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). + * `writes { ... }` / `throws { ... }` syntax pattern — a keyword/ident token + * immediately followed by a `{...}` block — not on bare identifiers elsewhere + * in TypeScript type positions (e.g. `reads` as a field name in an object type). */ 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]!; @@ -754,10 +766,22 @@ function buildArgsTs( continue; } } + // Strip `throws { types }` and collect the declared exception types. + if (t.kind === "ident" && t.text === "throws") { + const j = skipTrivia(tokens, i + 1); + const open = tokens[j]; + if (open && open.kind === "open" && open.text === "{" && open.matchedAt !== undefined) { + const types = parseLabelList(tokens, j + 1, open.matchedAt, src); + for (const ty of types) paramThrows.push(ty); + i = open.matchedAt + 1; + while (i < to && tokens[i]?.kind === "whitespace") i++; + continue; + } + } 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/thr-check.ts b/packages/compiler/src/passes/thr-check.ts index 56a9d71..1b2c336 100644 --- a/packages/compiler/src/passes/thr-check.ts +++ b/packages/compiler/src/passes/thr-check.ts @@ -2,8 +2,9 @@ * Throws declaration check (?bs 0.9+). * * Enforces transitivity of `throws { ... }` annotations across same-file - * function calls, and checks that a fn's body does not directly construct - * error types absent from its own `throws {}` declaration. + * function calls, checks that a fn's body does not directly construct error + * types absent from its own `throws {}` declaration, and ensures that + * callback parameters' throws annotations are reflected in the containing fn. * * THR001 throws under-declared: fn A calls fn B which (transitively) * declares `throws { X }` that A does not declare. For a direct @@ -18,6 +19,12 @@ * (`err(e)` where e's type is inferred) are out of scope — token-based * detection only. * + * THR003 callback throws not covered: a function-typed parameter carries + * `throws { X }` but the containing fn's own `throws {}` does not + * include X. Calling the callback can surface X, so the outer fn's + * throws surface must cover it (direct analogue of EFF003/EFF004 for + * the throws dimension). + * * 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. @@ -131,6 +138,17 @@ export function passThrCheck(src: string, version: VersionInfo): string { if (err) throw err; } + // THR003: fn's throws surface must cover all throws declared on callback parameters. + for (const decl of decls) { + if (decl.paramThrows.length === 0) continue; + const declared = new Set(decl.throws ?? []); + const missing = [...new Set(decl.paramThrows)] + .filter((t) => !declared.has(t)) + .sort(); + if (missing.length === 0) continue; + throw mkThr003Error(src, decl, missing, declared); + } + return src; } @@ -300,3 +318,39 @@ function checkBodyErrors( return null; } + +function mkThr003Error( + src: string, + decl: FnDecl, + missingThrows: string[], + declared: Set, +): BotscriptError { + const entry = getErrorCode("THR003")!; + const { line, column } = locationOf(src, decl.fnKeywordStart); + const nameEnd = decl.nameStart + decl.name.length; + + const currentDeclStr = + decl.throws === undefined + ? "no throws clause" + : declared.size === 0 + ? "throws {} (empty)" + : `throws { ${[...declared].sort().join(", ")} }`; + const proposed = [...new Set([...declared, ...missingThrows])].sort().join(", "); + const missingStr = missingThrows.join(", "); + + return new BotscriptError([{ + code: "THR003", + severity: "error" as const, + file: null, + line, + column, + start: decl.fnKeywordStart, + end: nameEnd, + message: + `fn '${decl.name}' accepts callback parameter(s) that together declare throws { ${missingStr} } ` + + `but '${decl.name}' ${decl.throws === undefined ? "has no throws clause" : `declares ${currentDeclStr}`}`, + rule: entry.rule, + idiom: entry.idiom, + rewrite: `fn ${decl.name}(...) throws { ${proposed} } -> ...`, + }]); +} diff --git a/packages/compiler/tests/thr-check.test.ts b/packages/compiler/tests/thr-check.test.ts index dd19886..ef8aa06 100644 --- a/packages/compiler/tests/thr-check.test.ts +++ b/packages/compiler/tests/thr-check.test.ts @@ -294,3 +294,111 @@ describe("THR002: body constructs undeclared error type (0.9+)", () => { expect(() => compile(src)).not.toThrow(); }); }); + +describe("THR003 — callback parameter throws not covered by containing fn", () => { + it("fires when callback parameter declares throws { X } but outer fn has no throws clause", () => { + const src = + "?bs 0.9\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: (s: string) throws { NetworkError } -> void\n" + + ") -> void {\n" + + " handler(items[0])\n" + + "}\n"; + expect(() => compile(src)).toThrow("THR003"); + expect(() => compile(src)).toThrow(/process/); + expect(() => compile(src)).toThrow(/NetworkError/); + }); + + it("fires when callback throws X but outer fn declares throws { Y } (missing X)", () => { + const src = + "?bs 0.9\n" + + "fn apply(\n" + + " f: (s: string) throws { IoError } -> string\n" + + ") throws { ParseError } -> string {\n" + + " f(\"x\")\n" + + "}\n"; + expect(() => compile(src)).toThrow("THR003"); + expect(() => compile(src)).toThrow(/IoError/); + }); + + it("does not fire when outer fn's throws is a superset of callback throws", () => { + const src = + "?bs 0.9\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: (s: 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 (superset)", () => { + const src = + "?bs 0.9\n" + + "fn wrap(\n" + + " f: () throws { IoError } -> void\n" + + ") throws { IoError, ParseError } -> void {\n" + + " f()\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire when callback parameter has no throws annotation", () => { + const src = + "?bs 0.9\n" + + "fn run(\n" + + " action: (s: string) -> void\n" + + ") -> void {\n" + + " action(\"x\")\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire below ?bs 0.9", () => { + const src = + "?bs 0.8\n" + + "fn process(\n" + + " handler: (s: string) throws { NetworkError } -> void\n" + + ") -> void {\n" + + " handler(\"x\")\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("strips throws {} from callback parameter type in emitted TypeScript", () => { + const src = + "?bs 0.9\n" + + "fn wrap(\n" + + " f: (s: string) throws { IoError } -> string\n" + + ") throws { IoError } -> string {\n" + + " f(\"x\")\n" + + "}\n"; + const out = compile(src); + expect(out).not.toContain("throws"); + expect(out).not.toContain("IoError"); + expect(out).toContain("=> string"); + }); + + it("collects throws from multiple callback parameters, fires when any are missing", () => { + const src = + "?bs 0.9\n" + + "fn both(\n" + + " a: () throws { IoError } -> void,\n" + + " b: () throws { ParseError } -> void\n" + + ") -> void {\n" + + " a()\n" + + "}\n"; + expect(() => compile(src)).toThrow("THR003"); + }); + + it("bs explain THR003 entry exists with rule, idiom, and rewrite", async () => { + const { getErrorCode } = await import("../src/error-codes.js"); + const entry = getErrorCode("THR003"); + expect(entry).toBeDefined(); + expect(entry!.rule).toMatch(/throws/); + expect(entry!.idiom).toBeDefined(); + expect(entry!.rewrite).toBeDefined(); + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index e6f59d1..8fe14f6 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -521,6 +521,43 @@ export const EXPLANATIONS: Readonly> = { "}\n", }, }, + THR003: { + code: "THR003", + title: "fn under-declares throws implied by callback parameter throws annotations", + body: + "THR003 fires from `?bs 0.9` when a function-typed parameter carries `throws { X }` " + + "but the containing fn does not declare `throws { X }` in its own header.\n\n" + + "This is the direct analogue of EFF003 (reads on callback) and EFF004 (writes on callback), " + + "applied to the throws surface. When a fn calls its callback parameter, it can exercise the " + + "callback's declared throws — so the outer fn's own `throws {}` must be a superset of all " + + "callback parameters' throws annotations.\n\n" + + "**Why it matters:** a reviewer reading the outer fn's header sees no throws declaration and " + + "has no warning that calling it may produce the error type. Callers that match exhaustively on " + + "the outer fn's return type will have no arm for the undeclared exception — it becomes dead " + + "code or a silent gap.\n\n" + + "**Fix:** add the callback parameter's throws labels to the containing fn's own `throws { }` clause.\n\n" + + "Over-declaration is allowed — if the containing fn declares more throws types than it can " + + "actually exercise, that is harmless (same policy as THR001/THR002).\n\n" + + "THR003 is gated on `?bs 0.9`. Files pinned to earlier versions are unaffected.", + example: { + fails: + "?bs 0.9\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: (s: string) throws { NetworkError } -> void\n" + + ") -> void {\n" + + " handler(items[0])\n" + + "}\n", + passes: + "?bs 0.9\n" + + "fn process(\n" + + " items: string[],\n" + + " handler: (s: string) throws { NetworkError } -> void\n" + + ") throws { NetworkError } -> void {\n" + + " handler(items[0])\n" + + "}\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 868b550..36a8a44 100644 --- a/packages/mcp/tests/server.test.ts +++ b/packages/mcp/tests/server.test.ts @@ -62,6 +62,7 @@ describe("botscript-mcp explanations", () => { "SYN001", "THR001", "THR002", + "THR003", "UNS001", "UNS002", "UNS003",