feat: throws {} header annotation + THR001 transitivity enforcement (?bs 0.9)#64
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a declarative throws { ErrorType, ... } header clause to fn declarations and enforces its transitivity from ?bs 0.9 via a new diagnostic THR001. The clause is parsed alongside reads/writes/intent (and stripped from the emitted TS), and the new pass thr-check performs a fixed-point closure over same-file callees mirroring the existing dep-check approach. This is described in the PR as the first body-independent piece of the broader effect-signatures RFC (#14).
Changes:
- Parser support for
throws { Ident, ... }in fn headers, with SYN001 on duplicate clauses and a refined invalid-label diagnostic message. - New
passThrCheckpipeline pass implementing THR001 transitivity via a fixed-point closure, plus aTHR001entry inerror-codes.ts. - A new test file
thr-check.test.tscovering parser, stripping, multi-hop propagation, version gating, and property-access exclusion.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compiler/src/parser/parse-fn.ts | Adds throws?: string[] to FnDecl, accepts throws { ... } in the header loop, and updates doc comments / error wording. |
| packages/compiler/src/passes/thr-check.ts | New pass implementing THR001 transitivity (largely duplicated from dep-check.ts). |
| packages/compiler/src/transform.ts | Registers passThrCheck after passDepCheck in the pipeline (min version 0.9). |
| packages/compiler/src/error-codes.ts | Adds the THR001 error-code entry (title/rule/idiom/rewrite/example). |
| packages/compiler/tests/thr-check.test.ts | Parser + transitivity tests for throws and THR001 (16 cases). |
Comments suppressed due to low confidence (4)
packages/compiler/src/passes/thr-check.ts:292
thr-check.tsduplicates a large amount of plumbing already present indep-check.ts:computeNesting,collectCallees,prevSignificant,nextSignificant,formatPath, the fixed-point closure loop, and most ofmkErrorare line-for-line (or very nearly) copies. Any future fix to call-graph collection (e.g. handling new call syntaxes, generics, decorators, comments) now has to be made in two places, and the two passes also re-parse the program independently. Consider extracting the shared call-graph + path-formatting utilities into a single module (e.g._call-graph.ts) consumed by bothdep-checkandthr-check, and generalizingFnRecord/mkErrorover the label kind ("reads"|"writes"|"throws").
// ---------------------------------------------------------------------------
// Helpers (adapted from dep-check.ts)
// ---------------------------------------------------------------------------
function computeNesting(decls: FnDecl[]): Map<FnDecl, FnDecl[]> {
const inner = new Map<FnDecl, FnDecl[]>();
for (const d of decls) inner.set(d, []);
const sorted = [...decls].sort((a, b) => a.tokenStart - b.tokenStart);
const stack: FnDecl[] = [];
for (const d of sorted) {
while (stack.length > 0 && stack[stack.length - 1]!.tokenEnd <= d.tokenStart) {
stack.pop();
}
for (const ancestor of stack) inner.get(ancestor)!.push(d);
stack.push(d);
}
return inner;
}
function collectCallees(
tokens: Token[],
fn: FnDecl,
inner: FnDecl[],
fnNames: Set<string>,
): Set<string> {
const callees = new Set<string>();
const open: FnDecl[] = [];
let nextInner = 0;
for (let i = 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];
if (!tok || tok.kind !== "ident") continue;
if (!fnNames.has(tok.text)) continue;
if (tok.text === fn.name) continue;
const prevIdx = prevSignificant(tokens, i - 1);
const prev = tokens[prevIdx];
if (prev && ((prev.kind === "punct" && prev.text === ".") || prev.kind === "questionDot"))
continue;
const nextIdx = nextSignificant(tokens, i + 1);
const next = tokens[nextIdx];
if (!next || next.kind !== "open" || next.text !== "(") continue;
callees.add(tok.text);
}
return callees;
}
function nextSignificant(tokens: Token[], start: number): number {
let i = start;
while (i < tokens.length) {
const t = tokens[i];
if (!t) return i;
if (
t.kind === "whitespace" ||
t.kind === "newline" ||
t.kind === "lineComment" ||
t.kind === "blockComment"
) {
i++;
continue;
}
return i;
}
return i;
}
function prevSignificant(tokens: Token[], start: number): number {
let i = start;
while (i >= 0) {
const t = tokens[i];
if (!t) return i;
if (
t.kind === "whitespace" ||
t.kind === "newline" ||
t.kind === "lineComment" ||
t.kind === "blockComment"
) {
i--;
continue;
}
return i;
}
return i;
}
function formatPath(path: ThrPath): string {
const segments: string[] = [];
let cur: ThrPath = path;
while (cur.kind === "via") {
segments.push(cur.fnName);
cur = cur.next;
}
segments.push(cur.fnName);
return segments.join(" -> ");
}
function mkError(src: string, rec: FnRecord, missingLabels: string[]): BotscriptError {
const entry = getErrorCode("THR001")!;
const { line, column } = locationOf(src, rec.decl.fnKeywordStart);
const firstLabel = missingLabels[0]!;
const firstPath = rec.transitiveThrows.get(firstLabel)!;
const pathStr = formatPath(firstPath);
const leaf = pathStr.split(" -> ").at(-1);
const directCall = firstPath.kind === "via" && firstPath.next.kind === "declared";
const transitively = directCall ? "" : " transitively";
const displayPath =
!directCall && firstPath.kind === "via"
? formatPath(firstPath.next)
: pathStr;
const currentDecl =
rec.declaredThrows.size === 0
? "(none)"
: [...rec.declaredThrows].join(", ");
const proposed = [...new Set([...rec.declaredThrows, ...missingLabels])].join(", ");
const otherMissing = missingLabels.slice(1);
const otherTail =
otherMissing.length > 0
? `; also missing: ${otherMissing.map((l) => `"${l}"`).join(", ")}`
: "";
const callDescription = directCall
? `'${leaf}' which throws { ${firstLabel} }`
: `${displayPath} — '${leaf}' throws { ${firstLabel} }`;
const message =
`fn '${rec.decl.name}'${transitively} calls ${callDescription}, ` +
`but '${rec.decl.name}' only declares throws { ${currentDecl} }${otherTail}`;
const callPath = `call path: ${pathStr}`;
const nameEnd = rec.decl.nameStart + rec.decl.name.length;
const diagnostic = {
code: "THR001",
severity: "error" as const,
file: null,
line,
column,
start: rec.decl.fnKeywordStart,
end: nameEnd,
message,
rule: entry.rule,
idiom: entry.idiom,
rewrite: `fn ${rec.decl.name}(...) throws { ${proposed} } -> ... // ${callPath}`,
};
return new BotscriptError([diagnostic]);
}
packages/compiler/src/passes/thr-check.ts:122
- The order in which fns are validated, and the order of missing labels reported for a given fn, both depend on
Mapinsertion order overrecords/transitiveThrows. The "first missing label" reported is therefore tied to source order of seeding (own declared throws first, then propagation order acrossrec.calleeswhich is aSet). For a fn missing several labels, the choice of which one becomes the primarycallDescriptionvs. which goes intoalso missingis implicit and never sorted — diagnostics may shift if unrelated source is reordered. Consider sortingmissing(and ideally selecting the path with the shortest hop count) before constructing the error so messages are stable across refactors. Same observation applies todep-check.ts, but introducingthr-checkis a good moment to fix it before three passes ship with the behavior.
for (const rec of records.values()) {
const missing = [...rec.transitiveThrows.keys()].filter(
(l) => !rec.declaredThrows.has(l),
);
if (missing.length > 0) {
throw mkError(src, rec, missing);
}
}
packages/compiler/src/passes/thr-check.ts:185
collectCalleesmatches anyidentwhose text is infnNamesand that is followed by(, regardless of surrounding lexical context. Because the scan covers the entire fn token range, it can pick up identifiers that look like calls but aren't body calls — most notably tokens inside the parameter list and return-type annotation (e.g. a default value likeparam: Foo = fetchRemote(), or a type position such as-> ReturnType<typeof fetchRemote>iffetchRemotehappened to be both a fn and a type). Forthrowsthis matters because over-declaration is silently accepted (good) but spurious THR001 errors are not. The same issue is latent indep-check.ts, butthrowsadds a second pass with the identical blind spot. Consider scanning only from the body-start offset (after->/=) rather than fromfn.tokenStart. At minimum, please add a regression test for a default-parameter-value scenario.
const callees = new Set<string>();
const open: FnDecl[] = [];
let nextInner = 0;
for (let i = 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];
if (!tok || tok.kind !== "ident") continue;
if (!fnNames.has(tok.text)) continue;
if (tok.text === fn.name) continue;
const prevIdx = prevSignificant(tokens, i - 1);
const prev = tokens[prevIdx];
if (prev && ((prev.kind === "punct" && prev.text === ".") || prev.kind === "questionDot"))
continue;
const nextIdx = nextSignificant(tokens, i + 1);
const next = tokens[nextIdx];
if (!next || next.kind !== "open" || next.text !== "(") continue;
callees.add(tok.text);
}
return callees;
}
packages/compiler/tests/thr-check.test.ts:86
- There is no test that exercises the multi-hop branch of
mkError(wherefirstPath.kind === "via"andfirstPath.next.kind !== "declared"). The existing "transitive (multi-hop)" test hasfetchUserdeclarethrows { HttpError }, so fromloadUser's perspective the path isloadUser -> fetchUserwithfetchUserbeing the declaring leaf — which is the direct-call branch inmkError, not the multi-hop branch. Thetransitively/displayPathformatting and the"B -> C — 'C' throws { X }"shape described in the module docstring are untested. Please add a case likeA -> B -> Cwhere onlyCdeclares the throws and assert the error message contains the path arrow form.
it("fires when a caller omits a transitive (multi-hop) throws label", () => {
const src =
"?bs 0.9\n" +
"fn callApi(id: string) throws { HttpError } -> string = id\n" +
"fn fetchUser(id: string) throws { HttpError } -> string = callApi(id)\n" +
"fn loadUser(id: string) -> string = fetchUser(id)\n";
expect(() => compile(src)).toThrow("THR001");
expect(() => compile(src)).toThrow(/loadUser/);
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const src = "fn fetchRemote(id: string) throws { HttpError } -> string = id\n"; | ||
| const out = compile(src); | ||
| expect(out).not.toContain("throws"); | ||
| expect(out).not.toContain("HttpError"); |
| // order between `uses {}` and `->`. All are metadata: stripped from TS output. | ||
| let reads: string[] | undefined; | ||
| let writes: string[] | undefined; | ||
| let throws_: string[] | undefined; |
| * | ||
| * 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. |
| // order between `uses {}` and `->`. All are metadata: stripped from TS output. | ||
| let reads: string[] | undefined; | ||
| let writes: string[] | undefined; | ||
| let throws_: string[] | undefined; // underscore: `throws` is a JS reserved word; FnDecl field is `throws` (unsuffixed) |
Comment on lines
+264
to
+268
| if (tok?.kind === "ident" && (tok.text === "reads" || tok.text === "writes" || tok.text === "throws")) { | ||
| const keyword = tok.text; | ||
| const isDuplicate = (keyword === "reads" && reads !== undefined) || | ||
| (keyword === "writes" && writes !== undefined); | ||
| (keyword === "writes" && writes !== undefined) || | ||
| (keyword === "throws" && throws_ !== undefined); |
11 tasks
Comment on lines
586
to
+589
| continue; | ||
| } | ||
| // Non-identifier, non-separator token inside a reads/writes list. | ||
| throwSyn001(src, t, `invalid label in reads/writes list — labels must be plain identifiers (e.g. \`cache\`, \`db\`), not ${JSON.stringify(t.text)}`); | ||
| throwSyn001(src, t, `invalid label in reads/writes/throws list — labels must be plain identifiers (e.g. \`cache\`, \`HttpError\`), not ${JSON.stringify(t.text)}`); |
Comment on lines
+131
to
+137
| // --------------------------------------------------------------------------- | ||
| // Helpers (adapted from dep-check.ts) | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| function computeNesting(decls: FnDecl[]): Map<FnDecl, FnDecl[]> { | ||
| const inner = new Map<FnDecl, FnDecl[]>(); | ||
| for (const d of decls) inner.set(d, []); |
| import type { FnDecl } from "../parser/parse-fn.js"; | ||
| import { atLeast, type VersionInfo } from "./version.js"; | ||
| import { locationOf } from "./_location.js"; | ||
| import { computeNesting, collectCallees, nextSignificant, prevSignificant } from "./_callgraph.js"; |
Comment on lines
+12
to
+19
| /** | ||
| * Build a map from each FnDecl to its direct inner (nested) FnDecls. | ||
| * | ||
| * A single sweep over `decls` sorted by `tokenStart` replaces a per-fn | ||
| * `decls.filter` (which is O(n²) overall). Fn ranges are properly nested — | ||
| * never partially overlapping — so a stack of "currently open" ancestors is | ||
| * sufficient: every decl is appended to each ancestor still on the stack. | ||
| * Each returned `inner` list is in ascending `tokenStart` order. |
Comment on lines
+77
to
+78
| * element is an ident naming an exception type the function (or its callees) may throw. | ||
| * Transitively enforced from `?bs 0.9` (THR001). Stripped from TS output. |
3 tasks
marcelofarias
pushed a commit
that referenced
this pull request
May 20, 2026
THR001 only propagates across same-file statically resolved callees, not cross-module or dynamic dispatch. Update the FnDecl.throws docstring to say so explicitly, addressing Copilot review feedback on PR #64. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…?bs 0.9)
Adds declarative `throws { ErrorType, ... }` clause to fn headers (parsed
alongside reads/writes/intent). Transitively enforced from ?bs 0.9 via
THR001: if fn A calls fn B and B declares throws { X }, A must also
declare throws { X }. Stripped from TS output like reads/writes.
- parse-fn.ts: parses throws {} into FnDecl.throws; duplicate clause
rejected as SYN001; error message updated to include throws in label
- error-codes.ts: THR001 entry with rule, idiom, rewrite, example
- passes/thr-check.ts: fixed-point transitive closure over same-file calls;
same approach as dep-check (computeNesting, collectCallees); over-declaration allowed
- transform.ts: wire passThrCheck at ?bs 0.9 after depCheck
- tests/thr-check.test.ts: 16 tests (parser + transitivity enforcement)
563/563 tests pass. Closes part of #14 (total effect signatures RFC).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- thr-check.test.ts: tighten stripped-output assertion to check `throws {`
syntax specifically (not just the `throws` keyword substring)
- parse-fn.ts: add comment explaining `throws_` trailing underscore
- thr-check.ts: add NOTE to module docstring clarifying transitivity-only
guarantee and pointing to #14 for body-level soundness
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`throws` is not a JS reserved word (`throw` is). The trailing underscore avoids shadowing the FnDecl field `throws`, not a keyword collision. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Strengthen strip test: assert fetchRemote IS in output, not just that
HttpError and throws are absent (tightens the intent of the check)
- Update SYN001 rule to mention throws {} alongside reads {} / writes {}
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Moves computeNesting, collectCallees, nextSignificant, and prevSignificant
out of dep-check.ts and thr-check.ts into a shared passes/_callgraph.ts
module. Fixes to call-resolution logic now apply to all passes from one
place. Also updates the parseLabelList docstring to mention throws {} in
addition to reads/writes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove unused nextSignificant/prevSignificant imports from dep-check.ts (those helpers now live in _callgraph.ts, dep-check only needs computeNesting/collectCallees) - Fix stale inline comment in parseLabelList (reads/writes -> reads/writes/throws) - Add 3-hop multi-hop test that exercises the transitively/displayPath branch of mkError Co-Authored-By: Botkowski <noreply@anthropic.com>
…just direct children Co-Authored-By: Botkowski <noreply@anthropic.com>
THR001 only propagates across same-file statically resolved callees, not cross-module or dynamic dispatch. Update the FnDecl.throws docstring to say so explicitly, addressing Copilot review feedback on PR #64. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
marcelofarias
force-pushed
the
botkowski/throws-header
branch
from
cc26e77 to
f452f91
Compare
Comment on lines
+120
to
+122
| const missing = [...rec.transitiveThrows.keys()].filter( | ||
| (l) => !rec.declaredThrows.has(l), | ||
| ); |
This was referenced May 20, 2026
…sharpen stripping test
- Sort missingLabels (and the proposed set) by label name before
building the error message so THR001 output is stable across Map
insertion-order variations. Matches dep-check's existing sort policy.
- Clarify the throws-stripping test: change not.toContain('throws') to
not.toContain('throws {') (more specific clause check) and add a
comment explaining why the HttpError absence assertion is meaningful
(the name only appears in the throws clause, not in the fn signature).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment on lines
+163
to
+166
| const currentDecl = | ||
| rec.declaredThrows.size === 0 | ||
| ? "(none)" | ||
| : [...rec.declaredThrows].join(", "); |
| "each fn header clause (reads {}, writes {}, throws {}, intent:) may appear at most once; " + | ||
| "labels inside reads/writes/throws must be plain identifiers, not quoted strings", | ||
| idiom: | ||
| "declare each resource dependency or intent claim exactly once; " + |
…idiom
When a caller has no throws {} clause, THR001's error message was
saying "only declares throws { (none) }" — invalid syntax that
confuses users trying to use the rewrite hint. Now renders as
"only declares throws { }" (valid empty clause).
Also updates SYN001 idiom to mention throws declarations alongside
resource dependencies and intent claims, since throws {} duplicates
are also covered by SYN001.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment on lines
+163
to
+166
| const currentDecl = [...rec.declaredThrows].join(", "); | ||
|
|
||
| const proposed = [...new Set([...rec.declaredThrows, ...missingLabels])].sort().join(", "); | ||
|
|
Empty declaredThrows was rendering as `throws { }` (joined empty set).
Now renders as "declares no throws clause" so the message is readable and
doesn't produce syntactically misleading output.
Co-Authored-By: Botkowski <noreply@anthropic.com>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
throws { ErrorType, ... }clause to fn headers (parsed alongsidereads/writes/intent, stripped from TS output)?bs 0.9via THR001: if fn A calls fn B and B declaresthrows { X }, A must also declarethrows { X }?bs 0.9 fn fetchRemote(id: string) throws { HttpError } -> string = id fn loadUser(id: string) throws { HttpError } -> string = fetchRemote(id) // clean fn badCaller(id: string) -> string = fetchRemote(id) // THR001Closes part of #14 (total effect signatures RFC —
throws {}is the first body-independent piece).Test plan
pnpm -r build && pnpm test— 563/563 pass (16 new tests inthr-check.test.ts)bs explain THR001returns rule/idiom/rewritethrows {}at?bs 0.9→ THR001throws {}→ cleanthrows {}clause → SYN001throws {}stripped from TS output (nothrowsor error type name in emitted code)?bs 0.9obj.helper) not flagged as same-file callees🤖 Generated with Claude Code