feat: THR003 — under-declared throws from callback parameter throws annotations (?bs 0.9)#76
Closed
marcelofarias wants to merge 6 commits into
Closed
feat: THR003 — under-declared throws from callback parameter throws annotations (?bs 0.9)#76marcelofarias wants to merge 6 commits into
marcelofarias wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new compiler rule (THR003, ?bs 0.9+) to ensure a function’s declared throws {} surface includes any throws { ... } annotations present on its callback-typed parameters, preventing “callback throws-leak” under-declarations.
Changes:
- Extend fn parsing to strip
throws { ... }from callback parameter type annotations in emitted TypeScript and collect those labels onFnDecl.paramThrows. - Add THR003 to the throws-check pass to diff
paramThrowsvs the containing fn’s declaredthrows {}and emit a diagnostic with rewrite guidance. - Update error code registry, MCP explanations, changelog, and add/extend tests for THR003.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compiler/src/parser/parse-fn.ts | Collects callback parameter throws {} labels into FnDecl.paramThrows and strips them from emitted TS arg types. |
| packages/compiler/src/passes/thr-check.ts | Implements THR003 validation and diagnostic construction. |
| packages/compiler/src/error-codes.ts | Registers THR003 metadata (rule/idiom/rewrite/example). |
| packages/compiler/tests/thr-check.test.ts | Adds coverage for THR003 behavior and TS-emission stripping. |
| packages/mcp/src/explanations.ts | Adds long-form bs explain THR003 content and examples. |
| packages/mcp/tests/server.test.ts | Adds THR003 to the known/exposed explanation codes list. |
| CHANGELOG.md | Documents THR003 behavior and gating. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+336
to
+340
| const missingStr = missingThrows.join(", "); | ||
|
|
||
| const otherMissing = missingThrows.slice(1); | ||
| const otherTail = | ||
| otherMissing.length > 0 |
| from: number, | ||
| to: number, | ||
| ): { text: string; paramCaps: string[] } { | ||
| ): { text: string; paramCaps: string[]; paramThrows: string[] } { |
| * function-typed parameters. Empty when no parameter carries a throws annotation. | ||
| * Used by `passThrCheck` (THR003). | ||
| * | ||
| * Example: `(handler: fn(string) throws { NetworkError } -> void)` → `["NetworkError"]` |
Comment on lines
+691
to
+694
| * The stripping is position-independent: any `uses { }` or `throws { }` inside | ||
| * the args list is treated as a parameter effect annotation. This is safe because | ||
| * `uses` and `throws` are botscript keywords and cannot appear as identifiers in | ||
| * TypeScript types or default value expressions. |
Comment on lines
+365
to
+370
| "// 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: fn(string) throws { NetworkError } -> void\n" + | ||
| ") -> void { // THR003: missing throws { NetworkError }\n" + |
Comment on lines
+489
to
+493
| "?bs 0.9\n" + | ||
| "fn process(\n" + | ||
| " items: string[],\n" + | ||
| " handler: fn(string) throws { NetworkError } -> void\n" + | ||
| ") -> void {\n" + |
Comment on lines
+332
to
+334
| declared.size === 0 | ||
| ? "no throws clause" | ||
| : `throws { ${[...declared].sort().join(", ")} }`; |
| 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); |
marcelofarias
force-pushed
the
botkowski/thr002
branch
from
15260d5 to
1e1fa57
Compare
…nnotations (?bs 0.9)
When a function-typed parameter carries `throws { X }`, the containing fn
can exercise that exception through any call to the callback. THR003 fires
when the outer fn's own `throws {}` does not cover the callback's declared
throws — closing the "callback throws-leak" vector, analogous to EFF003/EFF004
for reads/writes.
- parse-fn.ts: strip `throws {}` from callback param types in buildArgsTs,
collect into FnDecl.paramThrows (same pattern as paramCaps for uses {})
- thr-check.ts: THR003 check — union paramThrows, diff against declared throws,
emit diagnostic with rewrite hint
- error-codes.ts: THR003 entry with rule/idiom/rewrite/example
- explanations.ts: THR003 long-form explanation; add to KNOWN_CODES test
- 9 new tests in thr-check.test.ts; 586/586 pass
Closes #73.
Co-Authored-By: Botkowski <noreply@anthropic.com>
…sTs doc
- mkThr003Error: use firstMissing for primary message + otherTail for extras
(previously missingStr listed all missing AND otherTail repeated all-but-first)
- buildArgsTs: update docstring from "Two" to "Three transformations" to include
the throws {} stripping added for THR003
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…examples and simplify message - Replace `fn(string)` with `(s: string)` in paramThrows example, error-codes example, and explanations example — `fn` is a declaration keyword, not valid as a callback type literal - Fix safety comment in buildArgsTs: `throws` is not a botscript keyword (it's ident-tokenized); reword to accurately describe why stripping is safe - Simplify THR003 message: show all missing throws at once via `missingStr` rather than first + `otherTail` duplication; update test description to match Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… from missing clause
- buildArgsTs now accepts an optional src param and threads it to
parseLabelList, so malformed callback-throws annotations produce
SYN001 consistently (same as header throws {} clauses)
- mkThr003Error now uses decl.throws === undefined vs .length === 0 to
distinguish "no throws clause" from "throws {} (empty)" in the
diagnostic message, preventing the ambiguous "no throws clause" output
for explicit-but-empty declarations
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
marcelofarias
force-pushed
the
botkowski/thr003
branch
from
06f551e to
ddeb421
Compare
Comment on lines
702
to
706
| @@ -697,24 +706,27 @@ function skipTrivia(tokens: Token[], i: number): number { | |||
| * 3. `reads { label, … }` and `writes { label, … }` annotations on | |||
Co-Authored-By: Botkowski <noreply@anthropic.com>
Comment on lines
+332
to
+351
| 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}' declares ${currentDeclStr}`, |
"declares no throws clause" was grammatically awkward; now uses "has no throws clause" for the undefined-throws branch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment on lines
+350
to
+351
| `fn '${decl.name}' accepts callback parameter(s) that together declare throws { ${missingStr} } ` + | ||
| `but '${decl.name}' ${decl.throws === undefined ? "has no throws clause" : `declares ${currentDeclStr}`}`, |
Owner
Author
|
Superseded by #81, which implemented THR003 (callback throws-leak) in eff-check.ts alongside EFF003/EFF004. The thr-check.ts approach in this PR is no longer needed. |
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
Closes #73.
Extends the throws-checking machinery (THR001/THR002) to cover the "callback throws-leak" vector: when a function-typed parameter carries `throws { X }`, the containing fn can exercise that exception through any call to the callback — so the outer fn's `throws {}` must cover it.
This is the direct analogue of EFF003/EFF004 (`reads {}`/`writes {}` on callback params) for the throws surface.
Changes:
Test plan
🤖 Generated with Claude Code