Skip to content

feat: THR002 — undeclared error type construction (?bs 0.9)#66

Open
marcelofarias wants to merge 9 commits into
mainfrom
botkowski/thr002
Open

feat: THR002 — undeclared error type construction (?bs 0.9)#66
marcelofarias wants to merge 9 commits into
mainfrom
botkowski/thr002

Conversation

@marcelofarias
Copy link
Copy Markdown
Owner

Summary

  • Adds THR002: from ?bs 0.9, fires when a fn body contains err(TypeName(...)) or err(new TypeName(...)) where TypeName (CapCase ident) is not in the fn's own throws {} clause.
  • Producer-side complement to THR001: ensures fns declare what they actually construct, not just what transitively bubbles from callees.
  • Indirect patterns (err(e) where e has a type) are intentionally out of scope — token-based detection only.
  • Stacks on top of PR feat: throws {} header annotation + THR001 transitivity enforcement (?bs 0.9) #64 (throws {} / THR001).

Closes #65.

How it works

Token scan of each fn body (same nesting exclusion as THR001):

  • err(HttpError("msg")) → detects HttpError (CapCase ident followed by ()
  • err(BuildError) → detects BuildError (CapCase ident followed by ))
  • err(new ParseError(...)) → detects ParseError (after new)
  • err(e) → ignored (lowercase, not CapCase)

Test plan

  • pnpm -r build && pnpm test — 574/574 pass (11 new tests in thr-check.test.ts)
  • err(CapCase(...)) not in throws {} → THR002
  • err(new CapCase(...)) not in throws {} → THR002
  • err(CapCase) bare ref not in throws {} → THR002
  • Type declared in throws {} → clean
  • err(lowercase) → no THR002 (indirect, out of scope)
  • Property access obj.err(...) → no THR002
  • Inner fn with declared type → no THR002 for outer
  • Below ?bs 0.9 → no THR002
  • bs explain THR002 returns rule/idiom/rewrite
  • MCP KNOWN_CODES updated (THR001 + THR002 added)

