diff --git a/AGENTS.md b/AGENTS.md index 457e4ad..0244d6e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -190,10 +190,11 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | UNS002 | (0.3+) `unsafe "" { … }` — empty justification. (0.5+) Also fires on a declaration-level `unsafe "" fn name(…)` with an empty reason. | Replace `""` with a one-sentence reason. | | UNS003 | (0.3+) `unsafe "reason"` with no following body. | `unsafe "reason" { }`. | | UNS004 | (0.5+) Bare `as` cast outside an `unsafe "" { ... }` block or an `unsafe "reason" fn` body. Every cast must be justified. `import * as ns`, `import { foo as bar }`, and `export * as ns` are not flagged. | `unsafe "" { as }`, or declare the fn as `unsafe "reason" fn name(…)` when the fn is the module's one safe coercion point. | +| UNS005 | (0.9+) A stdlib capability call (`http.x`, `fs.x`, `time.x`, `random.x`, `stdout.x`, `stderr.x`) appears in a fn body with no declared result contract at the call site. | Wrap in `match ns.method(...) { ok { v } -> ... err { e } -> ... }`, use `unsafe "" { ... }`, or declare the fn as `unsafe "" fn`. | | FMT001 | (0.4+) Source is not in canonical form (RFC #13). Every program has exactly one canonical surface form; from `?bs 0.4` on, the compiler rejects whitespace / ordering variants rather than silently accepting them. The diagnostic points at the first UTF-16 code unit that differs from canonical. | `botscript fmt --write`. | | RES001 | (0.3+) `Result.try` / `Result.tryAsync` with no body. | `Result.try { }`. | | 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. | +| SYN001 | Duplicate fn header clause (e.g. two `reads { }` on the same fn, or two `intent:`, or two `throws {}`), or a label inside `reads {}` / `writes {}` / `throws {}` 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. | | INT002 | (0.7+) A fn declares `intent: "pure"` but its body directly references a stdlib capability (e.g. `http.get`, `fs.read`). Pure intent is enforced at the body level as well as the header. | Remove the stdlib call from the body, or change the intent. | | CAP003 | (0.9+, warning) A fn is declared `unsafe "reason" fn name(…)` and also has a `uses { … }` clause. The compiler cannot prove the capability is actually reached — the assertion is programmer-owned. Non-blocking; the fn compiles. | Remove the `uses {}` clause if it is not needed, or document why the assertion is trusted. | | EFF002 | (0.7+) A callback parameter declares `uses { … }` capabilities beyond what the outer fn declares. A fn that claims `uses { net }` cannot safely accept a callback that also writes to `fs` — the outer declaration would be a lie. | Extend the outer fn's `uses {}` to cover the callback's full capability set, or narrow the callback's annotation. | diff --git a/CHANGELOG.md b/CHANGELOG.md index 18e56ad..209340a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,19 @@ goes behind a new pin. - THR003 fires when any callback parameter's declared throws are not a subset of the outer fn's declared throws. +- **UNS005 — external call without declared result contract.** + From `?bs 0.9`, the compiler fires `UNS005` when a stdlib capability call + (`http.x`, `fs.x`, `time.x`, `random.x`, `stdout.x`, `stderr.x`) has no + declared result contract at the call site. The return value may be + structurally typed correctly but semantically incorrect in ways the compiler + cannot detect — UNS005 forces explicit handling. + - Unlike UNS001–UNS004 (programmer-applied), UNS005 is **compiler-inferred**. + - Suppress by making the call the direct subject of a `match` expression: + `match ns.method(...) { ok { v } -> ... err { e } -> ... }` (including `match await ...`). + - Suppress with `unsafe "" { ns.method(...) }` to accept the + uncertainty with a written explanation. + - `unsafe "" fn` declaration bodies are also suppressed. + - **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 @@ -83,10 +96,9 @@ goes behind a new pin. unaffected; the new field is additive. ### Compat -- Files on `?bs 0.8` (or earlier) are unaffected — DEP001/DEP002 and CAP003 - are gated on `?bs 0.9`. Existing code that uses `reads {}` / `writes {}` or - `unsafe fn` without transitivity/assertion warnings continues to compile at - its current pin. +- Files on `?bs 0.8` (or earlier) are unaffected — DEP001/DEP002, UNS005, and + CAP003 are gated on `?bs 0.9`. Existing code continues to compile at its + current pin. ## ?bs 0.8 — unreleased diff --git a/README.md b/README.md index 187e456..2838051 100644 --- a/README.md +++ b/README.md @@ -177,8 +177,8 @@ claude mcp add botscript -- npx -y @mbfarias/botscript-mcp | Tool | Input | Output | | ----------- | -------------------------------------- | --------------------------------------------------------------------------------------------------- | | `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 any stable diagnostic code (`BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`, `DEP002`, `EFF002`–`EFF004`, `FMT001`, `INT001`, `INT002`, `MAT001`, `RES001`, `SYN001`, `THR001`–`THR003`, `UNS001`–`UNS004`, `VER001`–`VER002`) plus a fails/passes example pair. | +| `transform` | `{ source: string, filename?: string }` | `{ ok: true, code, forms, version, warnings: [...] }` on success, or `{ ok: false, diagnostics: [...] }` on failure. `warnings` is an array of non-blocking diagnostics (e.g. CAP003). | +| `explain` | `{ code: string }` | Long-form explanation for any stable diagnostic code (`BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`, `DEP002`, `EFF002`–`EFF004`, `FMT001`, `INT001`, `INT002`, `MAT001`, `RES001`, `SYN001`, `THR001`–`THR003`, `UNS001`–`UNS005`, `VER001`–`VER002`) plus a fails/passes example pair. | A bot's loop becomes deterministic: `transform` → if `ok=false`, read `diagnostics[0].code` → `explain(code)` → apply `rewrite` → `transform` again. diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index 9bf81d2..5422236 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -77,8 +77,11 @@ const E: Record = { "}\n\n" + "// No CAP003: regular fn with the same claim is compiler-verified\n" + "?bs 0.9\n" + - "fn callApi(url: string) uses { net } -> string {\n" + - " http.get(url) // CAP001/CAP002 apply normally\n" + + "fn callApi(url: string) uses { net } -> Result {\n" + + " match http.get(url) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(`fetch failed: ${error}`)\n" + + " }\n" + "}", }, UNS001: { @@ -143,6 +146,36 @@ const E: Record = { "?bs 0.5\n" + 'const u = unsafe "Response.json() returns any" { data as User };', }, + UNS005: { + code: "UNS005", + title: "external call without declared result contract", + rule: + "a stdlib capability call (http.x, fs.x, time.x, etc.) must have a declared result contract " + + "at the call site — wrap in `match` to make success and failure paths explicit, use " + + "`unsafe \"\" { ... }` to accept the uncertainty with a written explanation, or " + + "declare the containing fn as `unsafe \"\" fn` when the entire body is the escape hatch", + idiom: + "prefer match over bare stdlib calls — " + + "`match ns.method(...)` makes both success and failure paths explicit; " + + "use `unsafe` only when you are certain about the shape and want to document why", + rewrite: + "match ns.method(...) {\n ok { value } -> { /* use value */ }\n err { error } -> { /* handle error */ }\n}", + example: + "// before — UNS005: no contract on what http.get returns\n" + + "?bs 0.9\n" + + "fn fetchUser(id: string) uses { net } -> string {\n" + + " const data = http.get(`/users/${id}`);\n" + + " data\n" + + "}\n\n" + + "// after — result contract via match\n" + + "?bs 0.9\n" + + "fn fetchUser(id: string) uses { net } -> Result {\n" + + " match http.get(`/users/${id}`) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(`fetch failed: ${error}`)\n" + + " }\n" + + "}", + }, FMT001: { code: "FMT001", title: "source is not in canonical form", diff --git a/packages/compiler/src/passes/cap-check.ts b/packages/compiler/src/passes/cap-check.ts index 879c0a5..7429d95 100644 --- a/packages/compiler/src/passes/cap-check.ts +++ b/packages/compiler/src/passes/cap-check.ts @@ -28,8 +28,8 @@ import { locationOf } from "./_location.js"; import { nextSignificant } from "./_callgraph.js"; import { atLeast, type VersionInfo } from "./version.js"; -/** stdlib namespace -> capability it consumes. */ -const STDLIB_TO_CAP: Readonly> = { +/** stdlib namespace -> capability it consumes. Canonical source; import from here to avoid drift. */ +export const STDLIB_TO_CAP: Readonly> = { http: "net", time: "time", random: "random", diff --git a/packages/compiler/src/passes/intent-check.ts b/packages/compiler/src/passes/intent-check.ts index 71d8c9d..cf1a550 100644 --- a/packages/compiler/src/passes/intent-check.ts +++ b/packages/compiler/src/passes/intent-check.ts @@ -37,16 +37,7 @@ import { parseProgram } from "../parser/parse.js"; import type { FnDecl } from "../parser/parse-fn.js"; import { locationOf } from "./_location.js"; import { atLeast, type VersionInfo } from "./version.js"; - -/** stdlib namespace -> capability it consumes (subset mirrored from cap-check). */ -const STDLIB_TO_CAP: Readonly> = { - http: "net", - time: "time", - random: "random", - fs: "fs", - stdout: "stdout", - stderr: "stderr", -}; +import { STDLIB_TO_CAP } from "./cap-check.js"; export function passIntentCheck(src: string, version: VersionInfo): string { if (!atLeast(version.resolved, "0.7")) return src; diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts new file mode 100644 index 0000000..c695c40 --- /dev/null +++ b/packages/compiler/src/passes/uns-check.ts @@ -0,0 +1,329 @@ +/** + * External call without declared result contract (?bs 0.9+). + * + * UNS005 A stdlib capability call (http.x, fs.x, time.x, random.x, + * stdout.x, stderr.x) appears in a function body with no + * declared output contract the compiler can verify. + * + * "Declared output contract" at the call site means one of: + * - The call is the direct subject of a `match` expression + * (`match http.get(url) { ... }`). + * - The call is inside an `unsafe "" { ... }` block. + * - The call is inside an `unsafe "" fn` body. + * + * This is compiler-inferred (not programmer-applied) — unlike + * UNS001-UNS004 which fire on malformed `unsafe` blocks. + * A reviewer can tell at a glance whether the author made a + * deliberate choice (unsafe block / suppression) or the compiler + * is flagging an omission. + * + * Suppression mechanisms: + * 1. Wrap in `match` to handle both ok and err arms. + * 2. Use `unsafe "" { ... }` to accept the uncertainty + * with a written explanation. + * 3. Declare the containing fn as `unsafe "" fn` when + * the entire fn body is the intended escape hatch. + * 4. (Future) Declare `ensures: "..."` on the callee — allows + * the compiler to verify the output contract structurally. + * + * pre-0.9 This pass is not run. + */ + +import { BotscriptError, type Diagnostic } from "../diagnostics.js"; +import { getErrorCode } from "../error-codes.js"; +import type { Token } from "../parser/lex.js"; +import { parseProgram } from "../parser/parse.js"; +import type { FnDecl } from "../parser/parse-fn.js"; +import { locationOf } from "./_location.js"; +import { atLeast, type VersionInfo } from "./version.js"; +import { STDLIB_TO_CAP } from "./cap-check.js"; +import { computeNesting, nextSignificant } from "./_callgraph.js"; + +const STDLIB_CAPS = new Set(Object.keys(STDLIB_TO_CAP)); + +interface CharRange { + start: number; + end: number; +} + +export function passUnsCheck(src: string, version: VersionInfo): string { + if (!atLeast(version.resolved, "0.9")) return src; + + const allowGenerics = atLeast(version.resolved, "0.4"); + const program = parseProgram(src, { allowGenerics, includeNestedFns: true }); + const tokens = program.tokens; + const decls = program.fns.map((s) => s.decl); + + if (decls.length === 0) return src; + + // Collect char-offset ranges for unsafe blocks and unsafe fn bodies. + // Any stdlib call inside these ranges is suppressed. + const unsafeRanges: CharRange[] = []; + collectUnsafeBlockRanges(tokens, unsafeRanges); + // Unsafe fn bodies come from the pre-parsed decls — no re-parsing needed. + for (const decl of decls) { + if (decl.unsafeReason !== undefined) { + unsafeRanges.push({ start: decl.body.start, end: decl.body.end }); + } + } + + const innerByDecl = computeNesting(decls); + const diagnostics: Diagnostic[] = []; + + for (const decl of decls) { + const inner = innerByDecl.get(decl) ?? []; + + // Cursor-based inner-fn exclusion (same pattern as dep-check). + const open: FnDecl[] = []; + let nextInner = 0; + + for (let i = decl.bodyTokenStart ?? decl.tokenStart; i < decl.tokenEnd; i++) { + // Maintain the open-inner-fn stack. + 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 (!STDLIB_CAPS.has(tok.text)) continue; + + // Must be `stdlib.method(` or `stdlib?.method(` — confirm the shape before acting. + const dotIdx = nextSignificant(tokens, i + 1); + const dotTok = tokens[dotIdx]; + if ( + !dotTok || + !((dotTok.kind === "punct" && dotTok.text === ".") || dotTok.kind === "questionDot") + ) continue; + + const memberIdx = nextSignificant(tokens, dotIdx + 1); + const memberTok = tokens[memberIdx]; + if (!memberTok || memberTok.kind !== "ident") continue; + + const parenIdx = nextSignificant(tokens, memberIdx + 1); + const parenTok = tokens[parenIdx]; + if (!parenTok || parenTok.kind !== "open" || parenTok.text !== "(") continue; + + // Suppression 1: inside an unsafe block or unsafe fn body. + if (insideAnyChar(tok.start, unsafeRanges)) continue; + + // Suppression 2: direct subject of a `match` expression. + // Skips trivia and `await` — `match await http.get(url) { }` is fine. + // Pass the closing-paren index so the forward check can verify the call + // is the full scrutinee (not part of a larger expression like `http.get(url) + "x"`). + if (isDirectMatchSubject(tokens, i, parenTok.matchedAt)) continue; + + // Suppression 3: malformed `unsafe "reason" ns.method(...)` (missing `{}`). + // passUnsafe will emit UNS003 for the missing block body; suppress UNS005 + // here so the more specific diagnostic wins. + if (isMalformedUnsafeExpr(tokens, i)) continue; + + const entry = getErrorCode("UNS005")!; + const loc = locationOf(src, tok.start); + const ns = tok.text; + const member = memberTok.text; + + const isOptChain = dotTok.kind === "questionDot"; + const accessOp = isOptChain ? "?." : "."; + const callExpr = `${ns}${accessOp}${member}`; + const closingParen = parenTok.matchedAt !== undefined ? tokens[parenTok.matchedAt] : undefined; + diagnostics.push({ + code: "UNS005", + severity: "error", + file: null, + line: loc.line, + column: loc.column, + start: tok.start, + end: closingParen?.end ?? memberTok.end, + message: + `'${callExpr}(...)' is an external call with no declared result contract — ` + + `the return value may be structurally typed but semantically incorrect`, + rule: entry.rule, + idiom: entry.idiom, + rewrite: + `// option A — match on the result (handles both ok and err):\n` + + `match ${callExpr}(...) {\n` + + ` ok { value } -> { /* use value */ }\n` + + ` err { error } -> { /* handle error */ }\n` + + `}\n\n` + + `// option B — accept the uncertainty with a written reason:\n` + + `unsafe "I know what ${callExpr} returns here" { ${callExpr}(...) }`, + }); + + // Do not advance past the closing paren — inner stdlib calls in the + // argument list (e.g. http.get(fs.readText(path))) must each be flagged. + } + } + + // UNS005 intentionally accumulates all violations before throwing so the + // caller sees every missing result contract in one pass — violations are + // independent (each has a mechanical fix) and reporting them all at once is + // more useful than bailing on the first. This differs from passes like + // bareAs/depCheck that bail on first error because a single violation there + // already indicates a structural problem that invalidates further analysis. + if (diagnostics.length > 0) { + throw new BotscriptError(diagnostics); + } + + return src; +} + +// --------------------------------------------------------------------------- +// Unsafe range collection +// --------------------------------------------------------------------------- + +/** Collects char-offset ranges for `unsafe "reason" { body }` block bodies. */ +function collectUnsafeBlockRanges(tokens: Token[], out: CharRange[]): void { + for (let i = 0; i < tokens.length; i++) { + const t = tokens[i]; + if (!t || t.kind !== "keyword" || t.keyword !== "unsafe") continue; + + const j = nextSignificant(tokens, i + 1); + const head = tokens[j]; + if (!head) continue; + + let braceIdx = -1; + if (head.kind === "open" && head.text === "{") { + braceIdx = j; + } else if (head.kind === "string") { + const k = nextSignificant(tokens, j + 1); + const open = tokens[k]; + if (open && open.kind === "open" && open.text === "{") { + braceIdx = k; + } + } + if (braceIdx === -1) continue; + + const open = tokens[braceIdx]!; + if (open.matchedAt === undefined) continue; + const close = tokens[open.matchedAt]; + if (!close) continue; + + out.push({ start: open.start, end: close.end }); + i = open.matchedAt; + } +} + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** + * Returns true when the stdlib call at `callIdx` is immediately preceded + * (ignoring whitespace) by `unsafe "reason"` without a following `{`. + * + * This is the "malformed unsafe expression" pattern: the author wrote + * `unsafe "reason" ns.method(...)` instead of `unsafe "reason" { ns.method(...) }`. + * passUnsafe will fire UNS003 (missing body) for this form — suppress UNS005 + * so the more specific diagnostic wins. + */ +function isTrivia(k: string): boolean { + return k === "whitespace" || k === "newline" || k === "lineComment" || k === "blockComment"; +} + +function isMalformedUnsafeExpr(tokens: Token[], callIdx: number): boolean { + let i = callIdx - 1; + // Scan backwards past trivia, await, idents, dots, and parens to find + // `unsafe "reason"` anywhere wrapping this call — handles both direct + // (`unsafe "r" ns.call()`) and wrapped (`unsafe "r" foo(ns.call())`). + while (i >= 0) { + const t = tokens[i]!; + if (isTrivia(t.kind)) { i--; continue; } + if (t.kind === "ident") { i--; continue; } + if (t.kind === "punct" && t.text === ".") { i--; continue; } + if (t.kind === "questionDot") { i--; continue; } + if (t.kind === "open" && t.text === "(") { i--; continue; } + if (t.kind === "close" && t.text === ")") { i--; continue; } + break; + } + if (i < 0 || tokens[i]?.kind !== "string") return false; + i--; + while (i >= 0 && isTrivia(tokens[i]!.kind)) i--; + const t = tokens[i]; + return !!(t && t.kind === "keyword" && t.keyword === "unsafe"); +} + +/** + * Returns true when the stdlib call at `callIdx` is the direct subject of a + * `match` expression. Skips trivia, `await`, and grouping parens looking + * backward. This means all of these are recognized: + * match http.get(url) { ... } + * match await http.get(url) { ... } + * match (http.get(url)) { ... } + * match (await http.get(url)) { ... } + * + * When `closingParenIdx` is provided, also verifies (forward) that the token + * immediately after the call's closing paren (skipping any grouping `)` from + * `match (...)`) is the `{` opening match arms. This prevents false suppression + * for `match (http.get(url) + "x") { }` where the call is part of a larger + * scrutinee expression. + */ +function isDirectMatchSubject(tokens: Token[], callIdx: number, closingParenIdx?: number): boolean { + let i = callIdx - 1; + while (i >= 0) { + const t = tokens[i]; + if (!t) { i--; continue; } + if ( + t.kind === "whitespace" || + t.kind === "newline" || + t.kind === "lineComment" || + t.kind === "blockComment" + ) { + i--; + continue; + } + // await is transparent — the match still covers the result. + // Note: await is not in the KEYWORDS set; it lexes as an ident. + if (t.kind === "ident" && t.text === "await") { + i--; + continue; + } + // Opening paren is transparent — match (http.get(url)) is equivalent to + // match http.get(url). Walk backward through as many grouping parens as + // needed so match (await (http.get(url))) is also recognized. + if (t.kind === "open" && t.text === "(") { + i--; + continue; + } + if (!(t.kind === "keyword" && t.keyword === "match")) return false; + break; + } + if (i < 0) return false; + + // Forward check: the token(s) after the call's closing paren must be `{`. + // Skip closing grouping parens (from `match (http.get(url)) { }` forms). + // If the next sig token is anything else (e.g. `+`), the call is part of a + // larger expression and is not the direct match subject. + if (closingParenIdx === undefined) return true; // can't verify forward; trust backward + let j = closingParenIdx + 1; + while (j < tokens.length) { + const t = tokens[j]; + if (!t) { j++; continue; } + if ( + t.kind === "whitespace" || + t.kind === "newline" || + t.kind === "lineComment" || + t.kind === "blockComment" + ) { + j++; + continue; + } + // A closing grouping paren is transparent (from `match (http.get(url)) { }`). + if (t.kind === "close" && t.text === ")") { + j++; + continue; + } + return t.kind === "open" && t.text === "{"; + } + return false; +} + +function insideAnyChar(offset: number, ranges: CharRange[]): boolean { + for (const r of ranges) { + if (offset >= r.start && offset < r.end) return true; + } + return false; +} + diff --git a/packages/compiler/src/transform.ts b/packages/compiler/src/transform.ts index be3788a..8ed1412 100644 --- a/packages/compiler/src/transform.ts +++ b/packages/compiler/src/transform.ts @@ -12,6 +12,7 @@ import { passDepCheck } from "./passes/dep-check.js"; import { passThrCheck } from "./passes/thr-check.js"; import { passEffCheck } from "./passes/eff-check.js"; import { passIntentCheck } from "./passes/intent-check.js"; +import { passUnsCheck } from "./passes/uns-check.js"; import { passMatch } from "./passes/match.js"; import { passMatCheck } from "./passes/mat-check.js"; import { passPrimer } from "./passes/primer.js"; @@ -85,6 +86,10 @@ const PASS_PIPELINE: ReadonlyArray = [ // an `unsafe fn` — the claim is programmer-asserted, not compiler-proven. // Runs before capCheck so capCheck still validates the claim's content. { name: "capAssert", fn: passCapAssert, minVersion: "0.9" }, + // unsCheck: fires UNS005 on stdlib capability calls with no declared result + // contract (no match, no unsafe block). Must run before passUnsafe, which + // rewrites the source and erases unsafe keywords used as suppression markers. + { name: "unsCheck", fn: passUnsCheck, minVersion: "0.9" }, { name: "capCheck", fn: passCapCheck, minVersion: "0.2" }, { name: "testMocks", fn: passTestMocks, minVersion: "0.2" }, { name: "test", fn: passTest }, diff --git a/packages/compiler/tests/cap-assert.test.ts b/packages/compiler/tests/cap-assert.test.ts index 2c9af3a..5495319 100644 --- a/packages/compiler/tests/cap-assert.test.ts +++ b/packages/compiler/tests/cap-assert.test.ts @@ -59,8 +59,11 @@ describe("CAP003: does not fire for regular fns", () => { it("does not warn for a regular fn with uses clause", () => { const src = "?bs 0.9\n" + - "fn callApi(url: string) uses { net } -> string {\n" + - " http.get(url)\n" + + "fn callApi(url: string) uses { net } -> Result {\n" + + " match http.get(url) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + "}\n"; const result = transform(src); const cap3 = result.warnings.filter((w) => w.code === "CAP003"); @@ -81,8 +84,8 @@ describe("CAP003: does not fire for regular fns", () => { it("does not warn for regular fn at ?bs 0.9", () => { const src = "?bs 0.9\n" + - "fn now() uses { time } -> number {\n" + - " time.now()\n" + + 'fn now() uses { time } -> number {\n' + + ' unsafe "time.now returns a plain number" { time.now() }\n' + "}\n"; const result = transform(src); const cap3 = result.warnings.filter((w) => w.code === "CAP003"); @@ -126,8 +129,11 @@ describe("CAP003: multiple fns", () => { it("fires for each unsafe fn with uses, not for safe fns", () => { const src = "?bs 0.9\n" + - "fn safeOne(url: string) uses { net } -> string {\n" + - " http.get(url)\n" + + "fn safeOne(url: string) uses { net } -> Result {\n" + + " match http.get(url) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + "}\n" + "unsafe \"adapter one\" fn unsafeOne(url: string) uses { net } -> string {\n" + " http.get(url)\n" + diff --git a/packages/compiler/tests/cap-check.test.ts b/packages/compiler/tests/cap-check.test.ts index d246a2e..01313b0 100644 --- a/packages/compiler/tests/cap-check.test.ts +++ b/packages/compiler/tests/cap-check.test.ts @@ -328,7 +328,11 @@ describe("cap-check: no false positive for stdlib namespace in parameter type", }); it("still fires CAP001 when stdlib call is in the body (not the header)", () => { - const src = "?bs 0.9\nfn fetchData(url: string) -> string {\n http.get(url)\n}\n"; + // Wrap in match to suppress UNS005 (which runs before cap-check); CAP001 still + // fires because uses { net } is absent. + const src = + "?bs 0.9\nfn fetchData(url: string) -> Result {\n" + + " match http.get(url) {\n ok { value } -> ok(value)\n err { error } -> err(error)\n }\n}\n"; expect(() => t(src)).toThrow(/CAP001/); }); }); diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts new file mode 100644 index 0000000..b51c036 --- /dev/null +++ b/packages/compiler/tests/uns-check.test.ts @@ -0,0 +1,438 @@ +/** + * Tests for UNS005: external call without declared result contract (?bs 0.9+). + * + * Fires on any stdlib capability call (http.x, fs.x, time.x, etc.) that has + * no declared result contract at the call site. + * + * Suppressed by: + * - `match ns.method(...) { ... }` — direct match at the call site + * - `match await ns.method(...) { ... }` — await is transparent + * - `unsafe "reason" { ns.method(...) }` — explicit escape hatch + * - `unsafe "reason" fn` — declaration-level escape hatch + */ + +import { describe, expect, it } from "vitest"; +import { transform } from "../src/transform.js"; + +function compile(src: string): string { + return transform(src).code; +} + +// --------------------------------------------------------------------------- +// Basic firing cases +// --------------------------------------------------------------------------- + +describe("UNS005: basic firing", () => { + it("fires on a bare http call assigned to a variable", () => { + const src = + "?bs 0.9\n" + + "fn fetchData(url: string) uses { net } -> string {\n" + + " const data = http.get(url);\n" + + " data\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + expect(() => compile(src)).toThrow(/http\.get/); + }); + + it("fires on a bare fs call", () => { + const src = + "?bs 0.9\n" + + "fn readConfig(path: string) uses { fs } -> string {\n" + + " fs.readText(path)\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + expect(() => compile(src)).toThrow(/fs\.read/); + }); + + it("fires on a bare time call", () => { + const src = + "?bs 0.9\n" + + "fn getTs() uses { time } -> number {\n" + + " time.now()\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + }); + + it("fires on a bare random call", () => { + const src = + "?bs 0.9\n" + + "fn roll() uses { random } -> number {\n" + + " random.next()\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + }); + + it("fires on a bare http.post call", () => { + const src = + "?bs 0.9\n" + + "fn send(url: string, body: string) uses { net } -> string {\n" + + " http.post(url, body)\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + }); +}); + +// --------------------------------------------------------------------------- +// Suppression: match +// --------------------------------------------------------------------------- + +describe("UNS005: suppressed by match", () => { + it("does not fire when the call is the direct subject of match", () => { + const src = + "?bs 0.9\n" + + "fn fetchData(url: string) uses { net } -> Result {\n" + + " match http.get(url) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire when await-wrapped call is the match subject", () => { + const src = + "?bs 0.9\n" + + "fn fetchData(url: string) uses { net } -> Result {\n" + + " match await http.get(url) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire for fs.readText in a match", () => { + const src = + "?bs 0.9\n" + + "fn readFile(path: string) uses { fs } -> Result {\n" + + " match fs.readText(path) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire when the call is parenthesized inside match", () => { + const src = + "?bs 0.9\n" + + "fn fetchData(url: string) uses { net } -> Result {\n" + + " match (http.get(url)) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("fires when the call follows a parenthesized match but is not its subject", () => { + // A match (x) { ... } block appears earlier in the fn, so there's a + // `match (` in the token stream before the bare http.get call. The backward + // scan from http must NOT cross statement boundaries and incorrectly conclude + // this call is suppressed. + const src = + "?bs 0.9\n" + + "fn fetchData(url: string, x: number) uses { net } -> string {\n" + + " match (x) {\n" + + " ok { v } -> v\n" + + " _ -> http.get(url)\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + }); + + it("fires when the call is part of a larger expression in the match scrutinee", () => { + // `http.get(url) + "x"` — the stdlib call is inside a binary expression, + // not the direct match subject. The forward scan sees `+` after the closing + // paren and correctly rejects suppression. + const src = + "?bs 0.9\n" + + "fn fetchData(url: string) uses { net } -> string {\n" + + " match (http.get(url) + \"x\") {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + }); +}); + +// --------------------------------------------------------------------------- +// Suppression: unsafe block +// --------------------------------------------------------------------------- + +describe("UNS005: suppressed by unsafe block", () => { + it("does not fire when the call is inside an unsafe block", () => { + const src = + "?bs 0.9\n" + + "fn fetchData(url: string) uses { net } -> string {\n" + + ' unsafe "I know http.get returns a plain string here" { http.get(url) }\n' + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire for fs.readText inside unsafe", () => { + const src = + "?bs 0.9\n" + + "fn readConfig(path: string) uses { fs } -> string {\n" + + ' unsafe "config file is always valid JSON" { fs.readText(path) }\n' + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); +}); + +// --------------------------------------------------------------------------- +// Suppression: unsafe fn declaration +// --------------------------------------------------------------------------- + +describe("UNS005: suppressed by unsafe fn declaration", () => { + it("does not fire inside an unsafe fn body", () => { + const src = + "?bs 0.9\n" + + 'unsafe "known adapter — callers trust the return type" fn fetchRaw(url: string) uses { net } -> string {\n' + + " http.get(url)\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); +}); + +// --------------------------------------------------------------------------- +// Version gate +// --------------------------------------------------------------------------- + +describe("UNS005: version gate", () => { + it("does not fire below ?bs 0.9", () => { + const src = + "?bs 0.8\n" + + "fn fetchData(url: string) uses { net } -> string {\n" + + " const data = http.get(url);\n" + + " data\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("does not fire at ?bs 0.7", () => { + const src = + "?bs 0.7\n" + + "fn fetchData(url: string) uses { net } -> string {\n" + + " http.get(url)\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); +}); + +// --------------------------------------------------------------------------- +// Inner fn exclusion +// --------------------------------------------------------------------------- + +describe("UNS005: inner fn exclusion", () => { + it("fires on the outer fn but not the inner if inner is unsafe-wrapped", () => { + // outer has a bare call → should fire UNS005 + // inner has a match → should not fire + const src = + "?bs 0.9\n" + + "fn outer(url: string) uses { net } -> string {\n" + + " const r = http.get(url);\n" + + " fn inner(u: string) uses { net } -> Result {\n" + + " match http.get(u) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + + " }\n" + + " r\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + }); + + it("does not fire on inner fn when inner fn uses match", () => { + const src = + "?bs 0.9\n" + + "fn outer(url: string) uses { net } -> Result {\n" + + " match http.get(url) {\n" + + " ok { value } -> {\n" + + " fn inner(u: string) uses { net } -> Result {\n" + + " match http.get(u) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + + " }\n" + + " ok(value)\n" + + " }\n" + + " err { error } -> err(error)\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); +}); + +// --------------------------------------------------------------------------- +// stdout / stderr namespaces +// --------------------------------------------------------------------------- + +describe("UNS005: stdout and stderr namespaces", () => { + it("fires on a bare stdout call", () => { + const src = + "?bs 0.9\n" + + "fn logMsg(msg: string) uses { stdout } -> string {\n" + + " const r = stdout.write(msg);\n" + + " msg\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + }); + + it("fires on a bare stderr call", () => { + const src = + "?bs 0.9\n" + + "fn logErr(msg: string) uses { stderr } -> string {\n" + + " const r = stderr.println(msg);\n" + + " msg\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + }); + + it("suppresses stdout call inside unsafe block", () => { + const src = + "?bs 0.9\n" + + "fn logMsg(msg: string) uses { stdout } -> string {\n" + + " unsafe \"fire and forget\" { stdout.write(msg) }\n" + + " msg\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); +}); + +// --------------------------------------------------------------------------- +// Multiple calls +// --------------------------------------------------------------------------- + +describe("UNS005: multiple calls in one fn", () => { + it("fires for a fn that has two bare stdlib calls", () => { + const src = + "?bs 0.9\n" + + "fn doWork(url: string, path: string) uses { net, fs } -> string {\n" + + " const a = http.get(url);\n" + + " const b = fs.readText(path);\n" + + " a\n" + + "}\n"; + // Should throw UNS005 (first violation, same as other passes) + expect(() => compile(src)).toThrow("UNS005"); + }); + + it("passes when all calls in a fn are match-wrapped", () => { + const src = + "?bs 0.9\n" + + "fn doWork(url: string, path: string) uses { net, fs } -> Result {\n" + + " match http.get(url) {\n" + + " ok { value } -> match fs.readText(path) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + + " err { error } -> err(error)\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); +}); + +// --------------------------------------------------------------------------- +// Diagnostic precedence: UNS003 over UNS005 for malformed unsafe blocks +// --------------------------------------------------------------------------- + +describe("UNS003 takes precedence over UNS005 for malformed unsafe blocks", () => { + it("fires UNS003 (not UNS005) for unsafe \"reason\" ns.method() (missing {})", () => { + // `unsafe "reason" http.get(url)` is a malformed unsafe block — the + // author forgot the `{ ... }` body. passUnsafe owns this and fires UNS003. + // passUnsCheck must not fire UNS005 for the call first. + const src = + "?bs 0.9\n" + + "fn fetch(url: string) uses { net } -> string {\n" + + " unsafe \"trust me\" http.get(url)\n" + + "}\n"; + try { + compile(src); + expect.fail("should have thrown"); + } catch (e) { + const err = e as { diagnostics?: Array<{ code: string }> }; + // Should be UNS003 (malformed block), not UNS005 (missing contract) + expect(err.diagnostics?.[0]?.code).toBe("UNS003"); + } + }); + + it("fires UNS003 (not UNS005) for unsafe \"reason\" await ns.method() (missing {})", () => { + const src = + "?bs 0.9\n" + + "fn fetch(url: string) uses { net } -> string {\n" + + " unsafe \"trust me\" await http.get(url)\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("UNS003"); + } + }); + + it("fires UNS003 (not UNS005) for unsafe \"reason\" (ns.method()) (missing {})", () => { + const src = + "?bs 0.9\n" + + "fn fetch(url: string) uses { net } -> string {\n" + + " unsafe \"trust me\" (http.get(url))\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("UNS003"); + } + }); + + it("fires UNS003 (not UNS005) for unsafe \"reason\" foo(ns.method()) — wrapped call", () => { + // `unsafe "reason" foo(http.get(url))` — the stdlib call is inside a + // wrapper function, but the unsafe block is still missing `{ ... }`. + // isMalformedUnsafeExpr must scan past the wrapper ident and parens to + // detect the enclosing unsafe expression and suppress UNS005. + const src = + "?bs 0.9\n" + + "fn fetch(url: string) uses { net } -> string {\n" + + " unsafe \"trust me\" log(http.get(url))\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("UNS003"); + } + }); +}); + +// --------------------------------------------------------------------------- +// Scan scope: body only, not fn header +// --------------------------------------------------------------------------- + +describe("UNS005: scan scope (body only)", () => { + it("does not fire when stdlib namespace appears as type annotation in header", () => { + // A user-defined type named 'http' in the return type should not trigger + // UNS005 — the scan must start from bodyTokenStart, not tokenStart. + // This tests that we skip the fn header (params + return type). + const src = + "?bs 0.9\n" + + "fn identity(x: string) -> string {\n" + + " x\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("fires on a stdlib call in the body but not on fn name or param types", () => { + // A fn whose name contains 'http' as a prefix must not confuse the scanner. + // Only the body call matters. + const src = + "?bs 0.9\n" + + "fn fetchData(url: string) uses { net } -> string {\n" + + " http.get(url)\n" + + "}\n"; + expect(() => compile(src)).toThrow("UNS005"); + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 699133c..afe74c4 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -104,8 +104,11 @@ export const EXPLANATIONS: Readonly> = { "}\n", passes: "?bs 0.9\n" + - "fn callApi(url: string) uses { net } -> string {\n" + - " http.get(url)\n" + + "fn callApi(url: string) uses { net } -> Result {\n" + + " match http.get(url) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + + " }\n" + "}\n", }, }, @@ -183,6 +186,47 @@ export const EXPLANATIONS: Readonly> = { "const u = unsafe \"third-party Response.json() returns any\" { data as User };\n", }, }, + UNS005: { + code: "UNS005", + title: "external call without declared result contract", + body: + "UNS005 fires when a stdlib capability call (`http.x`, `fs.x`, `time.x`, `random.x`, " + + "`stdout.x`, `stderr.x`) has no declared result contract at the call site. The return " + + "value may be structurally typed correctly — the compiler sees a `string` or `Result` " + + "— but be semantically incorrect in ways only the runtime context can detect.\n\n" + + "UNS005 is **compiler-inferred**, not programmer-applied. Unlike UNS001–UNS004 which fire " + + "on malformed `unsafe` blocks, UNS005 fires on ordinary-looking external calls. A reviewer " + + "can tell at a glance whether the author made a deliberate choice (unsafe block) or the " + + "compiler is flagging an omission.\n\n" + + "**Suppression mechanisms (in order of preference):**\n\n" + + "1. **match** — wrap the call as the direct match subject:\n" + + " ```\n match http.get(url) {\n ok { value } -> ...\n err { error } -> ...\n }\n ```\n" + + " Both success and failure paths are explicit. " + + "`match await http.get(url)` is also accepted (await is transparent).\n\n" + + "2. **unsafe block** — `unsafe \"I know what X returns\" { ns.method(...) }` accepts the " + + "uncertainty with a written explanation. The reason becomes the review record on the call.\n\n" + + "3. **unsafe fn** — `unsafe \"reason\" fn name(...) -> T { ns.method(...) }` suppresses UNS005 " + + "for the entire fn body. Use when the fn itself is the module's single safe adapter for the call.\n\n" + + "4. **(Future) ensures annotation** — when `ensures: \"...\"` lands in a future version, " + + "declaring it on the callee's header will suppress UNS005 for all call sites.\n\n" + + "UNS005 is gated on `?bs 0.9`. Files pinned to earlier versions are unaffected.", + example: { + fails: + "?bs 0.9\n" + + "fn fetchUser(id: string) uses { net } -> string {\n" + + " const data = http.get(`/users/${id}`);\n" + + " data\n" + + "}\n", + passes: + "?bs 0.9\n" + + "fn fetchUser(id: string) uses { net } -> Result {\n" + + " match http.get(`/users/${id}`) {\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(`fetch failed: ${error}`)\n" + + " }\n" + + "}\n", + }, + }, INT001: { code: "INT001", title: "intent declares 'pure' but function has capability or read/write declarations", diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index d5a371c..fab9504 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -53,7 +53,7 @@ export function createServer(): Server { { name: "transform", description: - "Compile a .bs source string to TypeScript. On success, returns { ok: true, code, forms, version }. On failure, returns { ok: false, diagnostics } where each diagnostic has { code, file?, line, column, message, rule?, idiom?, rewrite? }.", + "Compile a .bs source string to TypeScript. On success, returns { ok: true, code, forms, version, warnings } where warnings is an array of non-blocking diagnostics (e.g. CAP003). On failure, returns { ok: false, diagnostics } where each diagnostic has { code, file?, line, column, message, rule?, idiom?, rewrite? }.", inputSchema: { type: "object", properties: { @@ -100,8 +100,8 @@ server.setRequestHandler(CallToolRequestSchema, async (req) => { return errorText("transform: `source` must be a string"); } try { - const { code, forms, version } = transform(source, filename ? { filename } : {}); - return json({ ok: true, code, forms, version }); + const { code, forms, version, warnings } = transform(source, filename ? { filename } : {}); + return json({ ok: true, code, forms, version, warnings: [...warnings] }); } catch (e) { if (e instanceof BotscriptError) { return json({ ok: false, diagnostics: [...e.diagnostics] }); diff --git a/packages/mcp/tests/server.test.ts b/packages/mcp/tests/server.test.ts index 4e89963..d14d84a 100644 --- a/packages/mcp/tests/server.test.ts +++ b/packages/mcp/tests/server.test.ts @@ -68,6 +68,7 @@ describe("botscript-mcp explanations", () => { "UNS002", "UNS003", "UNS004", + "UNS005", "VER001", "VER002", ]);