From 9714cb6a56fe6f61000cf20b3c13514ff310412d Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Mon, 18 May 2026 13:10:56 -0300 Subject: [PATCH 01/19] =?UTF-8?q?feat:=20UNS005=20=E2=80=94=20external=20c?= =?UTF-8?q?all=20without=20declared=20result=20contract=20(=3Fbs=200.9)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Suppressed by wrapping in `match` (including `match await ...`) or by an `unsafe "reason" { }` block or `unsafe "reason" fn` declaration. UNS005 is compiler-inferred — unlike UNS001-UNS004 which fire on malformed unsafe blocks. This lets reviewers distinguish deliberate escapes (unsafe block) from omissions the compiler caught. Gated on ?bs 0.9. 27 new tests, 554/554 pass. Closes #50 Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/error-codes.ts | 29 +++ packages/compiler/src/passes/uns-check.ts | 284 ++++++++++++++++++++++ packages/compiler/src/transform.ts | 5 + packages/compiler/tests/uns-check.test.ts | 256 +++++++++++++++++++ packages/mcp/src/explanations.ts | 38 +++ packages/mcp/tests/server.test.ts | 1 + 6 files changed, 613 insertions(+) create mode 100644 packages/compiler/src/passes/uns-check.ts create mode 100644 packages/compiler/tests/uns-check.test.ts diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index 9bf81d2..70263fb 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -143,6 +143,35 @@ 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 — either wrap in `match` to handle all cases exhaustively, or use " + + "`unsafe \"\" { ... }` to accept the uncertainty with a written explanation", + idiom: + "prefer `match ns.method(...) { Ok(v) => ..., Err(e) => ... }` — it makes both success " + + "and failure paths explicit and compiler-checked; use `unsafe` only when you are certain " + + "about the shape and want to document why", + rewrite: + 'match ns.method(...) { Ok(value) => { /* use value */ }, Err(e) => { /* handle */ } }', + 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 exhaustive match\n" + + "?bs 0.9\n" + + "fn fetchUser(id: string) uses { net } -> Result {\n" + + " match http.get(`/users/${id}`) {\n" + + " Ok(data) => ok(data),\n" + + " Err(e) => err(`fetch failed: ${e}`),\n" + + " }\n" + + "}", + }, FMT001: { code: "FMT001", title: "source is not in canonical form", diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts new file mode 100644 index 0000000..f7d09a1 --- /dev/null +++ b/packages/compiler/src/passes/uns-check.ts @@ -0,0 +1,284 @@ +/** + * 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. + * + * 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. (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 { lex, type Token } from "../parser/lex.js"; +import { parseFn } from "../parser/parse-fn.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"; + +/** stdlib namespaces that consume external capabilities. */ +const STDLIB_CAPS = new Set(["http", "fs", "time", "random", "stdout", "stderr"]); + +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); + collectUnsafeFnBodyRanges(tokens, unsafeRanges); + + 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.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(` — confirm the shape before acting. + const dotIdx = nextSignificant(tokens, i + 1); + const dotTok = tokens[dotIdx]; + if (!dotTok || dotTok.kind !== "punct" || dotTok.text !== ".") 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. + if (isDirectMatchSubject(tokens, i)) continue; + + const entry = getErrorCode("UNS005")!; + const loc = locationOf(src, tok.start); + const ns = tok.text; + const member = memberTok.text; + + diagnostics.push({ + code: "UNS005", + severity: "error", + file: null, + line: loc.line, + column: loc.column, + start: tok.start, + end: memberTok.end, + message: + `'${ns}.${member}(...)' 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 ${ns}.${member}(...) {\n` + + ` Ok(value) => { /* use value */ },\n` + + ` Err(e) => { /* handle error */ },\n` + + `}\n\n` + + `// option B — accept the uncertainty with a written reason:\n` + + `unsafe "I know what ${ns}.${member} returns here" { ${ns}.${member}(...) }`, + }); + + // Advance past the call to avoid redundant hits (e.g. chained calls). + i = parenTok.matchedAt ?? parenIdx; + } + } + + 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; + } +} + +/** Collects char-offset ranges for `unsafe "reason" fn` declaration bodies. */ +function collectUnsafeFnBodyRanges(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 reasonTok = tokens[j]; + if (!reasonTok || reasonTok.kind !== "string") continue; + + const k = nextSignificant(tokens, j + 1); + const next = tokens[k]; + if (!next || next.kind !== "keyword") continue; + + let fnIdx: number; + if (next.keyword === "fn") { + fnIdx = k; + } else if (next.keyword === "async") { + const l = nextSignificant(tokens, k + 1); + const fnTok = tokens[l]; + if (!fnTok || fnTok.kind !== "keyword" || fnTok.keyword !== "fn") continue; + fnIdx = l; + } else { + continue; + } + + const decl = parseFn(tokens, fnIdx, { allowGenerics: true }); + if (!decl) continue; + + out.push({ start: decl.body.start, end: decl.body.end }); + i = decl.tokenEnd - 1; + } +} + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** + * Returns true when the stdlib call at `callIdx` is the direct subject of a + * `match` expression. Skips trivia and `await` tokens looking backward. + */ +function isDirectMatchSubject(tokens: Token[], callIdx: 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; + } + return t.kind === "keyword" && t.keyword === "match"; + } + return false; +} + +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 insideAnyChar(offset: number, ranges: CharRange[]): boolean { + for (const r of ranges) { + if (offset >= r.start && offset < r.end) return true; + } + return false; +} + +function computeNesting(decls: FnDecl[]): Map { + const result = new Map(); + for (const outer of decls) { + const inner = decls.filter( + (d) => d !== outer && d.tokenStart >= outer.tokenStart && d.tokenEnd <= outer.tokenEnd, + ); + inner.sort((a, b) => a.tokenStart - b.tokenStart); + result.set(outer, inner); + } + return result; +} 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/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts new file mode 100644 index 0000000..3ad171d --- /dev/null +++ b/packages/compiler/tests/uns-check.test.ts @@ -0,0 +1,256 @@ +/** + * 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.read(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.float()\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(data) => ok(data),\n" + + " Err(e) => err(e),\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow("UNS005"); + }); + + 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(data) => ok(data),\n" + + " Err(e) => err(e),\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow("UNS005"); + }); + + it("does not fire for fs.read in a match", () => { + const src = + "?bs 0.9\n" + + "fn readFile(path: string) uses { fs } -> Result {\n" + + " match fs.read(path) {\n" + + " Ok(contents) => ok(contents),\n" + + " Err(e) => err(e),\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.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("UNS005"); + }); + + it("does not fire for fs.read inside unsafe", () => { + const src = + "?bs 0.9\n" + + "fn readConfig(path: string) uses { fs } -> string {\n" + + ' unsafe "config file is always valid JSON" { fs.read(path) }\n' + + "}\n"; + expect(() => compile(src)).not.toThrow("UNS005"); + }); +}); + +// --------------------------------------------------------------------------- +// 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("UNS005"); + }); +}); + +// --------------------------------------------------------------------------- +// 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("UNS005"); + }); + + 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("UNS005"); + }); +}); + +// --------------------------------------------------------------------------- +// 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(d) => ok(d),\n" + + " Err(e) => err(e),\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(d) => {\n" + + " fn inner(u: string) uses { net } -> Result {\n" + + " match http.get(u) {\n" + + " Ok(data) => ok(data),\n" + + " Err(e) => err(e),\n" + + " }\n" + + " }\n" + + " ok(d)\n" + + " },\n" + + " Err(e) => err(e),\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow("UNS005"); + }); +}); + +// --------------------------------------------------------------------------- +// 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.read(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(a) => match fs.read(path) {\n" + + " Ok(b) => ok(a),\n" + + " Err(e) => err(e),\n" + + " },\n" + + " Err(e) => err(e),\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow("UNS005"); + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 699133c..6fa1a0d 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -183,6 +183,44 @@ 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. **Exhaustive match** — `match http.get(url) { Ok(v) => ..., Err(e) => ... }` wraps " + + "the call in a structural contract check. Both success and failure paths are explicit " + + "and compiler-enforced. `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. **(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(data) => ok(data),\n" + + " Err(e) => err(`fetch failed: ${e}`),\n" + + " }\n" + + "}\n", + }, + }, INT001: { code: "INT001", title: "intent declares 'pure' but function has capability or read/write declarations", 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", ]); From b0630184ff5ea4862de96326a29fe546bebcef59 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Mon, 18 May 2026 13:11:47 -0300 Subject: [PATCH 02/19] docs(changelog): document UNS005 in ?bs 0.9 section Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) 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 From 3a6dbd42ffc2516b10c6f5f4a291af5169d0fe7a Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Mon, 18 May 2026 17:03:10 -0300 Subject: [PATCH 03/19] =?UTF-8?q?fix(uns-check):=20address=20Copilot=20rev?= =?UTF-8?q?iew=20=E2=80=94=20drop=20unused=20lex=20import,=20align=20test?= =?UTF-8?q?=20API=20names,=20soften=20match-suppression=20wording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove unused `lex` import from uns-check.ts (only Token type is needed) - Replace `fs.read` with `fs.readText` and `random.float()` with `random.next()` in tests — aligns with actual runtime stdlib surface - Remove "exhaustively"/"compiler-checked"/"compiler-enforced" claims from error-codes.ts and explanations.ts — match suppression does not verify arm completeness; wording now says "makes both paths explicit" without overclaiming Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/error-codes.ts | 6 +++--- packages/compiler/src/passes/uns-check.ts | 2 +- packages/compiler/tests/uns-check.test.ts | 16 ++++++++-------- packages/mcp/src/explanations.ts | 6 +++--- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index 70263fb..7a63403 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -148,11 +148,11 @@ const E: Record = { 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 — either wrap in `match` to handle all cases exhaustively, or use " + + "at the call site — either wrap in `match` to make the success and failure paths explicit, or use " + "`unsafe \"\" { ... }` to accept the uncertainty with a written explanation", idiom: "prefer `match ns.method(...) { Ok(v) => ..., Err(e) => ... }` — it makes both success " + - "and failure paths explicit and compiler-checked; use `unsafe` only when you are certain " + + "and failure paths explicit; use `unsafe` only when you are certain " + "about the shape and want to document why", rewrite: 'match ns.method(...) { Ok(value) => { /* use value */ }, Err(e) => { /* handle */ } }', @@ -163,7 +163,7 @@ const E: Record = { " const data = http.get(`/users/${id}`);\n" + " data\n" + "}\n\n" + - "// after — result contract via exhaustive match\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" + diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index f7d09a1..f00ce83 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -28,7 +28,7 @@ import { BotscriptError, type Diagnostic } from "../diagnostics.js"; import { getErrorCode } from "../error-codes.js"; -import { lex, type Token } from "../parser/lex.js"; +import type { Token } from "../parser/lex.js"; import { parseFn } from "../parser/parse-fn.js"; import { parseProgram } from "../parser/parse.js"; import type { FnDecl } from "../parser/parse-fn.js"; diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts index 3ad171d..bd41cc4 100644 --- a/packages/compiler/tests/uns-check.test.ts +++ b/packages/compiler/tests/uns-check.test.ts @@ -38,7 +38,7 @@ describe("UNS005: basic firing", () => { const src = "?bs 0.9\n" + "fn readConfig(path: string) uses { fs } -> string {\n" + - " fs.read(path)\n" + + " fs.readText(path)\n" + "}\n"; expect(() => compile(src)).toThrow("UNS005"); expect(() => compile(src)).toThrow(/fs\.read/); @@ -57,7 +57,7 @@ describe("UNS005: basic firing", () => { const src = "?bs 0.9\n" + "fn roll() uses { random } -> number {\n" + - " random.float()\n" + + " random.next()\n" + "}\n"; expect(() => compile(src)).toThrow("UNS005"); }); @@ -101,11 +101,11 @@ describe("UNS005: suppressed by match", () => { expect(() => compile(src)).not.toThrow("UNS005"); }); - it("does not fire for fs.read in a match", () => { + 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.read(path) {\n" + + " match fs.readText(path) {\n" + " Ok(contents) => ok(contents),\n" + " Err(e) => err(e),\n" + " }\n" + @@ -128,11 +128,11 @@ describe("UNS005: suppressed by unsafe block", () => { expect(() => compile(src)).not.toThrow("UNS005"); }); - it("does not fire for fs.read inside unsafe", () => { + 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.read(path) }\n' + + ' unsafe "config file is always valid JSON" { fs.readText(path) }\n' + "}\n"; expect(() => compile(src)).not.toThrow("UNS005"); }); @@ -232,7 +232,7 @@ describe("UNS005: multiple calls in one fn", () => { "?bs 0.9\n" + "fn doWork(url: string, path: string) uses { net, fs } -> string {\n" + " const a = http.get(url);\n" + - " const b = fs.read(path);\n" + + " const b = fs.readText(path);\n" + " a\n" + "}\n"; // Should throw UNS005 (first violation, same as other passes) @@ -244,7 +244,7 @@ describe("UNS005: multiple calls in one fn", () => { "?bs 0.9\n" + "fn doWork(url: string, path: string) uses { net, fs } -> Result {\n" + " match http.get(url) {\n" + - " Ok(a) => match fs.read(path) {\n" + + " Ok(a) => match fs.readText(path) {\n" + " Ok(b) => ok(a),\n" + " Err(e) => err(e),\n" + " },\n" + diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 6fa1a0d..1277ca2 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -196,9 +196,9 @@ export const EXPLANATIONS: Readonly> = { "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. **Exhaustive match** — `match http.get(url) { Ok(v) => ..., Err(e) => ... }` wraps " + - "the call in a structural contract check. Both success and failure paths are explicit " + - "and compiler-enforced. `match await http.get(url)` is also accepted (await is transparent).\n\n" + + "1. **match** — `match http.get(url) { Ok(v) => ..., Err(e) => ... }` wraps " + + "the call in a structural contract check. 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. **(Future) ensures annotation** — when `ensures: \"...\"` lands in a future version, " + From 37e72783a40d2a4d3be1b0cc4c24c281662878f2 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Mon, 18 May 2026 17:23:14 -0300 Subject: [PATCH 04/19] fix(cap-assert,explanations): update test examples to be UNS005-compliant at ?bs 0.9 CAP003 tests and MCP examples used bare stdlib calls in regular fn bodies, which now trigger UNS005 at ?bs 0.9. Updated to use match-wrapped calls or unsafe blocks so the examples compile cleanly alongside the UNS005 pass. Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/error-codes.ts | 7 +++++-- packages/compiler/tests/cap-assert.test.ts | 18 ++++++++++++------ packages/mcp/src/explanations.ts | 7 +++++-- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index 7a63403..d1f2ece 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(data) => ok(data),\n" + + " Err(e) => err(`fetch failed: ${e}`),\n" + + " }\n" + "}", }, UNS001: { diff --git a/packages/compiler/tests/cap-assert.test.ts b/packages/compiler/tests/cap-assert.test.ts index 2c9af3a..64d9e5b 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(data) => ok(data),\n" + + " Err(e) => err(e),\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(d) => ok(d),\n" + + " Err(e) => err(e),\n" + + " }\n" + "}\n" + "unsafe \"adapter one\" fn unsafeOne(url: string) uses { net } -> string {\n" + " http.get(url)\n" + diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 1277ca2..25722e6 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(data) => ok(data),\n" + + " Err(e) => err(e),\n" + + " }\n" + "}\n", }, }, From 4b8b9c6f3b310de78d89abb88fcd60e421a0b18a Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Tue, 19 May 2026 01:08:14 -0300 Subject: [PATCH 05/19] fix: address Copilot review on UNS005 PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove advance-past-closing-paren so nested stdlib calls inside args are also flagged (e.g. http.get(fs.readText(path)) now fires twice) - Replace collectUnsafeFnBodyRanges token-scan with a simple filter over pre-parsed decls — removes re-parsing and drift risk - Handle parenthesized match subjects in isDirectMatchSubject (treat `(` as transparent like `await`); add test for match (http.get(url)) - Fix diagnostic `end` to span full call expression including closing paren - Replace O(N²) computeNesting with O(N log N) sort+stack (from dep-check.ts) - Strengthen negative-case test assertions: not.toThrow("UNS005") → not.toThrow() - Add comment pointing at cap-check.ts as canonical source for STDLIB_CAPS Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/uns-check.ts | 90 +++++++++++------------ packages/compiler/tests/uns-check.test.ts | 32 +++++--- 2 files changed, 63 insertions(+), 59 deletions(-) diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index f00ce83..f3bfaed 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -29,13 +29,15 @@ import { BotscriptError, type Diagnostic } from "../diagnostics.js"; import { getErrorCode } from "../error-codes.js"; import type { Token } from "../parser/lex.js"; -import { parseFn } from "../parser/parse-fn.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"; -/** stdlib namespaces that consume external capabilities. */ +/** + * stdlib namespaces that consume external capabilities. + * Must stay in sync with STDLIB_TO_CAP in cap-check.ts (the canonical source). + */ const STDLIB_CAPS = new Set(["http", "fs", "time", "random", "stdout", "stderr"]); interface CharRange { @@ -57,7 +59,12 @@ export function passUnsCheck(src: string, version: VersionInfo): string { // Any stdlib call inside these ranges is suppressed. const unsafeRanges: CharRange[] = []; collectUnsafeBlockRanges(tokens, unsafeRanges); - collectUnsafeFnBodyRanges(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[] = []; @@ -107,6 +114,7 @@ export function passUnsCheck(src: string, version: VersionInfo): string { const ns = tok.text; const member = memberTok.text; + const closingParen = parenTok.matchedAt !== undefined ? tokens[parenTok.matchedAt] : undefined; diagnostics.push({ code: "UNS005", severity: "error", @@ -114,7 +122,7 @@ export function passUnsCheck(src: string, version: VersionInfo): string { line: loc.line, column: loc.column, start: tok.start, - end: memberTok.end, + end: closingParen?.end ?? memberTok.end, message: `'${ns}.${member}(...)' is an external call with no declared result contract — ` + `the return value may be structurally typed but semantically incorrect`, @@ -130,8 +138,8 @@ export function passUnsCheck(src: string, version: VersionInfo): string { `unsafe "I know what ${ns}.${member} returns here" { ${ns}.${member}(...) }`, }); - // Advance past the call to avoid redundant hits (e.g. chained calls). - i = parenTok.matchedAt ?? parenIdx; + // 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. } } @@ -178,47 +186,18 @@ function collectUnsafeBlockRanges(tokens: Token[], out: CharRange[]): void { } } -/** Collects char-offset ranges for `unsafe "reason" fn` declaration bodies. */ -function collectUnsafeFnBodyRanges(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 reasonTok = tokens[j]; - if (!reasonTok || reasonTok.kind !== "string") continue; - - const k = nextSignificant(tokens, j + 1); - const next = tokens[k]; - if (!next || next.kind !== "keyword") continue; - - let fnIdx: number; - if (next.keyword === "fn") { - fnIdx = k; - } else if (next.keyword === "async") { - const l = nextSignificant(tokens, k + 1); - const fnTok = tokens[l]; - if (!fnTok || fnTok.kind !== "keyword" || fnTok.keyword !== "fn") continue; - fnIdx = l; - } else { - continue; - } - - const decl = parseFn(tokens, fnIdx, { allowGenerics: true }); - if (!decl) continue; - - out.push({ start: decl.body.start, end: decl.body.end }); - i = decl.tokenEnd - 1; - } -} - // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- /** * Returns true when the stdlib call at `callIdx` is the direct subject of a - * `match` expression. Skips trivia and `await` tokens looking backward. + * `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)) { ... } */ function isDirectMatchSubject(tokens: Token[], callIdx: number): boolean { let i = callIdx - 1; @@ -240,6 +219,13 @@ function isDirectMatchSubject(tokens: Token[], callIdx: number): boolean { 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; + } return t.kind === "keyword" && t.keyword === "match"; } return false; @@ -272,13 +258,19 @@ function insideAnyChar(offset: number, ranges: CharRange[]): boolean { } function computeNesting(decls: FnDecl[]): Map { - const result = new Map(); - for (const outer of decls) { - const inner = decls.filter( - (d) => d !== outer && d.tokenStart >= outer.tokenStart && d.tokenEnd <= outer.tokenEnd, - ); - inner.sort((a, b) => a.tokenStart - b.tokenStart); - result.set(outer, inner); + const inner = new Map(); + 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 result; + + return inner; } diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts index bd41cc4..c9e7238 100644 --- a/packages/compiler/tests/uns-check.test.ts +++ b/packages/compiler/tests/uns-check.test.ts @@ -86,7 +86,7 @@ describe("UNS005: suppressed by match", () => { " Err(e) => err(e),\n" + " }\n" + "}\n"; - expect(() => compile(src)).not.toThrow("UNS005"); + expect(() => compile(src)).not.toThrow(); }); it("does not fire when await-wrapped call is the match subject", () => { @@ -98,7 +98,7 @@ describe("UNS005: suppressed by match", () => { " Err(e) => err(e),\n" + " }\n" + "}\n"; - expect(() => compile(src)).not.toThrow("UNS005"); + expect(() => compile(src)).not.toThrow(); }); it("does not fire for fs.readText in a match", () => { @@ -110,7 +110,19 @@ describe("UNS005: suppressed by match", () => { " Err(e) => err(e),\n" + " }\n" + "}\n"; - expect(() => compile(src)).not.toThrow("UNS005"); + 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(data) => ok(data),\n" + + " Err(e) => err(e),\n" + + " }\n" + + "}\n"; + expect(() => compile(src)).not.toThrow(); }); }); @@ -125,7 +137,7 @@ describe("UNS005: suppressed by unsafe block", () => { "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("UNS005"); + expect(() => compile(src)).not.toThrow(); }); it("does not fire for fs.readText inside unsafe", () => { @@ -134,7 +146,7 @@ describe("UNS005: suppressed by unsafe block", () => { "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("UNS005"); + expect(() => compile(src)).not.toThrow(); }); }); @@ -149,7 +161,7 @@ describe("UNS005: suppressed by unsafe fn declaration", () => { '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("UNS005"); + expect(() => compile(src)).not.toThrow(); }); }); @@ -165,7 +177,7 @@ describe("UNS005: version gate", () => { " const data = http.get(url);\n" + " data\n" + "}\n"; - expect(() => compile(src)).not.toThrow("UNS005"); + expect(() => compile(src)).not.toThrow(); }); it("does not fire at ?bs 0.7", () => { @@ -174,7 +186,7 @@ describe("UNS005: version gate", () => { "fn fetchData(url: string) uses { net } -> string {\n" + " http.get(url)\n" + "}\n"; - expect(() => compile(src)).not.toThrow("UNS005"); + expect(() => compile(src)).not.toThrow(); }); }); @@ -218,7 +230,7 @@ describe("UNS005: inner fn exclusion", () => { " Err(e) => err(e),\n" + " }\n" + "}\n"; - expect(() => compile(src)).not.toThrow("UNS005"); + expect(() => compile(src)).not.toThrow(); }); }); @@ -251,6 +263,6 @@ describe("UNS005: multiple calls in one fn", () => { " Err(e) => err(e),\n" + " }\n" + "}\n"; - expect(() => compile(src)).not.toThrow("UNS005"); + expect(() => compile(src)).not.toThrow(); }); }); From 1b0e626552f4c23025fb1f2ff7d57b78037be46a Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Tue, 19 May 2026 05:08:03 -0300 Subject: [PATCH 06/19] fix(uns005): correct botscript match syntax in docs, diagnostics, and tests Replace Ok(v) => ... / Err(e) => ... with ok { value } -> ... / err { error } -> ... throughout error-codes.ts, explanations.ts, uns-check.ts rewrite strings, uns-check.test.ts, cap-assert.test.ts, and CHANGELOG.md. The runtime Result has kind:"ok"/"err" fields, so the correct tag patterns are lowercase with brace binds and -> arms. Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/error-codes.ts | 12 +++---- packages/compiler/src/passes/uns-check.ts | 6 ++-- packages/compiler/tests/cap-assert.test.ts | 8 ++--- packages/compiler/tests/uns-check.test.ts | 42 +++++++++++----------- packages/mcp/src/explanations.ts | 10 +++--- 5 files changed, 39 insertions(+), 39 deletions(-) diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index d1f2ece..f9bb791 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -79,8 +79,8 @@ const E: Record = { "?bs 0.9\n" + "fn callApi(url: string) uses { net } -> Result {\n" + " match http.get(url) {\n" + - " Ok(data) => ok(data),\n" + - " Err(e) => err(`fetch failed: ${e}`),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(`fetch failed: ${error}`)\n" + " }\n" + "}", }, @@ -154,11 +154,11 @@ const E: Record = { "at the call site — either wrap in `match` to make the success and failure paths explicit, or use " + "`unsafe \"\" { ... }` to accept the uncertainty with a written explanation", idiom: - "prefer `match ns.method(...) { Ok(v) => ..., Err(e) => ... }` — it makes both success " + + "prefer `match ns.method(...) { ok { value } -> ...\n err { error } -> ... }` — it 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(...) { Ok(value) => { /* use value */ }, Err(e) => { /* handle */ } }', + "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" + @@ -170,8 +170,8 @@ const E: Record = { "?bs 0.9\n" + "fn fetchUser(id: string) uses { net } -> Result {\n" + " match http.get(`/users/${id}`) {\n" + - " Ok(data) => ok(data),\n" + - " Err(e) => err(`fetch failed: ${e}`),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(`fetch failed: ${error}`)\n" + " }\n" + "}", }, diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index f3bfaed..a72fd22 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -129,10 +129,10 @@ export function passUnsCheck(src: string, version: VersionInfo): string { rule: entry.rule, idiom: entry.idiom, rewrite: - `// option A — match on the result (handles both Ok and Err):\n` + + `// option A — match on the result (handles both ok and err):\n` + `match ${ns}.${member}(...) {\n` + - ` Ok(value) => { /* use value */ },\n` + - ` Err(e) => { /* handle error */ },\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 ${ns}.${member} returns here" { ${ns}.${member}(...) }`, diff --git a/packages/compiler/tests/cap-assert.test.ts b/packages/compiler/tests/cap-assert.test.ts index 64d9e5b..5495319 100644 --- a/packages/compiler/tests/cap-assert.test.ts +++ b/packages/compiler/tests/cap-assert.test.ts @@ -61,8 +61,8 @@ describe("CAP003: does not fire for regular fns", () => { "?bs 0.9\n" + "fn callApi(url: string) uses { net } -> Result {\n" + " match http.get(url) {\n" + - " Ok(data) => ok(data),\n" + - " Err(e) => err(e),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + " }\n" + "}\n"; const result = transform(src); @@ -131,8 +131,8 @@ describe("CAP003: multiple fns", () => { "?bs 0.9\n" + "fn safeOne(url: string) uses { net } -> Result {\n" + " match http.get(url) {\n" + - " Ok(d) => ok(d),\n" + - " Err(e) => err(e),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + " }\n" + "}\n" + "unsafe \"adapter one\" fn unsafeOne(url: string) uses { net } -> string {\n" + diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts index c9e7238..c4f5882 100644 --- a/packages/compiler/tests/uns-check.test.ts +++ b/packages/compiler/tests/uns-check.test.ts @@ -82,8 +82,8 @@ describe("UNS005: suppressed by match", () => { "?bs 0.9\n" + "fn fetchData(url: string) uses { net } -> Result {\n" + " match http.get(url) {\n" + - " Ok(data) => ok(data),\n" + - " Err(e) => err(e),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + " }\n" + "}\n"; expect(() => compile(src)).not.toThrow(); @@ -94,8 +94,8 @@ describe("UNS005: suppressed by match", () => { "?bs 0.9\n" + "fn fetchData(url: string) uses { net } -> Result {\n" + " match await http.get(url) {\n" + - " Ok(data) => ok(data),\n" + - " Err(e) => err(e),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + " }\n" + "}\n"; expect(() => compile(src)).not.toThrow(); @@ -106,8 +106,8 @@ describe("UNS005: suppressed by match", () => { "?bs 0.9\n" + "fn readFile(path: string) uses { fs } -> Result {\n" + " match fs.readText(path) {\n" + - " Ok(contents) => ok(contents),\n" + - " Err(e) => err(e),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + " }\n" + "}\n"; expect(() => compile(src)).not.toThrow(); @@ -118,8 +118,8 @@ describe("UNS005: suppressed by match", () => { "?bs 0.9\n" + "fn fetchData(url: string) uses { net } -> Result {\n" + " match (http.get(url)) {\n" + - " Ok(data) => ok(data),\n" + - " Err(e) => err(e),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + " }\n" + "}\n"; expect(() => compile(src)).not.toThrow(); @@ -204,8 +204,8 @@ describe("UNS005: inner fn exclusion", () => { " const r = http.get(url);\n" + " fn inner(u: string) uses { net } -> Result {\n" + " match http.get(u) {\n" + - " Ok(d) => ok(d),\n" + - " Err(e) => err(e),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + " }\n" + " }\n" + " r\n" + @@ -218,16 +218,16 @@ describe("UNS005: inner fn exclusion", () => { "?bs 0.9\n" + "fn outer(url: string) uses { net } -> Result {\n" + " match http.get(url) {\n" + - " Ok(d) => {\n" + + " ok { value } -> {\n" + " fn inner(u: string) uses { net } -> Result {\n" + " match http.get(u) {\n" + - " Ok(data) => ok(data),\n" + - " Err(e) => err(e),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + " }\n" + " }\n" + - " ok(d)\n" + - " },\n" + - " Err(e) => err(e),\n" + + " ok(value)\n" + + " }\n" + + " err { error } -> err(error)\n" + " }\n" + "}\n"; expect(() => compile(src)).not.toThrow(); @@ -256,11 +256,11 @@ describe("UNS005: multiple calls in one fn", () => { "?bs 0.9\n" + "fn doWork(url: string, path: string) uses { net, fs } -> Result {\n" + " match http.get(url) {\n" + - " Ok(a) => match fs.readText(path) {\n" + - " Ok(b) => ok(a),\n" + - " Err(e) => err(e),\n" + - " },\n" + - " Err(e) => err(e),\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(); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 25722e6..85eb924 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -106,8 +106,8 @@ export const EXPLANATIONS: Readonly> = { "?bs 0.9\n" + "fn callApi(url: string) uses { net } -> Result {\n" + " match http.get(url) {\n" + - " Ok(data) => ok(data),\n" + - " Err(e) => err(e),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(error)\n" + " }\n" + "}\n", }, @@ -199,7 +199,7 @@ export const EXPLANATIONS: Readonly> = { "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** — `match http.get(url) { Ok(v) => ..., Err(e) => ... }` wraps " + + "1. **match** — `match http.get(url) { ok { value } -> ...\n err { error } -> ... }` wraps " + "the call in a structural contract check. 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 " + @@ -218,8 +218,8 @@ export const EXPLANATIONS: Readonly> = { "?bs 0.9\n" + "fn fetchUser(id: string) uses { net } -> Result {\n" + " match http.get(`/users/${id}`) {\n" + - " Ok(data) => ok(data),\n" + - " Err(e) => err(`fetch failed: ${e}`),\n" + + " ok { value } -> ok(value)\n" + + " err { error } -> err(`fetch failed: ${error}`)\n" + " }\n" + "}\n", }, From 67b723bf8f1463d0c7c143e99df7f5a3201f81c7 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Tue, 19 May 2026 09:06:46 -0300 Subject: [PATCH 07/19] fix: address third-round Copilot review on UNS005 PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Document unsafe fn body as suppression in uns-check.ts, error-codes.ts, and explanations.ts — compiler already suppressed it, docs now match - Add stdout/stderr firing and suppression tests (568/568 pass) - Add UNS005, CAP003, DEP001, DEP002 rows to AGENTS.md diagnostic table - Update SYN001 in AGENTS.md table to mention throws {} Co-Authored-By: Claude Sonnet 4.6 --- AGENTS.md | 6 +++- packages/compiler/src/error-codes.ts | 5 ++-- packages/compiler/src/passes/uns-check.ts | 5 +++- packages/compiler/tests/uns-check.test.ts | 36 +++++++++++++++++++++++ packages/mcp/src/explanations.ts | 4 ++- 5 files changed, 51 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 457e4ad..9da49bc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -186,14 +186,18 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | BS002 | Unsupported version (e.g. `?bs 99.0`). | Pin to a supported version; see `SUPPORTED_VERSIONS`. | | CAP001 | A fn calls or transitively reaches `http/time/random/fs/stdout/stderr.X` whose capability isn't in its `uses { … }`. (0.2 is direct-only; 0.3 adds transitive call-graph propagation and the diagnostic names the path.) | Either add the capability or remove the call. The diagnostic includes the literal `fn name(...) uses { … } -> ...` rewrite. | | CAP002 | (0.3+) A fn declares a capability nothing in its body or callees reaches. | Remove the unused capability from the `uses { … }` clause, or actually use it. | +| CAP003 | (0.9+, warning) A `uses {}` declaration on an `unsafe fn` is programmer-asserted, not compiler-proven. Inference still runs on the visible body, but `as` casts can alias namespaces and bypass name-based detection. | Treat as advisory; document the unsafe reason clearly. Suppress by removing `uses {}` if the body has no visible stdlib calls. | | UNS001 | (0.3+) `unsafe { … }` block missing a justification string. | `unsafe "" { … }`. | | 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`. | +| DEP001 | (0.9+) A fn transitively calls a callee that declares `reads { X }`, but the calling fn does not declare `reads { X }` itself. | Add `reads { X }` to the caller's header, or restructure the call graph. | +| DEP002 | (0.9+) A fn transitively calls a callee that declares `writes { X }`, but the calling fn does not declare `writes { X }` itself. | Add `writes { X }` to the caller's header, or restructure the call graph. | | 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/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index f9bb791..11a65ba 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -151,8 +151,9 @@ const E: Record = { 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 — either wrap in `match` to make the success and failure paths explicit, or use " + - "`unsafe \"\" { ... }` to accept the uncertainty with a written explanation", + "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 ns.method(...) { ok { value } -> ...\n err { error } -> ... }` — it makes both success " + "and failure paths explicit; use `unsafe` only when you are certain " + diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index a72fd22..a0cfe3e 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -9,6 +9,7 @@ * - 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. @@ -20,7 +21,9 @@ * 1. Wrap in `match` to handle both Ok and Err arms. * 2. Use `unsafe "" { ... }` to accept the uncertainty * with a written explanation. - * 3. (Future) Declare `ensures: "..."` on the callee — allows + * 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. diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts index c4f5882..34394e3 100644 --- a/packages/compiler/tests/uns-check.test.ts +++ b/packages/compiler/tests/uns-check.test.ts @@ -234,6 +234,42 @@ describe("UNS005: inner fn exclusion", () => { }); }); +// --------------------------------------------------------------------------- +// 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.write(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 // --------------------------------------------------------------------------- diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 85eb924..4af4a13 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -204,7 +204,9 @@ export const EXPLANATIONS: Readonly> = { "`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. **(Future) ensures annotation** — when `ensures: \"...\"` lands in a future version, " + + "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: { From 1f707fe6e6e3e6287f51f7178d73a48df758402f Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Tue, 19 May 2026 13:08:55 -0300 Subject: [PATCH 08/19] fix(uns-check): address fourth-round Copilot review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - isDirectMatchSubject: add forward check to prevent false suppression when a stdlib call is part of a larger match scrutinee expression. Passes the call's closing paren index and verifies that what follows (skipping grouping parens) is `{`, not another operator. - STDLIB_CAPS: derive from STDLIB_TO_CAP (exported from cap-check.ts) so the two passes share one canonical namespace list. - AGENTS.md: remove premature throws {} references from SYN001 row (throws {} support lands in a separate PR). - README.md: update MCP transform output to include warnings field; expand explain code list to include CAP003, UNS005, DEP001, DEP002. - uns-check.test.ts: add regression test — stdlib call inside a match arm (not the scrutinee) still fires UNS005. Co-Authored-By: Claude Sonnet 4.6 --- README.md | 4 +- packages/compiler/src/passes/cap-check.ts | 4 +- packages/compiler/src/passes/uns-check.ts | 49 +++++++++++++++++++---- packages/compiler/tests/uns-check.test.ts | 16 ++++++++ 4 files changed, 61 insertions(+), 12 deletions(-) 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/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/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index a0cfe3e..082ab7a 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -36,12 +36,9 @@ 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"; -/** - * stdlib namespaces that consume external capabilities. - * Must stay in sync with STDLIB_TO_CAP in cap-check.ts (the canonical source). - */ -const STDLIB_CAPS = new Set(["http", "fs", "time", "random", "stdout", "stderr"]); +const STDLIB_CAPS = new Set(Object.keys(STDLIB_TO_CAP)); interface CharRange { start: number; @@ -110,7 +107,9 @@ export function passUnsCheck(src: string, version: VersionInfo): string { // Suppression 2: direct subject of a `match` expression. // Skips trivia and `await` — `match await http.get(url) { }` is fine. - if (isDirectMatchSubject(tokens, i)) continue; + // 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; const entry = getErrorCode("UNS005")!; const loc = locationOf(src, tok.start); @@ -201,8 +200,14 @@ function collectUnsafeBlockRanges(tokens: Token[], out: CharRange[]): void { * 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): boolean { +function isDirectMatchSubject(tokens: Token[], callIdx: number, closingParenIdx?: number): boolean { let i = callIdx - 1; while (i >= 0) { const t = tokens[i]; @@ -229,7 +234,35 @@ function isDirectMatchSubject(tokens: Token[], callIdx: number): boolean { i--; continue; } - return t.kind === "keyword" && t.keyword === "match"; + 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; } diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts index 34394e3..13e6d4d 100644 --- a/packages/compiler/tests/uns-check.test.ts +++ b/packages/compiler/tests/uns-check.test.ts @@ -124,6 +124,22 @@ describe("UNS005: suppressed by match", () => { "}\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"); + }); }); // --------------------------------------------------------------------------- From 61ae009a999501182fe0cbe04aade8f1e61d1774 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Tue, 19 May 2026 17:10:59 -0300 Subject: [PATCH 09/19] fix: address Copilot review on UNS005 PR (fifth round) - Fix stderr.write -> stderr.println (stderr.write doesn't exist in runtime) - Remove literal \n from inline code backtick spans in idiom/explanations/CHANGELOG - Add regression test: UNS005 fires when stdlib call is inside a binary expression in the match scrutinee (forward scan correctly rejects suppression) - Fix UNS005 explanation to use a fenced code block for the match example Co-Authored-By: Botkowski --- packages/compiler/src/error-codes.ts | 6 +++--- packages/compiler/tests/uns-check.test.ts | 17 ++++++++++++++++- packages/mcp/src/explanations.ts | 5 +++-- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index 11a65ba..5422236 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -155,9 +155,9 @@ const E: Record = { "`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 ns.method(...) { ok { value } -> ...\n err { error } -> ... }` — it makes both success " + - "and failure paths explicit; use `unsafe` only when you are certain " + - "about the shape and want to document why", + "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: diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts index 13e6d4d..7c2eaf3 100644 --- a/packages/compiler/tests/uns-check.test.ts +++ b/packages/compiler/tests/uns-check.test.ts @@ -140,6 +140,21 @@ describe("UNS005: suppressed by match", () => { "}\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"); + }); }); // --------------------------------------------------------------------------- @@ -269,7 +284,7 @@ describe("UNS005: stdout and stderr namespaces", () => { const src = "?bs 0.9\n" + "fn logErr(msg: string) uses { stderr } -> string {\n" + - " const r = stderr.write(msg);\n" + + " const r = stderr.println(msg);\n" + " msg\n" + "}\n"; expect(() => compile(src)).toThrow("UNS005"); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 4af4a13..afe74c4 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -199,8 +199,9 @@ export const EXPLANATIONS: Readonly> = { "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** — `match http.get(url) { ok { value } -> ...\n err { error } -> ... }` wraps " + - "the call in a structural contract check. Both success and failure paths are explicit. " + + "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" + From 67146d23aa12e0dfab13d9968837bbf5f6d06a7d Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Tue, 19 May 2026 21:04:09 -0300 Subject: [PATCH 10/19] fix(mcp): include warnings in transform tool response MCP server was destructuring only {code, forms, version} from transform(), dropping the warnings array entirely. README already documented warnings in the response contract (e.g. CAP003 non-blocking diagnostics). Co-Authored-By: Botkowski --- packages/mcp/src/server.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index d5a371c..10f89db 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -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] }); From b273e693f2d70a2b7f5408ca487a9e95c7428bb3 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Wed, 20 May 2026 05:09:49 -0300 Subject: [PATCH 11/19] fix(uns-check): suppress UNS005 when passUnsafe will fire UNS003 A malformed unsafe expression like `unsafe "reason" http.get(url)` (missing `{}`) was causing UNS005 to fire first, preventing passUnsafe from surfacing the more specific UNS003 diagnostic. Add isMalformedUnsafeExpr guard and a regression test. Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/uns-check.ts | 30 +++++++++++++++++++++++ packages/compiler/tests/uns-check.test.ts | 25 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index 082ab7a..08900c3 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -111,6 +111,11 @@ export function passUnsCheck(src: string, version: VersionInfo): string { // 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; @@ -145,6 +150,12 @@ export function passUnsCheck(src: string, version: VersionInfo): string { } } + // 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); } @@ -192,6 +203,25 @@ function collectUnsafeBlockRanges(tokens: Token[], out: CharRange[]): void { // 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 isMalformedUnsafeExpr(tokens: Token[], callIdx: number): boolean { + let i = callIdx - 1; + while (i >= 0 && (tokens[i]?.kind === "whitespace" || tokens[i]?.kind === "newline")) i--; + if (i < 0 || tokens[i]?.kind !== "string") return false; + i--; + while (i >= 0 && (tokens[i]?.kind === "whitespace" || tokens[i]?.kind === "newline")) 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 diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts index 7c2eaf3..889d0a8 100644 --- a/packages/compiler/tests/uns-check.test.ts +++ b/packages/compiler/tests/uns-check.test.ts @@ -333,3 +333,28 @@ describe("UNS005: multiple calls in one fn", () => { 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"); + } + }); +}); From 2bd7cad4271497abd68d7b559a6b351c5ed0fb35 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Wed, 20 May 2026 09:13:04 -0300 Subject: [PATCH 12/19] fix: import STDLIB_TO_CAP from canonical source; update MCP transform description - intent-check.ts had its own STDLIB_TO_CAP copy despite cap-check.ts being the declared canonical source. Import directly from cap-check.js to prevent drift if namespaces are added. - MCP transform tool description now reflects the warnings field added to the response in a prior round. Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/intent-check.ts | 11 +---------- packages/mcp/src/server.ts | 2 +- 2 files changed, 2 insertions(+), 11 deletions(-) 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/mcp/src/server.ts b/packages/mcp/src/server.ts index 10f89db..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: { From 6c4280fd44483b040a8a5b1ea4bb030b63e23b04 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 21 May 2026 10:55:38 -0300 Subject: [PATCH 13/19] fix(uns-check): import computeNesting from _callgraph; fix trivia skipping in isMalformedUnsafeExpr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove local `computeNesting` duplicate; import shared version from `_callgraph.ts` (same helper used by dep-check and thr-check). - `isMalformedUnsafeExpr` now skips lineComment and blockComment tokens in addition to whitespace/newline, so `unsafe "reason" // comment\nhttp.get(...)` is correctly recognized as a malformed unsafe expression. - Fix cap-check test that expected CAP001 on a bare http.get — UNS005 fires first at ?bs 0.9; wrap in match to suppress UNS005 so CAP001 still fires as intended. Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/uns-check.ts | 26 ++++++----------------- packages/compiler/tests/cap-check.test.ts | 6 +++++- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index 08900c3..72b7110 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -37,6 +37,7 @@ 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 } from "./_callgraph.js"; const STDLIB_CAPS = new Set(Object.keys(STDLIB_TO_CAP)); @@ -212,12 +213,16 @@ function collectUnsafeBlockRanges(tokens: Token[], out: CharRange[]): void { * 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; - while (i >= 0 && (tokens[i]?.kind === "whitespace" || tokens[i]?.kind === "newline")) i--; + while (i >= 0 && isTrivia(tokens[i]!.kind)) i--; if (i < 0 || tokens[i]?.kind !== "string") return false; i--; - while (i >= 0 && (tokens[i]?.kind === "whitespace" || tokens[i]?.kind === "newline")) i--; + while (i >= 0 && isTrivia(tokens[i]!.kind)) i--; const t = tokens[i]; return !!(t && t.kind === "keyword" && t.keyword === "unsafe"); } @@ -323,20 +328,3 @@ function insideAnyChar(offset: number, ranges: CharRange[]): boolean { return false; } -function computeNesting(decls: FnDecl[]): Map { - const inner = new Map(); - 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; -} diff --git a/packages/compiler/tests/cap-check.test.ts b/packages/compiler/tests/cap-check.test.ts index d246a2e..60ebeb0 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 } -> value\n err { error } -> error\n }\n}\n"; expect(() => t(src)).toThrow(/CAP001/); }); }); From 40b0a032b7777b341fbc72f1f3ea35b53b747283 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 21 May 2026 14:43:59 -0300 Subject: [PATCH 14/19] fix(uns-check): extend isMalformedUnsafeExpr to skip await and grouping parens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Handles `unsafe "r" await ns.call()` and `unsafe "r" (ns.call())` — both are malformed unsafe expressions that passUnsafe will flag as UNS003. Suppress UNS005 for these so the more specific diagnostic wins. Adds two tests covering the new forms. Co-Authored-By: Botkowski --- packages/compiler/src/passes/uns-check.ts | 10 +++++++- packages/compiler/tests/uns-check.test.ts | 30 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index 72b7110..c609beb 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -219,7 +219,15 @@ function isTrivia(k: string): boolean { function isMalformedUnsafeExpr(tokens: Token[], callIdx: number): boolean { let i = callIdx - 1; - while (i >= 0 && isTrivia(tokens[i]!.kind)) i--; + // Skip trivia, await, and grouping parens between the call and the reason string. + // Handles: `unsafe "r" ns.call()`, `unsafe "r" await ns.call()`, `unsafe "r" (ns.call())`. + while (i >= 0) { + const t = tokens[i]!; + if (isTrivia(t.kind)) { i--; continue; } + if (t.kind === "ident" && t.text === "await") { i--; continue; } + if (t.kind === "open" && t.text === "(") { i--; continue; } + break; + } if (i < 0 || tokens[i]?.kind !== "string") return false; i--; while (i >= 0 && isTrivia(tokens[i]!.kind)) i--; diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts index 889d0a8..0bb7d0b 100644 --- a/packages/compiler/tests/uns-check.test.ts +++ b/packages/compiler/tests/uns-check.test.ts @@ -357,4 +357,34 @@ describe("UNS003 takes precedence over UNS005 for malformed unsafe blocks", () = 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"); + } + }); }); From 7ba3f0c78ff5705d46d9571741edfb80d4c7e27c Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 21 May 2026 18:42:32 -0300 Subject: [PATCH 15/19] fix: extend isMalformedUnsafeExpr to catch wrapped stdlib calls; fix test return type isMalformedUnsafeExpr was only scanning through trivia, await, and parens, missing cases like `unsafe "r" foo(http.get(url))` where the call is inside a wrapper. Now also scans past idents, dots, and close-parens so the UNS003 diagnostic wins over UNS005 for any malformed unsafe expression form. Also fixes a cap-check test fixture that returned plain strings from Result-typed arms; arms now use ok()/err() to match the declared return type. Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/uns-check.ts | 9 ++++++--- packages/compiler/tests/cap-check.test.ts | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index c609beb..2cd5831 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -219,13 +219,16 @@ function isTrivia(k: string): boolean { function isMalformedUnsafeExpr(tokens: Token[], callIdx: number): boolean { let i = callIdx - 1; - // Skip trivia, await, and grouping parens between the call and the reason string. - // Handles: `unsafe "r" ns.call()`, `unsafe "r" await ns.call()`, `unsafe "r" (ns.call())`. + // 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" && t.text === "await") { i--; continue; } + if (t.kind === "ident") { i--; continue; } + if (t.kind === "punct" && t.text === ".") { 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; diff --git a/packages/compiler/tests/cap-check.test.ts b/packages/compiler/tests/cap-check.test.ts index 60ebeb0..01313b0 100644 --- a/packages/compiler/tests/cap-check.test.ts +++ b/packages/compiler/tests/cap-check.test.ts @@ -332,7 +332,7 @@ describe("cap-check: no false positive for stdlib namespace in parameter type", // fires because uses { net } is absent. const src = "?bs 0.9\nfn fetchData(url: string) -> Result {\n" + - " match http.get(url) {\n ok { value } -> value\n err { error } -> error\n }\n}\n"; + " match http.get(url) {\n ok { value } -> ok(value)\n err { error } -> err(error)\n }\n}\n"; expect(() => t(src)).toThrow(/CAP001/); }); }); From 1fdbed3cf325f89619e988a49b056dc60c3a7b65 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 21 May 2026 18:50:05 -0300 Subject: [PATCH 16/19] test(uns-check): add coverage for UNS003 precedence with wrapped stdlib call Covers the case where a stdlib call is nested inside a wrapper function inside a malformed unsafe expression (unsafe "r" foo(ns.method())). The extended backwards scan in isMalformedUnsafeExpr must see through the wrapper ident and parens to detect the enclosing unsafe context. Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/tests/uns-check.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts index 0bb7d0b..ef8e51c 100644 --- a/packages/compiler/tests/uns-check.test.ts +++ b/packages/compiler/tests/uns-check.test.ts @@ -387,4 +387,23 @@ describe("UNS003 takes precedence over UNS005 for malformed unsafe blocks", () = 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"); + } + }); }); From 55cf98e367cb5ae2281a1568613e0dd7e927250a Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Fri, 22 May 2026 02:50:35 -0300 Subject: [PATCH 17/19] fix(uns-check): remove duplicate nextSignificant, handle questionDot, fix ok/err caps - Import nextSignificant from _callgraph.ts (shared helper, no drift) - Detect stdlib calls via optional chaining (`?.`) same as dot access - Fix Ok/Err capitalisation in file-level comment (botscript uses lowercase) Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/uns-check.ts | 30 ++++++----------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index 2cd5831..bb9742f 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -18,7 +18,7 @@ * is flagging an omission. * * Suppression mechanisms: - * 1. Wrap in `match` to handle both Ok and Err arms. + * 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 @@ -37,7 +37,7 @@ 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 } from "./_callgraph.js"; +import { computeNesting, nextSignificant } from "./_callgraph.js"; const STDLIB_CAPS = new Set(Object.keys(STDLIB_TO_CAP)); @@ -90,10 +90,13 @@ export function passUnsCheck(src: string, version: VersionInfo): string { if (!tok || tok.kind !== "ident") continue; if (!STDLIB_CAPS.has(tok.text)) continue; - // Must be `stdlib.method(` — confirm the shape before acting. + // 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 !== ".") continue; + if ( + !dotTok || + !((dotTok.kind === "punct" && dotTok.text === ".") || dotTok.kind === "questionDot") + ) continue; const memberIdx = nextSignificant(tokens, dotIdx + 1); const memberTok = tokens[memberIdx]; @@ -313,25 +316,6 @@ function isDirectMatchSubject(tokens: Token[], callIdx: number, closingParenIdx? return false; } -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 insideAnyChar(offset: number, ranges: CharRange[]): boolean { for (const r of ranges) { if (offset >= r.start && offset < r.end) return true; From 4dbc872d1a26cfa9224996f910e54b77c06387e1 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Fri, 22 May 2026 18:41:41 -0300 Subject: [PATCH 18/19] fix(uns-check): use correct accessor in message/rewrite for optional-chaining calls; handle questionDot in isMalformedUnsafeExpr Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/uns-check.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index bb9742f..9d27466 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -125,6 +125,9 @@ export function passUnsCheck(src: string, version: VersionInfo): string { 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", @@ -135,18 +138,18 @@ export function passUnsCheck(src: string, version: VersionInfo): string { start: tok.start, end: closingParen?.end ?? memberTok.end, message: - `'${ns}.${member}(...)' is an external call with no declared result contract — ` + + `'${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 ${ns}.${member}(...) {\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 ${ns}.${member} returns here" { ${ns}.${member}(...) }`, + `unsafe "I know what ${callExpr} returns here" { ${callExpr}(...) }`, }); // Do not advance past the closing paren — inner stdlib calls in the @@ -230,6 +233,7 @@ function isMalformedUnsafeExpr(tokens: Token[], callIdx: number): boolean { 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; From 3351aaebadbf55544ecebfe895d0d860a301106f Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Sat, 23 May 2026 10:52:40 -0300 Subject: [PATCH 19/19] fix(uns-check): start body scan from bodyTokenStart; dedup AGENTS.md table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Copilot review items: 1. passUnsCheck was scanning from decl.tokenStart, including the fn header (params, return type). Changed to decl.bodyTokenStart ?? decl.tokenStart, matching cap-check and intent-check. Adds two tests that pin the body-only scope. 2. AGENTS.md diagnostic table had duplicate rows for CAP003 and DEP001/DEP002 — removed the shorter first-occurrence duplicates, keeping the more detailed entries with transitivity examples. Co-Authored-By: Claude Sonnet 4.6 --- AGENTS.md | 3 --- packages/compiler/src/passes/uns-check.ts | 2 +- packages/compiler/tests/uns-check.test.ts | 29 +++++++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9da49bc..0244d6e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -186,14 +186,11 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | BS002 | Unsupported version (e.g. `?bs 99.0`). | Pin to a supported version; see `SUPPORTED_VERSIONS`. | | CAP001 | A fn calls or transitively reaches `http/time/random/fs/stdout/stderr.X` whose capability isn't in its `uses { … }`. (0.2 is direct-only; 0.3 adds transitive call-graph propagation and the diagnostic names the path.) | Either add the capability or remove the call. The diagnostic includes the literal `fn name(...) uses { … } -> ...` rewrite. | | CAP002 | (0.3+) A fn declares a capability nothing in its body or callees reaches. | Remove the unused capability from the `uses { … }` clause, or actually use it. | -| CAP003 | (0.9+, warning) A `uses {}` declaration on an `unsafe fn` is programmer-asserted, not compiler-proven. Inference still runs on the visible body, but `as` casts can alias namespaces and bypass name-based detection. | Treat as advisory; document the unsafe reason clearly. Suppress by removing `uses {}` if the body has no visible stdlib calls. | | UNS001 | (0.3+) `unsafe { … }` block missing a justification string. | `unsafe "" { … }`. | | 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`. | -| DEP001 | (0.9+) A fn transitively calls a callee that declares `reads { X }`, but the calling fn does not declare `reads { X }` itself. | Add `reads { X }` to the caller's header, or restructure the call graph. | -| DEP002 | (0.9+) A fn transitively calls a callee that declares `writes { X }`, but the calling fn does not declare `writes { X }` itself. | Add `writes { X }` to the caller's header, or restructure the call graph. | | 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. | diff --git a/packages/compiler/src/passes/uns-check.ts b/packages/compiler/src/passes/uns-check.ts index 9d27466..c695c40 100644 --- a/packages/compiler/src/passes/uns-check.ts +++ b/packages/compiler/src/passes/uns-check.ts @@ -77,7 +77,7 @@ export function passUnsCheck(src: string, version: VersionInfo): string { const open: FnDecl[] = []; let nextInner = 0; - for (let i = decl.tokenStart; i < decl.tokenEnd; i++) { + 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) { diff --git a/packages/compiler/tests/uns-check.test.ts b/packages/compiler/tests/uns-check.test.ts index ef8e51c..b51c036 100644 --- a/packages/compiler/tests/uns-check.test.ts +++ b/packages/compiler/tests/uns-check.test.ts @@ -407,3 +407,32 @@ describe("UNS003 takes precedence over UNS005 for malformed unsafe blocks", () = } }); }); + +// --------------------------------------------------------------------------- +// 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"); + }); +});