🤖 Generated with Claude Code

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds THR002, the producer-side complement to THR001 (PR #64), which fires from ?bs 0.9 when a fn body directly constructs an error type (err(TypeName(...)), err(new TypeName(...)), or bare err(TypeName)) that is not present in the fn's own throws {} declaration. Detection is token-based with the same inner-fn nesting exclusion used by THR001; indirect patterns like err(e) are intentionally out of scope.

Changes:

  • Add checkBodyErrors pass in thr-check.ts that scans each fn body's tokens for err(CapCaseIdent...) constructs not in the declared throws set, then raises a THR002 diagnostic with a throws { … } rewrite hint.
  • Register THR002 in error-codes.ts and explanations.ts, and add it to the MCP known-codes test.
  • Add 11 vitest cases covering positive/negative detection (direct call, new, bare ref, lowercase, property access, inner fn, version gate) and update CHANGELOG.md.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/compiler/src/passes/thr-check.ts Implements THR002 body-scan check; renames mkError to mkThr001Error; updates header doc.
packages/compiler/src/error-codes.ts Adds THR002 entry (rule/idiom/rewrite/example).
packages/compiler/tests/thr-check.test.ts Adds 11 THR002 test cases.
packages/mcp/src/explanations.ts Adds THR001 and THR002 explanation entries.
packages/mcp/tests/server.test.ts Adds THR001/THR002 to the KNOWN_CODES expectation.
CHANGELOG.md Documents THR001 and THR002 under ?bs 0.9.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment on lines +227 to +233
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;
Comment on lines +275 to +289
const { line, column } = locationOf(src, tok.start);
const currentDecl =
declaredThrows.size === 0 ? "(none)" : [...declaredThrows].join(", ");
const proposed = [...new Set([...declaredThrows, typeName])].join(", ");
const nameEnd = fn.nameStart + fn.name.length;

return new BotscriptError([{
code: "THR002",
severity: "error" as const,
file: null,
line,
column,
start: fn.fnKeywordStart,
end: nameEnd,
message:
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

packages/compiler/src/passes/thr-check.ts:213

  • checkBodyErrors’s docstring lists only constructor-call forms, but the code also treats err(TypeName) (bare ref) as a violation. Update the function comment so it matches the behavior (and the THR002 explanation/tests).
/**
 * THR002: scan fn body for `err(TypeName(...))` or `err(new TypeName(...))`
 * where TypeName (CapCase ident) is not in the fn's own `throws {}` set.
 * Returns a BotscriptError on the first violation found, or null.
 */

Comment on lines +13 to +18
* THR002 undeclared error construction: fn body contains `err(TypeName(...))`
* or `err(new 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.
Comment thread packages/compiler/src/error-codes.ts Outdated
Comment on lines +328 to +333
"if a fn body contains `err(TypeName(...))` or `err(new 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)` are out of scope — only direct constructor calls are checked",
Comment thread CHANGELOG.md Outdated
Comment on lines +19 to +22
`err(TypeName(...))` or `err(new 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.
* 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(...)) where TypeName is not in throws {}.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comment on lines 119 to 121
const missing = [...rec.transitiveThrows.keys()].filter(
(l) => !rec.declaredThrows.has(l),
);
const { line, column } = locationOf(src, tok.start);
const currentDecl =
declaredThrows.size === 0 ? "(none)" : [...declaredThrows].join(", ");
const proposed = [...new Set([...declaredThrows, typeName])].join(", ");
Comment thread packages/mcp/src/explanations.ts Outdated
Comment on lines +440 to +442
"THR002 fires from `?bs 0.9` when a fn body contains `err(TypeName(...))` or " +
"`err(new TypeName(...))` where TypeName (a CapCase identifier) is not present in " +
"the fn's own `throws { }` clause.\n\n" +
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment on lines +169 to +188
@@ -179,7 +185,7 @@ function mkError(src: string, rec: FnRecord, missingLabels: string[]): Botscript

const message =
`fn '${rec.decl.name}'${transitively} calls ${callDescription}, ` +
`but '${rec.decl.name}' only declares throws { ${currentDecl} }${otherTail}`;
`but '${rec.decl.name}' declares ${currentDeclStr}${otherTail}`;
Comment on lines +277 to +293
const currentDeclStr =
declaredThrows.size === 0
? "no throws clause"
: `throws { ${[...declaredThrows].sort().join(", ")} }`;
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:
`fn '${fn.name}' constructs err(${typeName}...) but '${typeName}' ` +
`is not in ${currentDeclStr}`,
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment on lines +281 to +285
const currentDeclStr =
declaredThrows.size === 0
? "no throws clause"
: `throws { ${[...declaredThrows].sort().join(", ")} }`;
const proposed = [...new Set([...declaredThrows, typeName])].sort().join(", ");
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@marcelofarias marcelofarias changed the base branch from botkowski/throws-header to main May 21, 2026 13:41
@marcelofarias marcelofarias requested a review from Copilot May 21, 2026 13:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Marcelo Farias and others added 7 commits May 22, 2026 02:44
Fires when a fn body contains err(TypeName(...)) or err(new TypeName(...))
where TypeName (CapCase ident) is absent from the fn's own throws {} clause.

Producer-side complement to THR001: ensures the fn declares what it actually
constructs, not just what transitively bubbles up. Callers' exhaustive match
arms for the undeclared type would otherwise be permanently dead code.

Scope: token-based, direct construction only. Indirect patterns (err(e) where
e's type is inferred) are out of scope. Version-gated at ?bs 0.9.

11 new tests in thr-check.test.ts, 574/574 pass.

Closes #65.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…o _callgraph.ts

After the _callgraph.ts refactor (rebased from throws-header), thr-check.ts
still relied on locally-defined nextSignificant/prevSignificant and Token that
no longer exist. Add the missing imports from _callgraph.js and lex.js.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rr token

- checkBodyErrors() now starts at fn.bodyTokenStart ?? fn.tokenStart instead
  of fn.tokenStart, matching the pattern used by collectCallees in _callgraph.ts.
  This avoids false THR002 hits on CapCase names in parameter types or the
  return type annotation (e.g. a param typed NetworkError would have incorrectly
  triggered the check).
- THR002 diagnostic now uses tok.start/end as both the line/column anchor and
  the reported span, so editors highlight the actual err() call rather than
  the fn header. Previously line/column pointed at err but start/end pointed
  at the fn keyword, which would produce confusing double-location diagnostics.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ptions

All comment/doc strings for THR002 previously listed only
err(TypeName(...)) and err(new TypeName(...)); the bare-reference
form err(TypeName) (where the ident is followed directly by ')') is
also detected. Updated file header, checkBodyErrors docstring,
error-codes.ts rule/idiom, CHANGELOG, and test file comment to
reflect the full detection surface.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nd THR002 explanation

- THR001: sort missing labels for deterministic diagnostic output
- THR001/THR002: render empty throws as "no throws clause" instead of "(none)"
  or empty-joined string — clearer prose, not pseudo-syntax
- THR002: sort proposed labels in rewrite hint
- explanations.ts: mention bare err(TypeName) in THR002 opening sentence

Co-Authored-By: Botkowski <noreply@anthropic.com>
- THR001: replace "declares <currentDeclStr>" with "only declares throws {…}"
  or "has no throws clause" to avoid the awkward "declares no throws clause"
  phrasing flagged by Copilot
- THR002: when fn has no throws clause, emit "but has no throws clause"
  instead of "is not in no throws clause" (grammatically broken)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Variable was computed but never referenced in the diagnostic message
construction. Message already handles the empty-throws case inline
on the conditional expression directly below.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment thread README.md
| `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 a stable diagnostic code (`BS001`, `BS002`, `CAP001`, `CAP002`, `UNS001`–`UNS004`, `RES001`, `FMT001`, `INT001`, `SYN001`, `THR002`) plus a fails/passes example pair. |
Comment thread AGENTS.md
| RES001 | (0.3+) `Result.try` / `Result.tryAsync` with no body. | `Result.try { <body that may throw> }`. |
| 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. |
| 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. |
Comment on lines +291 to +295
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,
- Fix THR001 message: "only declares throws { X }" → "has throws { X } but not { Y }"
  to avoid redundant "declares" + "throws" phrasing
- Add THR001 row to AGENTS.md diagnostic codes table (was missing alongside THR002)
- Add THR002 test: fn with no throws clause constructs err(TypeName) → message says
  "has no throws clause"
- 608/608 tests pass
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread README.md
| `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 a stable diagnostic code (`BS001`, `BS002`, `CAP001`, `CAP002`, `UNS001`–`UNS004`, `RES001`, `FMT001`, `INT001`, `SYN001`, `THR002`) plus a fails/passes example pair. |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RFC: THR002 — under-declared throws from direct error construction

2 participants