From f8fe3583691b7c7335e8ba022bf0c4fbac27abe0 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Wed, 10 Jun 2026 11:50:32 -0300 Subject: [PATCH 01/11] =?UTF-8?q?feat(compiler):=20SYN007=20=E2=80=94=20wa?= =?UTF-8?q?rn=20on=20fetch()=20calls=20that=20bypass=20the=20net=20capabil?= =?UTF-8?q?ity=20model=20(=3Fbs=200.7+)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetch() makes real HTTP requests but is invisible to CAP001, which only checks http.* member calls. A fn that calls fetch() has an undeclared net dependency — uses { net } is absent from its header and callers cannot see the network access. This is the same bypass class as console.* (SYN003). Detects: fetch(...), fetch?.(...), and TypeScript instantiation forms fetch(...). Suppressed inside unsafe { } blocks and unsafe fn bodies. Excludes obj.fetch(...) method calls. Fires at ?bs 0.7+. Co-Authored-By: Claude Sonnet 4.6 --- AGENTS.md | 1 + packages/compiler/src/error-codes.ts | 41 +++++++ packages/compiler/src/passes/syn-check.ts | 90 ++++++++++++++- packages/compiler/tests/error-codes.test.ts | 2 +- packages/compiler/tests/syn007-check.test.ts | 112 +++++++++++++++++++ packages/mcp/src/explanations.ts | 42 +++++++ packages/mcp/tests/server.test.ts | 1 + 7 files changed, 287 insertions(+), 2 deletions(-) create mode 100644 packages/compiler/tests/syn007-check.test.ts diff --git a/AGENTS.md b/AGENTS.md index 6bb47fdb..d4927b7e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -200,6 +200,7 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | SYN003 | (0.7+, warning) A fn body contains a `console.*` call (console.log, console.error, etc.). Direct console output bypasses the `stdout`/`stderr` capability model — the compiler cannot enforce or surface the output declaration for callers. | Replace `console.log(...)` with `stdout.write(...)` and add `uses { stdout }` to the fn header; replace `console.error(...)` with `stderr.write(...)` and add `uses { stderr }`. | | SYN005 | (0.7+, warning) A fn body accesses `process.env`. `process.env` is a global deployment-environment namespace — access is invisible to callers, there is no capability or resource declaration that covers it, and the fn has an undeclared dependency on deployment configuration. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `env`. `obj.process.env` (member access on a local), `unsafe {}` blocks, and `unsafe "reason" fn` bodies are excluded. | Pass config and secrets as explicit fn parameters so the dependency is visible in the call signature; if env access is required at the load site, wrap in `unsafe "reads deployment env" { }`. | | SYN006 | (0.7+, warning) A fn body calls `process.exit()`, `process?.exit()`, or `process.exit?.()`. All forms terminate the entire host process — not just the fn, not just the bot. They produce no return value, bypass `Result` propagation, `throws {}`, `match`, and any caller recovery path. No capability declaration covers them. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `exit` then `(` or `?.(`. `obj.process.exit(...)`, `process.exit` without `(`, and `process.exitCode` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Return `err(...)` and let the caller decide whether to terminate. If `process.exit` is genuinely required at a bootstrap entry point, wrap in `unsafe "exits on invalid config" { process.exit(1) }`. | +| SYN007 | (0.7+, warning) A fn body calls `fetch(...)`, `fetch?.(...)`, or TypeScript instantiation form `fetch(...)`. The `fetch` global makes real HTTP requests but is invisible to CAP001: `uses { net }` is never triggered, and callers cannot see the network dependency in the fn header. Detection: `fetch` not preceded by `.`/`?.`, followed by `(`, `?.(`, or `(`. `obj.fetch(...)` (method call on a local) is excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Replace with `http.get(url)` or `http.post(url, body)` and add `uses { net }` to the fn header. If the raw `fetch` API is genuinely required (e.g. a thin adapter), wrap in `unsafe "wraps fetch directly" { fetch(...) }`. | | 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. | | INT003 | (0.7+) A fn declares `intent: "idempotent"` but also has `uses { random }` or `uses { time }`. Both capabilities produce different values on each call, making the function non-idempotent. Only `random` and `time` are flagged; other capabilities are not structurally flagged by this check (INT003 is a narrow heuristic, not a proof of idempotence). | Remove `random`/`time` from `uses {}`, or change the intent. | | INT004 | (0.7+) A fn declares `intent: "idempotent"` but its body directly references `random` or `time` without declaring them. Under-declaration variant of INT003 — fires when INT003 does not. | Remove the non-idempotent call from the body, or declare the capability and remove the idempotent intent. | diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index aa8aa867..6f415679 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -611,6 +611,47 @@ const E: Record = { " return ok(undefined)\n" + "}", }, + SYN007: { + code: "SYN007", + title: "fetch() call bypasses the net capability model — use http.get() / http.post() instead", + rule: + "`fetch(...)`, `fetch?.(...)`, and TypeScript instantiation forms like `fetch(...)` " + + "make real HTTP requests at runtime but are invisible to botscript's capability model: " + + "CAP001 checks for `http.*` member calls, not the `fetch` global. A fn that calls `fetch` " + + "has an undeclared network dependency — no `uses { net }` will reflect it in the fn header, " + + "no audit tool can see it, and callers cannot reason about the blast radius.", + idiom: + "replace `fetch(url, init)` with `http.get(url)` or `http.post(url, body)` and add " + + "`uses { net }` to the fn header; if the raw `fetch` API is genuinely required " + + "(e.g. a thin adapter that wraps the global for a specific use case), " + + "wrap in `unsafe \"wraps fetch directly\" { fetch(...) }`", + rewrite: + "// before — fetch is invisible to the capability model\n" + + "fn loadData(url: string) -> Result {\n" + + " const res = await fetch(url) // SYN007\n" + + " return ok(await res.text())\n" + + "}\n\n" + + "// after — http.get declares the net dependency\n" + + "fn loadData(url: string) uses { net } -> Result {\n" + + " match await http.get(url) {\n" + + " ok { res } -> ok(await res.text())\n" + + " err { e } -> err(e.message)\n" + + " }\n" + + "}", + example: + "// SYN007: fetch bypasses the net capability model\n" + + "fn getData(url: string) -> string {\n" + + " const res = await fetch(url) // SYN007\n" + + " return await res.text()\n" + + "}\n\n" + + "// fix: use http.get and declare the capability\n" + + "fn getData(url: string) uses { net } -> Result {\n" + + " match await http.get(url) {\n" + + " ok { res } -> ok(await res.text())\n" + + " err { e } -> err(e.message)\n" + + " }\n" + + "}", + }, DEP001: { code: "DEP001", title: "fn transitively reads a resource category not declared in its header", diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index eb1c94ed..56e39e9f 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -23,6 +23,14 @@ * depends on runtime deployment values that callers cannot see, * audit, or mock in tests. The idiomatic fix is to pass config * and secrets as explicit fn parameters. + * + * SYN007 A `fetch(...)` call was detected in a fn body (?bs 0.7+). + * `fetch()` makes real HTTP requests at runtime but is invisible to + * botscript's capability model: CAP001 checks for `http.*` member + * calls, not the `fetch` global. A fn that calls `fetch` has an + * undeclared network dependency — no `uses { net }` in the fn header + * will reflect it. Use `http.get()` or `http.post()` instead so the + * net capability is declared and visible to callers. */ import type { Diagnostic } from "../diagnostics.js"; @@ -55,8 +63,9 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const syn003 = getErrorCode("SYN003")!; const syn005 = getErrorCode("SYN005")!; const syn006 = getErrorCode("SYN006")!; + const syn007 = getErrorCode("SYN007")!; - // Collect char-offset ranges where SYN002/SYN003/SYN005/SYN006 are suppressed: + // Collect char-offset ranges where SYN002/SYN003/SYN005/SYN006/SYN007 are suppressed: // 1. `unsafe "reason" { ... }` expression blocks — explicit acknowledgment. // 2. `unsafe "reason" fn` bodies — the entire body is exempt, including any // non-unsafe nested fns declared inside it (matching uns-check's pattern). @@ -396,6 +405,85 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult rewrite: syn006.rewrite, }); } + + // SYN007: fetch() call detection. + // Fires when a fn body calls `fetch(...)`, `fetch?.(...)`, or a TypeScript + // instantiation form `fetch(...)`. All forms make real HTTP requests + // at runtime but are invisible to CAP001 (which only checks `http.*` member + // calls). A fn that uses `fetch` directly has an undeclared `net` dependency. + // Suppressed inside `unsafe { }` blocks and `unsafe fn` bodies. + nextInner = 0; + const open007: typeof inner = []; + for (let i = bodyStart; i < decl.tokenEnd; i++) { + while (open007.length > 0 && open007[open007.length - 1]!.tokenEnd <= i) open007.pop(); + while (nextInner < inner.length && inner[nextInner]!.tokenStart <= i) { + open007.push(inner[nextInner]!); + nextInner++; + } + if (open007.length > 0) continue; + + const tok7 = tokens[i]; + if (!tok7 || tok7.kind !== "ident" || tok7.text !== "fetch") continue; + + // Exclude: `obj.fetch(...)` or `obj?.fetch(...)` — member call on a local. + const prevIdx7 = prevSignificant(tokens, i - 1); + const prev7 = tokens[prevIdx7]; + if (prev7 && ((prev7.kind === "punct" && prev7.text === ".") || prev7.kind === "questionDot")) + continue; + + // Must be followed by `(`, `?.(`, or `(` — confirming this is a call, + // not a bare `fetch` reference or a property access like `fetch.name`. + let afterFetchIdx = nextSignificant(tokens, i + 1); + const afterFetch = tokens[afterFetchIdx]; + if (!afterFetch) continue; + + // TypeScript instantiation form: `fetch(...)` — skip over `<...>` to find `(` + if (afterFetch.kind === "operator" && afterFetch.text === "<") { + let anglDepth = 1; + afterFetchIdx++; + while (afterFetchIdx < decl.tokenEnd && anglDepth > 0) { + const at = tokens[afterFetchIdx]; + if (!at) { afterFetchIdx++; continue; } + if (at.kind === "operator" && at.text === "<") { anglDepth++; } + else if (at.kind === "operator" && at.text === ">") { anglDepth--; } + else if (at.kind === "operator" && (at.text === ">>" || at.text === ">>>")) { + anglDepth -= at.text.length - 1; + } + afterFetchIdx++; + } + afterFetchIdx = nextSignificant(tokens, afterFetchIdx); + const afterAngle = tokens[afterFetchIdx]; + if (!afterAngle || !(afterAngle.kind === "open" && afterAngle.text === "(")) continue; + } else if (afterFetch.kind === "questionDot") { + // `fetch?.(...)` — optional call + const afterQD7 = nextSignificant(tokens, afterFetchIdx + 1); + const afterQDTok = tokens[afterQD7]; + if (!afterQDTok || !(afterQDTok.kind === "open" && afterQDTok.text === "(")) continue; + } else if (!(afterFetch.kind === "open" && afterFetch.text === "(")) { + continue; + } + + // Suppression check: unsafe block or unsafe fn body + if (isInsideRange(tok7.start, unsafeRanges)) continue; + + const loc7 = locationOf(src, tok7.start); + warnings.push({ + code: "SYN007", + severity: "warning", + file: null, + line: loc7.line, + column: loc7.column, + start: tok7.start, + end: tok7.end, + message: + `fn '${decl.name}' calls fetch() — fetch bypasses the net capability model; ` + + `CAP001 cannot see it; use http.get() or http.post() and declare uses { net }, ` + + `or wrap in unsafe "wraps fetch directly" { fetch(...) }`, + rule: syn007.rule, + idiom: syn007.idiom, + rewrite: syn007.rewrite, + }); + } } return { code: src, warnings }; diff --git a/packages/compiler/tests/error-codes.test.ts b/packages/compiler/tests/error-codes.test.ts index 0eb75b0a..4d4d9632 100644 --- a/packages/compiler/tests/error-codes.test.ts +++ b/packages/compiler/tests/error-codes.test.ts @@ -15,7 +15,7 @@ describe("error-code registry", () => { "INT001", "INT002", "INT003", "INT004", "INT005", "MAT001", "MAT002", "MAT003", "MAT004", "RES001", "RES002", - "SYN001", "SYN002", "SYN003", "SYN005", "SYN006", + "SYN001", "SYN002", "SYN003", "SYN005", "SYN006", "SYN007", "THR001", "THR002", "THR003", "THR004", "UNS001", "UNS002", "UNS003", "UNS004", "UNS005", "VER001", "VER002", "VER003", diff --git a/packages/compiler/tests/syn007-check.test.ts b/packages/compiler/tests/syn007-check.test.ts new file mode 100644 index 00000000..f32d4ac3 --- /dev/null +++ b/packages/compiler/tests/syn007-check.test.ts @@ -0,0 +1,112 @@ +/** + * Tests for SYN007: fetch() call detection in fn bodies (?bs 0.7+). + * + * SYN007 is a non-blocking warning — transform must not throw. + */ + +import { describe, expect, it } from "vitest"; +import { transform } from "../src/transform.js"; + +describe("SYN007: fetch() call detection (?bs 0.7+)", () => { + it("fires on fetch() call in a fn body", () => { + const src = + "?bs 0.7\n" + + "fn loadData(url: string) -> string {\n" + + " return fetch(url)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(true); + }); + + it("fires on fetch() with multiple args", () => { + const src = + "?bs 0.7\n" + + "fn postData(url: string, body: string) -> string {\n" + + " return fetch(url, { method: 'POST', body })\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(true); + }); + + it("fires on fetch?.() optional call form", () => { + const src = + "?bs 0.7\n" + + "fn loadData(url: string) -> string {\n" + + " return fetch?.(url)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(true); + }); + + it("fires on fetch() TypeScript instantiation form", () => { + const src = + "?bs 0.7\n" + + "fn loadData(url: string) -> string {\n" + + " return fetch(url)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(true); + }); + + it("does not fire when fetch is inside an unsafe block", () => { + const src = + "?bs 0.7\n" + + "fn loadData(url: string) -> string {\n" + + ' return unsafe "uses raw fetch for compat" { fetch(url) }\n' + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); + + it("does not fire when fetch is inside an unsafe fn body", () => { + const src = + "?bs 0.7\n" + + 'unsafe "wraps fetch" fn loadRaw(url: string) -> string {\n' + + " return fetch(url)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); + + it("does not fire on .fetch() — method call on a local object", () => { + const src = + "?bs 0.7\n" + + "fn loadData(client: { fetch: (url: string) -> string }) -> string {\n" + + " return client.fetch(url)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); + + it("does not fire on bare fetch reference — not called", () => { + const src = + "?bs 0.7\n" + + "fn getFetcher() -> string {\n" + + " return typeof fetch !== 'undefined'\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); + + it("does not fire below ?bs 0.7", () => { + const src = + "?bs 0.6\n" + + "fn loadData(url: string) -> string {\n" + + " return fetch(url)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); + + it("severity is warning (non-blocking)", () => { + const src = + "?bs 0.7\n" + + "fn loadData(url: string) -> string {\n" + + " return fetch(url)\n" + + "}\n"; + const result = transform(src); + const w = result.warnings.find((w) => w.code === "SYN007"); + expect(w).toBeDefined(); + expect(w?.severity).toBe("warning"); + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index fe445593..d909728f 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -812,6 +812,48 @@ export const EXPLANATIONS: Readonly> = { "}\n", }, }, + SYN007: { + code: "SYN007", + title: "fetch() call bypasses the net capability model — use http.get() / http.post() instead", + body: + "Botscript's capability model maps network access to `uses { net }` via the `http` stdlib " + + "namespace. CAP001 enforces this by checking for `http.*` member calls (e.g. `http.get()`, " + + "`http.post()`). But the `fetch` global makes real HTTP requests without any `http.*` call — " + + "CAP001 never fires, and `uses { net }` is absent from the fn header.\n\n" + + "The impact is concrete:\n" + + "- A fn that calls `fetch(url)` has a live network dependency that callers cannot see\n" + + "- The capability manifest hash proves the source unchanged, but the network surface is invisible\n" + + "- Audit tools and orchestrators that read `uses { net }` declarations will miss the dependency\n" + + "- A bot author inspecting another module's header has no signal that it makes network calls\n\n" + + "This is the same class of bypass as `console.*` (SYN003): both are real-effects calls " + + "that sidestep the declared capability surface.\n\n" + + "**Fix:** replace `fetch(url)` with `http.get(url)` (or `http.post(url, body)`) and add " + + "`uses { net }` to the fn header. The stdlib `http.*` methods return `Result`, " + + "so the error path is explicit and the net dependency is now visible to callers. " + + "If the raw `fetch` API is genuinely required (e.g. a thin compatibility adapter), " + + "wrap in `unsafe \"wraps fetch directly\" { fetch(...) }` to make the escape hatch visible in the diff.\n\n" + + "SYN007 fires at `?bs 0.7+` as a non-blocking warning. Detection is token-based: " + + "`fetch` not preceded by `.`/`?.` followed by `(`, `?.(`, or `(`; " + + "TypeScript instantiation forms `fetch(...)` are also detected. " + + "`obj.fetch(...)` (method call on a local object) is excluded. " + + "Calls inside `unsafe { }` blocks or `unsafe \"reason\" fn` bodies are suppressed.", + example: { + fails: + "?bs 0.7\n" + + "fn loadData(url: string) -> string {\n" + + " const res = await fetch(url)\n" + + " return await res.text()\n" + + "}\n", + passes: + "?bs 0.7\n" + + "fn loadData(url: string) uses { net } -> Result {\n" + + " match await http.get(url) {\n" + + " ok { res } -> ok(await res.text())\n" + + " err { e } -> err(e.message)\n" + + " }\n" + + "}\n", + }, + }, DEP001: { code: "DEP001", title: "fn transitively reads a resource category not declared in its header", diff --git a/packages/mcp/tests/server.test.ts b/packages/mcp/tests/server.test.ts index 57e87d71..9619865e 100644 --- a/packages/mcp/tests/server.test.ts +++ b/packages/mcp/tests/server.test.ts @@ -75,6 +75,7 @@ describe("botscript-mcp explanations", () => { "SYN003", "SYN005", "SYN006", + "SYN007", "THR001", "THR002", "THR003", From b7bb2cb9753e41bcb40c20db1d499a1d5a5655a4 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Wed, 10 Jun 2026 15:28:39 -0300 Subject: [PATCH 02/11] fix(syn007): correct nested-generic depth scan, http.post signature, and Promise return type - `anglDepth -= at.text.length - 1` was off by one for `>>` and `>>>` tokens; `fetch>(url)` was never detected. Aligns with the SYN002 pattern that uses `depth -= t.text.length`. - Add nested-generic test to prevent regressions. - `http.post` takes an optional `RequestInit` bag, not a raw body string; fix idiom in error-codes.ts and MCP explanation to use `{ body }`. - MCP explanation stated `Result` as the return type; the actual return is `Promise>` (runtime/effects.ts). Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/error-codes.ts | 2 +- packages/compiler/src/passes/syn-check.ts | 2 +- packages/compiler/tests/syn007-check.test.ts | 10 ++++++++++ packages/mcp/src/explanations.ts | 6 +++--- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index 6f415679..6ea60d38 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -621,7 +621,7 @@ const E: Record = { "has an undeclared network dependency — no `uses { net }` will reflect it in the fn header, " + "no audit tool can see it, and callers cannot reason about the blast radius.", idiom: - "replace `fetch(url, init)` with `http.get(url)` or `http.post(url, body)` and add " + + "replace `fetch(url, init)` with `http.get(url)` or `http.post(url, { body })` and add " + "`uses { net }` to the fn header; if the raw `fetch` API is genuinely required " + "(e.g. a thin adapter that wraps the global for a specific use case), " + "wrap in `unsafe \"wraps fetch directly\" { fetch(...) }`", diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index 56e39e9f..7a0a2b72 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -447,7 +447,7 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult if (at.kind === "operator" && at.text === "<") { anglDepth++; } else if (at.kind === "operator" && at.text === ">") { anglDepth--; } else if (at.kind === "operator" && (at.text === ">>" || at.text === ">>>")) { - anglDepth -= at.text.length - 1; + anglDepth -= at.text.length; } afterFetchIdx++; } diff --git a/packages/compiler/tests/syn007-check.test.ts b/packages/compiler/tests/syn007-check.test.ts index f32d4ac3..5471c149 100644 --- a/packages/compiler/tests/syn007-check.test.ts +++ b/packages/compiler/tests/syn007-check.test.ts @@ -48,6 +48,16 @@ describe("SYN007: fetch() call detection (?bs 0.7+)", () => { expect(result.warnings.some((w) => w.code === "SYN007")).toBe(true); }); + it("fires on fetch>() nested generic instantiation form", () => { + const src = + "?bs 0.7\n" + + "fn loadData(url: string) -> string {\n" + + " return fetch>(url)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(true); + }); + it("does not fire when fetch is inside an unsafe block", () => { const src = "?bs 0.7\n" + diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index d909728f..8fbc09ee 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -827,9 +827,9 @@ export const EXPLANATIONS: Readonly> = { "- A bot author inspecting another module's header has no signal that it makes network calls\n\n" + "This is the same class of bypass as `console.*` (SYN003): both are real-effects calls " + "that sidestep the declared capability surface.\n\n" + - "**Fix:** replace `fetch(url)` with `http.get(url)` (or `http.post(url, body)`) and add " + - "`uses { net }` to the fn header. The stdlib `http.*` methods return `Result`, " + - "so the error path is explicit and the net dependency is now visible to callers. " + + "**Fix:** replace `fetch(url)` with `http.get(url)` (or `http.post(url, { body })`) and add " + + "`uses { net }` to the fn header. The stdlib `http.*` methods return `Promise>` — " + + "`await` them to get a `Result`, making the error path explicit and the net dependency visible to callers. " + "If the raw `fetch` API is genuinely required (e.g. a thin compatibility adapter), " + "wrap in `unsafe \"wraps fetch directly\" { fetch(...) }` to make the escape hatch visible in the diff.\n\n" + "SYN007 fires at `?bs 0.7+` as a non-blocking warning. Detection is token-based: " + From 001525ca05e1bc8098983e846df92c11379511ac Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Wed, 10 Jun 2026 23:30:20 -0300 Subject: [PATCH 03/11] =?UTF-8?q?fix(compiler):=20SYN007=20address=20Copil?= =?UTF-8?q?ot=20review=20=E2=80=94=20async=20fn,=20http.post=20signature,?= =?UTF-8?q?=20>>=20consistency?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add SYN006 to syn-check.ts header comment (doc was listing SYN002/SYN003/SYN005/SYN007, skipping SYN006) - error-codes.ts SYN007 rewrite/example: fn signatures using await must be async fn with Promise<> return - explanations.ts SYN007 fails/passes examples: same async fn + Promise<> fix - AGENTS.md SYN007 row: http.post(url, body) → http.post(url, { body }) to match RequestInit signature - syn-check.ts generic depth scan: unify >> / >>> handling to match SYN002 pattern (single clause, depth -= t.text.length) Co-Authored-By: Claude Sonnet 4.6 --- AGENTS.md | 2 +- packages/compiler/src/error-codes.ts | 8 ++++---- packages/compiler/src/passes/syn-check.ts | 10 ++++++++-- packages/mcp/src/explanations.ts | 4 ++-- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d4927b7e..a0a03c75 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -200,7 +200,7 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | SYN003 | (0.7+, warning) A fn body contains a `console.*` call (console.log, console.error, etc.). Direct console output bypasses the `stdout`/`stderr` capability model — the compiler cannot enforce or surface the output declaration for callers. | Replace `console.log(...)` with `stdout.write(...)` and add `uses { stdout }` to the fn header; replace `console.error(...)` with `stderr.write(...)` and add `uses { stderr }`. | | SYN005 | (0.7+, warning) A fn body accesses `process.env`. `process.env` is a global deployment-environment namespace — access is invisible to callers, there is no capability or resource declaration that covers it, and the fn has an undeclared dependency on deployment configuration. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `env`. `obj.process.env` (member access on a local), `unsafe {}` blocks, and `unsafe "reason" fn` bodies are excluded. | Pass config and secrets as explicit fn parameters so the dependency is visible in the call signature; if env access is required at the load site, wrap in `unsafe "reads deployment env" { }`. | | SYN006 | (0.7+, warning) A fn body calls `process.exit()`, `process?.exit()`, or `process.exit?.()`. All forms terminate the entire host process — not just the fn, not just the bot. They produce no return value, bypass `Result` propagation, `throws {}`, `match`, and any caller recovery path. No capability declaration covers them. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `exit` then `(` or `?.(`. `obj.process.exit(...)`, `process.exit` without `(`, and `process.exitCode` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Return `err(...)` and let the caller decide whether to terminate. If `process.exit` is genuinely required at a bootstrap entry point, wrap in `unsafe "exits on invalid config" { process.exit(1) }`. | -| SYN007 | (0.7+, warning) A fn body calls `fetch(...)`, `fetch?.(...)`, or TypeScript instantiation form `fetch(...)`. The `fetch` global makes real HTTP requests but is invisible to CAP001: `uses { net }` is never triggered, and callers cannot see the network dependency in the fn header. Detection: `fetch` not preceded by `.`/`?.`, followed by `(`, `?.(`, or `(`. `obj.fetch(...)` (method call on a local) is excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Replace with `http.get(url)` or `http.post(url, body)` and add `uses { net }` to the fn header. If the raw `fetch` API is genuinely required (e.g. a thin adapter), wrap in `unsafe "wraps fetch directly" { fetch(...) }`. | +| SYN007 | (0.7+, warning) A fn body calls `fetch(...)`, `fetch?.(...)`, or TypeScript instantiation form `fetch(...)`. The `fetch` global makes real HTTP requests but is invisible to CAP001: `uses { net }` is never triggered, and callers cannot see the network dependency in the fn header. Detection: `fetch` not preceded by `.`/`?.`, followed by `(`, `?.(`, or `(`. `obj.fetch(...)` (method call on a local) is excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Replace with `http.get(url)` or `http.post(url, { body })` and add `uses { net }` to the fn header. If the raw `fetch` API is genuinely required (e.g. a thin adapter), wrap in `unsafe "wraps fetch directly" { fetch(...) }`. | | 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. | | INT003 | (0.7+) A fn declares `intent: "idempotent"` but also has `uses { random }` or `uses { time }`. Both capabilities produce different values on each call, making the function non-idempotent. Only `random` and `time` are flagged; other capabilities are not structurally flagged by this check (INT003 is a narrow heuristic, not a proof of idempotence). | Remove `random`/`time` from `uses {}`, or change the intent. | | INT004 | (0.7+) A fn declares `intent: "idempotent"` but its body directly references `random` or `time` without declaring them. Under-declaration variant of INT003 — fires when INT003 does not. | Remove the non-idempotent call from the body, or declare the capability and remove the idempotent intent. | diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index 6ea60d38..a75599c6 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -627,12 +627,12 @@ const E: Record = { "wrap in `unsafe \"wraps fetch directly\" { fetch(...) }`", rewrite: "// before — fetch is invisible to the capability model\n" + - "fn loadData(url: string) -> Result {\n" + + "async fn loadData(url: string) -> Promise> {\n" + " const res = await fetch(url) // SYN007\n" + " return ok(await res.text())\n" + "}\n\n" + "// after — http.get declares the net dependency\n" + - "fn loadData(url: string) uses { net } -> Result {\n" + + "async fn loadData(url: string) uses { net } -> Promise> {\n" + " match await http.get(url) {\n" + " ok { res } -> ok(await res.text())\n" + " err { e } -> err(e.message)\n" + @@ -640,12 +640,12 @@ const E: Record = { "}", example: "// SYN007: fetch bypasses the net capability model\n" + - "fn getData(url: string) -> string {\n" + + "async fn getData(url: string) -> Promise {\n" + " const res = await fetch(url) // SYN007\n" + " return await res.text()\n" + "}\n\n" + "// fix: use http.get and declare the capability\n" + - "fn getData(url: string) uses { net } -> Result {\n" + + "async fn getData(url: string) uses { net } -> Promise> {\n" + " match await http.get(url) {\n" + " ok { res } -> ok(await res.text())\n" + " err { e } -> err(e.message)\n" + diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index 7a0a2b72..6689cff9 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -24,6 +24,13 @@ * audit, or mock in tests. The idiomatic fix is to pass config * and secrets as explicit fn parameters. * + * SYN006 A `process.exit()` call was detected in a fn body (?bs 0.7+). + * `process.exit()` terminates the entire host process — not just the + * fn, not just the bot. It produces no return value and bypasses + * Result propagation, throws {}, match, and any caller recovery + * path. The idiomatic fix is `return err(...)` so the caller can + * decide whether to terminate. + * * SYN007 A `fetch(...)` call was detected in a fn body (?bs 0.7+). * `fetch()` makes real HTTP requests at runtime but is invisible to * botscript's capability model: CAP001 checks for `http.*` member @@ -445,8 +452,7 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const at = tokens[afterFetchIdx]; if (!at) { afterFetchIdx++; continue; } if (at.kind === "operator" && at.text === "<") { anglDepth++; } - else if (at.kind === "operator" && at.text === ">") { anglDepth--; } - else if (at.kind === "operator" && (at.text === ">>" || at.text === ">>>")) { + else if (at.kind === "operator" && (at.text === ">" || at.text === ">>" || at.text === ">>>")) { anglDepth -= at.text.length; } afterFetchIdx++; diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 8fbc09ee..4c11c7c2 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -840,13 +840,13 @@ export const EXPLANATIONS: Readonly> = { example: { fails: "?bs 0.7\n" + - "fn loadData(url: string) -> string {\n" + + "async fn loadData(url: string) -> Promise {\n" + " const res = await fetch(url)\n" + " return await res.text()\n" + "}\n", passes: "?bs 0.7\n" + - "fn loadData(url: string) uses { net } -> Result {\n" + + "async fn loadData(url: string) uses { net } -> Promise> {\n" + " match await http.get(url) {\n" + " ok { res } -> ok(await res.text())\n" + " err { e } -> err(e.message)\n" + From 1a4aaaa82222867782fd55599c111b850492eb4c Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 11 Jun 2026 03:31:50 -0300 Subject: [PATCH 04/11] fix(compiler): update outdated unsafe-fn skip comment to match all SYN checks --- packages/compiler/src/passes/syn-check.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index 6689cff9..10637a8a 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -86,7 +86,7 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const nesting = computeNesting(program.fns.map((f) => f.decl)); for (const { decl } of program.fns) { - // An `unsafe "reason" fn` body is an explicit acknowledgment — skip SYN002/SYN003/SYN005. + // An `unsafe "reason" fn` body is an explicit acknowledgment — all SYN checks are skipped. // The range-based suppression above also covers nested non-unsafe fns within it, // so this early-continue is kept purely as an optimisation. if (decl.unsafeReason !== undefined) continue; From d8646c35445b1d00eccfe90c8abddd8c20edabab Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 11 Jun 2026 11:31:57 -0300 Subject: [PATCH 05/11] fix(syn007): exclude object-literal method shorthands and type-literal signatures from SYN007 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the same guard SYN002 uses: if fetch(...) is followed by { / => / : after the closing paren it is a method definition, not a runtime call — skip to avoid false positives. Add two regression tests covering the object-method-shorthand and type-literal cases. Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/syn-check.ts | 19 ++++++++++++++++++ packages/compiler/tests/syn007-check.test.ts | 21 ++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index 10637a8a..f866ea1c 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -469,6 +469,25 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult continue; } + // Exclude object-literal method shorthands and type-literal method signatures: + // `{ fetch(url) { ... } }` and `{ fetch(url): RetType }` inside a fn body. + // Detection: if the `(` has a matching `)` and the token after `)` is `{`, `=>`, + // or `:`, this is a method definition/annotation, not a runtime call. + if (afterFetch.kind === "open" && afterFetch.text === "(") { + const closeParenIdx7 = afterFetch.matchedAt; + if (closeParenIdx7 !== undefined) { + const afterParenIdx7 = nextSignificant(tokens, closeParenIdx7 + 1); + const afterParen7 = tokens[afterParenIdx7]; + if ( + afterParen7 && + ((afterParen7.kind === "open" && afterParen7.text === "{") || + afterParen7.kind === "fatArrow" || + (afterParen7.kind === "punct" && afterParen7.text === ":")) + ) + continue; + } + } + // Suppression check: unsafe block or unsafe fn body if (isInsideRange(tok7.start, unsafeRanges)) continue; diff --git a/packages/compiler/tests/syn007-check.test.ts b/packages/compiler/tests/syn007-check.test.ts index 5471c149..2ec2327b 100644 --- a/packages/compiler/tests/syn007-check.test.ts +++ b/packages/compiler/tests/syn007-check.test.ts @@ -78,6 +78,27 @@ describe("SYN007: fetch() call detection (?bs 0.7+)", () => { expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); }); + it("does not fire on fetch() as an object-literal method shorthand inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn makeClient() -> any {\n" + + " return { fetch(url: string) { return url } }\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); + + it("does not fire on fetch() as a type-literal method signature inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn check(client: any) -> any {\n" + + " const typed: { fetch(url: string): string } = client\n" + + " return typed\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); + it("does not fire on .fetch() — method call on a local object", () => { const src = "?bs 0.7\n" + From 061cc883b0978ef9a0a0226f2f0b524c95e1ca22 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 11 Jun 2026 15:45:07 -0300 Subject: [PATCH 06/11] fix(syn007): apply method-shorthand exclusion uniformly across all call forms Previously, the object/type-literal method exclusion only ran when the token immediately after `fetch` was `(` (direct call). Generic (`fetch(...)`) and optional (`fetch?.(...)`) forms bypassed the exclusion, causing false positives on method signatures like `{ fetch(url): T }` inside fn bodies. Refactor to normalize `parenIdx7` to the actual `(` token index for all three call forms before running the exclusion check. Add two regression tests covering the generic method-signature and return-type-annotated method-shorthand cases. Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/syn-check.ts | 69 ++++++++++---------- packages/compiler/tests/syn007-check.test.ts | 21 ++++++ 2 files changed, 56 insertions(+), 34 deletions(-) diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index f866ea1c..6a0bdea3 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -440,52 +440,53 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult // Must be followed by `(`, `?.(`, or `(` — confirming this is a call, // not a bare `fetch` reference or a property access like `fetch.name`. - let afterFetchIdx = nextSignificant(tokens, i + 1); - const afterFetch = tokens[afterFetchIdx]; + // parenIdx7 is always normalized to the actual `(` token index. + const afterFetchFirstIdx = nextSignificant(tokens, i + 1); + const afterFetch = tokens[afterFetchFirstIdx]; if (!afterFetch) continue; - // TypeScript instantiation form: `fetch(...)` — skip over `<...>` to find `(` + let parenIdx7: number; + if (afterFetch.kind === "operator" && afterFetch.text === "<") { + // TypeScript instantiation form: `fetch(...)` — skip over `<...>` to find `(` let anglDepth = 1; - afterFetchIdx++; - while (afterFetchIdx < decl.tokenEnd && anglDepth > 0) { - const at = tokens[afterFetchIdx]; - if (!at) { afterFetchIdx++; continue; } - if (at.kind === "operator" && at.text === "<") { anglDepth++; } - else if (at.kind === "operator" && (at.text === ">" || at.text === ">>" || at.text === ">>>")) { + let j = afterFetchFirstIdx + 1; + while (j < decl.tokenEnd && anglDepth > 0) { + const at = tokens[j]; + if (!at) { j++; continue; } + if (at.kind === "operator" && at.text === "<") anglDepth++; + else if (at.kind === "operator" && (at.text === ">" || at.text === ">>" || at.text === ">>>")) anglDepth -= at.text.length; - } - afterFetchIdx++; + j++; } - afterFetchIdx = nextSignificant(tokens, afterFetchIdx); - const afterAngle = tokens[afterFetchIdx]; - if (!afterAngle || !(afterAngle.kind === "open" && afterAngle.text === "(")) continue; + parenIdx7 = nextSignificant(tokens, j); } else if (afterFetch.kind === "questionDot") { // `fetch?.(...)` — optional call - const afterQD7 = nextSignificant(tokens, afterFetchIdx + 1); - const afterQDTok = tokens[afterQD7]; - if (!afterQDTok || !(afterQDTok.kind === "open" && afterQDTok.text === "(")) continue; - } else if (!(afterFetch.kind === "open" && afterFetch.text === "(")) { + parenIdx7 = nextSignificant(tokens, afterFetchFirstIdx + 1); + } else if (afterFetch.kind === "open" && afterFetch.text === "(") { + // Direct call: `fetch(...)` + parenIdx7 = afterFetchFirstIdx; + } else { continue; } + const parenTok7 = tokens[parenIdx7]; + if (!parenTok7 || !(parenTok7.kind === "open" && parenTok7.text === "(")) continue; + // Exclude object-literal method shorthands and type-literal method signatures: - // `{ fetch(url) { ... } }` and `{ fetch(url): RetType }` inside a fn body. - // Detection: if the `(` has a matching `)` and the token after `)` is `{`, `=>`, - // or `:`, this is a method definition/annotation, not a runtime call. - if (afterFetch.kind === "open" && afterFetch.text === "(") { - const closeParenIdx7 = afterFetch.matchedAt; - if (closeParenIdx7 !== undefined) { - const afterParenIdx7 = nextSignificant(tokens, closeParenIdx7 + 1); - const afterParen7 = tokens[afterParenIdx7]; - if ( - afterParen7 && - ((afterParen7.kind === "open" && afterParen7.text === "{") || - afterParen7.kind === "fatArrow" || - (afterParen7.kind === "punct" && afterParen7.text === ":")) - ) - continue; - } + // `{ fetch(url) { ... } }` and `{ fetch(url): RetType }` — applies to all call forms + // including generic (`fetch(...)`) and optional (`fetch?.(...)`) since parenTok7 + // is always the actual `(` token. + const closeParenIdx7 = parenTok7.matchedAt; + if (closeParenIdx7 !== undefined) { + const afterParenIdx7 = nextSignificant(tokens, closeParenIdx7 + 1); + const afterParen7 = tokens[afterParenIdx7]; + if ( + afterParen7 && + ((afterParen7.kind === "open" && afterParen7.text === "{") || + afterParen7.kind === "fatArrow" || + (afterParen7.kind === "punct" && afterParen7.text === ":")) + ) continue; } // Suppression check: unsafe block or unsafe fn body diff --git a/packages/compiler/tests/syn007-check.test.ts b/packages/compiler/tests/syn007-check.test.ts index 2ec2327b..b949b179 100644 --- a/packages/compiler/tests/syn007-check.test.ts +++ b/packages/compiler/tests/syn007-check.test.ts @@ -140,4 +140,25 @@ describe("SYN007: fetch() call detection (?bs 0.7+)", () => { expect(w).toBeDefined(); expect(w?.severity).toBe("warning"); }); + + it("does not fire on fetch() as a generic type-literal method signature", () => { + const src = + "?bs 0.7\n" + + "fn check(client: any) -> any {\n" + + " const typed: { fetch(url: string): T } = client\n" + + " return typed\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); + + it("does not fire on fetch?.() as an optional method shorthand inside an object", () => { + const src = + "?bs 0.7\n" + + "fn makeClient() -> any {\n" + + " return { fetch(url: string): string { return url } }\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); }); From 806bc9477e61c6975391e7de1ce209c099095119 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 11 Jun 2026 19:43:36 -0300 Subject: [PATCH 07/11] fix(compiler): address SYN007 Copilot review feedback Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/syn-check.ts | 30 +++++++++----------- packages/compiler/tests/syn007-check.test.ts | 24 +++++++++++++++- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index 6a0bdea3..a9b2d576 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -440,17 +440,17 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult // Must be followed by `(`, `?.(`, or `(` — confirming this is a call, // not a bare `fetch` reference or a property access like `fetch.name`. - // parenIdx7 is always normalized to the actual `(` token index. - const afterFetchFirstIdx = nextSignificant(tokens, i + 1); - const afterFetch = tokens[afterFetchFirstIdx]; + // afterFetchIdx is advanced past `?.` for optional-call forms so it always + // points at the effective `(` token; the object-literal/type-literal + // exclusion below relies on this invariant. + let afterFetchIdx = nextSignificant(tokens, i + 1); + const afterFetch = tokens[afterFetchIdx]; if (!afterFetch) continue; - let parenIdx7: number; - if (afterFetch.kind === "operator" && afterFetch.text === "<") { // TypeScript instantiation form: `fetch(...)` — skip over `<...>` to find `(` let anglDepth = 1; - let j = afterFetchFirstIdx + 1; + let j = afterFetchIdx + 1; while (j < decl.tokenEnd && anglDepth > 0) { const at = tokens[j]; if (!at) { j++; continue; } @@ -459,24 +459,22 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult anglDepth -= at.text.length; j++; } - parenIdx7 = nextSignificant(tokens, j); + afterFetchIdx = nextSignificant(tokens, j); } else if (afterFetch.kind === "questionDot") { - // `fetch?.(...)` — optional call - parenIdx7 = nextSignificant(tokens, afterFetchFirstIdx + 1); - } else if (afterFetch.kind === "open" && afterFetch.text === "(") { - // Direct call: `fetch(...)` - parenIdx7 = afterFetchFirstIdx; - } else { + // `fetch?.(...)` — optional call: advance past `?.` so afterFetchIdx points at `(` + afterFetchIdx = nextSignificant(tokens, afterFetchIdx + 1); + } else if (!(afterFetch.kind === "open" && afterFetch.text === "(")) { + // Not a call form we recognise — bare reference or property access continue; } - const parenTok7 = tokens[parenIdx7]; + const parenTok7 = tokens[afterFetchIdx]; if (!parenTok7 || !(parenTok7.kind === "open" && parenTok7.text === "(")) continue; // Exclude object-literal method shorthands and type-literal method signatures: // `{ fetch(url) { ... } }` and `{ fetch(url): RetType }` — applies to all call forms - // including generic (`fetch(...)`) and optional (`fetch?.(...)`) since parenTok7 - // is always the actual `(` token. + // including generic (`fetch(...)`) and optional (`fetch?.(...)`) since + // tokens[afterFetchIdx] is always the actual `(` token. const closeParenIdx7 = parenTok7.matchedAt; if (closeParenIdx7 !== undefined) { const afterParenIdx7 = nextSignificant(tokens, closeParenIdx7 + 1); diff --git a/packages/compiler/tests/syn007-check.test.ts b/packages/compiler/tests/syn007-check.test.ts index b949b179..ea5d73e1 100644 --- a/packages/compiler/tests/syn007-check.test.ts +++ b/packages/compiler/tests/syn007-check.test.ts @@ -152,7 +152,7 @@ describe("SYN007: fetch() call detection (?bs 0.7+)", () => { expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); }); - it("does not fire on fetch?.() as an optional method shorthand inside an object", () => { + it("does not fire on fetch() as an object-literal method with return-type annotation inside a fn body", () => { const src = "?bs 0.7\n" + "fn makeClient() -> any {\n" + @@ -161,4 +161,26 @@ describe("SYN007: fetch() call detection (?bs 0.7+)", () => { const result = transform(src); expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); }); + + it("does not fire on a type-literal member signature { fetch(url: string): string } inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn check(client: any) -> any {\n" + + " const adapter: { fetch(url: string): string } = client\n" + + " return adapter\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); + + it("does not fire on a generic type-literal method signature { fetch(url: string): string } inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn check(client: any) -> any {\n" + + " const adapter: { fetch(url: string): string } = client\n" + + " return adapter\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN007")).toBe(false); + }); }); From c3eb588b01eac5c69e893e904f88d4966434b300 Mon Sep 17 00:00:00 2001 From: Marcelo Bukowski de Farias Date: Thu, 11 Jun 2026 23:30:13 -0300 Subject: [PATCH 08/11] =?UTF-8?q?feat(compiler):=20DEP003/DEP004=20?= =?UTF-8?q?=E2=80=94=20warn=20when=20reads/writes=20labels=20are=20over-de?= =?UTF-8?q?clared=20(=3Fbs=200.9+)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(compiler): DEP003/DEP004 — warn when reads/writes labels are over-declared (?bs 0.9+) Squash-rebased onto current main. DEP003 fires when a fn declares reads { x } but no same-file callee (transitively) reads { x } — the annotation likely became stale after a refactor. DEP004 is the symmetric check for writes { x }. Both are warning-level (non-blocking). Leaf fns are excluded. Adds DEP003/DEP004 to compiler registry, MCP explain, and tests. Supersedes PR #100 (same content, rebased). All 996 tests pass. Co-Authored-By: Claude Sonnet 4.6 * fix(dep003): hasOpaqueCall now follows full member chain — detects obj.a.b() and obj.a?.b() Previous logic only checked one segment deep: obj.method() was detected but obj.a.b() was not, because afterMethod was `.` rather than `(`. Walk the full chain until a call or non-chain token is found. Adds two regression tests for the multi-segment cases. Co-Authored-By: Claude Sonnet 4.6 * fix(dep003): address Copilot review — arrow parens and conditional wording - Add parens around single-param arrow functions in dep-check.ts (style consistency) - Clarify DEP001/DEP002 cross-refs: "may be warned about" with scope conditions, not unconditional Co-Authored-By: Claude Sonnet 4.6 * fix(dep003): add DEP003/DEP004 to exhaustive error-codes test allowlist Co-Authored-By: Claude Sonnet 4.6 * fix(compiler): derive STDLIB_NAMESPACES from STDLIB_TO_CAP to eliminate drift risk STDLIB_NAMESPACES was a hardcoded duplicate of the key set of STDLIB_TO_CAP (_stdlib.ts). Adding or removing a stdlib namespace required updating two places and hasOpaqueCall behavior could silently diverge. Now derived from the single source of truth. Co-Authored-By: Claude Sonnet 4.6 * fix(compiler): collectTopLevelParamNames handles untyped/default params, collectFnBodyLocalNames handles var collectTopLevelParamNames: - Adds skipUntilNextParam flag so type annotations and default values after the param name are not re-scanned as candidate param names. Fixes false captures like treating `string` in `(name: string)` as a param name when a untyped-param lookahead would have matched it. - Adds fallback for completely untyped and default params: bare identifier immediately before `,`, `)`, or `=` is collected as a param name. Fixes the original Copilot gap: `fn f(x) -> void` left `x` unknown, causing `x.trim()` to be treated as an opaque external call. - Closing `}` at depth 1 sets skipUntilNextParam so the type annotation after a destructured binding (`{a, b}: Context`) is not captured. collectFnBodyLocalNames: - Adds `var` alongside `const`/`let` so that `var x = ...` bindings are recognised as local names and `x.method()` is not mistaken for an opaque namespace call. Co-Authored-By: Claude Sonnet 4.6 * test(dep-check): add coverage for untyped param and var local opaque suppression Two regression tests to cover the fixes in collectTopLevelParamNames and collectFnBodyLocalNames: - Untyped param `fn f(name)`: name.trim() must not suppress DEP003 - Var binding `var x = ...`: x.method() must not suppress DEP003 Co-Authored-By: Claude Sonnet 4.6 * fix(compiler): DEP003/DEP004 rule/idiom text and docs reflect moduleEffects and opaque-call suppression Update DEP003/DEP004 entries in error-codes.ts (rule, idiom), MCP explanations, and AGENTS.md to accurately describe the two conditions under which the warning is suppressed: fns with opaque/untracked external calls, and tracked callees that include moduleEffects entries (not just same-file fns). Co-Authored-By: Claude Sonnet 4.6 * fix(dep003): correct doc comment — collectFnBodyLocalNames also collects var The implementation collects `const`, `let`, and `var` bindings, but the doc comment only mentioned `const`/`let`. Updated to match. Co-Authored-By: Claude Sonnet 4.6 * fix(callgraph): collect destructured binding names in params and local vars collectTopLevelParamNames: when a parameter is a destructuring pattern ({ a, b: c } or [x]), extract the binding names (those NOT followed by `:`) so method calls on destructured locals (e.g. name.trim()) are not mistaken for opaque external calls that suppress DEP003/DEP004/THR004. collectFnBodyLocalNames: add collectDestructuredTokenBindings helper that uses the token matchedAt index to scan { } and [ ] destructuring patterns recursively, collecting binding names by the same rule (ident not followed by `:` is a binding, not a property key). Adds two regression tests: one for destructured params and one for destructured const locals — both confirm DEP003 still fires when the only member calls in the body are on locally-scoped destructured names. Co-Authored-By: Claude Sonnet 4.6 * fix(callgraph): skip bracket groups in type annotations; fix destructuring depth collectTopLevelParamNames: when skipping past a param's type annotation (skipUntilNextParam=true), skip entire {}/[] groups rather than char-by-char so commas inside `x: { a: string, b: number }` are not mistaken for param separators. collectDestructuringStringBindings: change depth guard from `depth !== 1` to `depth < 1` so nested destructuring patterns like `{ a: { b } }` are handled — binding names at depth > 1 are now correctly collected. Co-Authored-By: Claude Sonnet 4.6 * fix(dep003): skip default-value expressions in destructuring binding collectors Identifiers in `= ` default values (e.g. `{ a = externalDb }`) were incorrectly collected as binding names, causing hasOpaqueCall() to treat member calls on those names as local and suppress DEP003/DEP004/THR004 when it shouldn't. Co-Authored-By: Claude Sonnet 4.6 * fix(dep003): clarify DEP003/DEP004 titles — tracked callee, not same-file only Copilot pointed out the titles said "in the same file" but the implementation also counts moduleEffects entries as tracked callees. Updated titles in error-codes.ts and explanations.ts to say "any tracked callee" instead. Co-Authored-By: Claude Sonnet 4.6 * fix(dep003): exclude module-level stdlib aliases from opaque-call detection hasOpaqueCall suppresses DEP003/DEP004 when a fn contains a member call whose receiver is not in STDLIB_NAMESPACES or effectiveLocalNames. Module-level stdlib aliases like `const t = time` were not included in either set, causing `t.now()` to be treated as an opaque external call and incorrectly suppressing the warning. Fix: collect module-level stdlib alias names via collectStdlibAliases() once per pass invocation and add them to localNames at each hasOpaqueCall call site. Co-Authored-By: Claude Sonnet 4.6 * fix(callgraph): move orphaned JSDoc to collectTopLevelParamNames The JSDoc block for collectTopLevelParamNames was placed before collectDestructuringStringBindings, leaving both functions without proper documentation. Moved the doc block to attach to the correct symbol. Co-Authored-By: Claude Sonnet 4.6 * fix(thr-check): pass stdlib aliases to hasOpaqueCall for THR004 suppression hasOpaqueCall in passThrCheck was called without localNames, so module-level stdlib aliases (e.g. `const t = time`) were treated as opaque external member calls and incorrectly suppressed THR004 warnings. passDepCheck already passed stdlibAliasNames correctly; align passThrCheck to match. - Import collectStdlibAliases from _alias.js and collectFnBodyLocalNames from _callgraph.js in thr-check.ts - Compute stdlibAliasNames once at passThrCheck entry - Build localNames = paramNames + bodyLocals + stdlibAliasNames for the hasOpaqueCall call (mirrors the pattern in passDepCheck) - Add regression test: fn with t.now() (stdlib alias) still fires THR004 Co-Authored-By: Claude Sonnet 4.6 * fix(callgraph): track angle-bracket depth in collectTopLevelParamNames Commas inside generic type argument lists like `Map` were incorrectly resetting skipUntilNextParam, causing the `{ a }` fragment to be parsed as a destructuring parameter and adding `a` to the param-names set. This made `a.method()` in the body appear local rather than opaque, falsely suppressing DEP003/DEP004 when they should fire (and vice versa — allowing opaque-call suppression to be defeated). Fix: track `<`/`>` angle depth in skipUntilNextParam mode. Commas are only treated as top-level param separators when angleDepth === 0. Co-Authored-By: Claude Sonnet 4.6 * fix(callgraph): remove unused chainWalk: label in hasOpaqueCall The label had no corresponding `break chainWalk` or `continue chainWalk` usage; all break/continue statements in the loop are bare. Removing it eliminates the noise and avoids potential linter warnings. Co-Authored-By: Claude Sonnet 4.6 * fix(thr-check): filter import aliases to tracked moduleEffects targets for opaque-call check THR004 opaque-call suppression used allCalleeNames, which includes ALL import aliases. An alias to an untracked external (not in moduleEffects) is itself an opaque call — including it in knownForOpaque caused THR004 to fire for fns with genuinely unknown external dependencies (false warnings). Fix: derive opaqueKnownBase from fnNames + knownExternalNames + only the aliases whose resolved target exists in moduleEffects. allCalleeNames is preserved as-is for collectCallees (call graph building). Co-Authored-By: Claude Sonnet 4.6 * fix(dep003/dep004): correct rule/idiom text — call-graph heuristic, not body scanner The previous wording implied the compiler scans fn bodies for direct resource access, which it does not. Reword rule and idiom to accurately describe the implementation: a call-graph justification check that fires only when all same-file callees are resolvable and none propagates the declared label. --------- Co-authored-by: Marcelo Farias Co-authored-by: Claude Sonnet 4.6 --- AGENTS.md | 2 + README.md | 2 +- packages/compiler/src/error-codes.ts | 48 ++ packages/compiler/src/module-effects.ts | 11 +- packages/compiler/src/passes/_callgraph.ts | 442 ++++++++++++-- packages/compiler/src/passes/dep-check.ts | 189 +++++- packages/compiler/src/passes/thr-check.ts | 32 +- packages/compiler/tests/dep-check.test.ts | 546 ++++++++++++++++++ packages/compiler/tests/error-codes.test.ts | 2 +- .../compiler/tests/module-effects.test.ts | 51 +- packages/compiler/tests/thr-check.test.ts | 17 + packages/mcp/src/explanations.ts | 61 +- packages/mcp/tests/server.test.ts | 2 + 13 files changed, 1325 insertions(+), 80 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6bb47fdb..4663a03a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -213,6 +213,8 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | EFF004 | (0.9+) A callback parameter declares `writes { … }` labels not covered by the outer fn's `writes {}`. | Add the missing label(s) to the outer fn's `writes {}`, or narrow the callback annotation. | | DEP001 | (0.9+) A fn's body (or a callee in the same file) reads a resource label not declared in the fn's own `reads {}`. Transitivity is enforced: if `loadUser` calls `fetchRow` which reads `userDb`, `loadUser` must also declare `reads { userDb }`. | Add the missing label(s) to `reads {}`, or remove the undeclared read. | | DEP002 | (0.9+) Same as DEP001 but for `writes {}` labels. A fn whose callee writes a resource must declare that write in its own header. | Add the missing label(s) to `writes {}`, or remove the undeclared write. | +| DEP003 | (0.9+, warning) A fn declares `reads { x }` but no tracked callee (same-file or `moduleEffects` entry, direct or transitive) also declares `reads { x }`. Suppressed when the fn body contains any opaque/untracked external call (the label may still be live cross-module). Leaf fns are excluded. Non-blocking. | Remove the stale label from `reads {}`, or verify the fn is the intended access point. | +| DEP004 | (0.9+, warning) Same as DEP003 but for `writes {}`. A fn declares a write label that no tracked callee (same-file or `moduleEffects` entry) justifies; suppressed on fns with opaque external calls. Non-blocking. | Remove the stale label from `writes {}`, or verify the fn is the intended access point. | | THR001 | (0.9+) A fn's body (or a same-file callee) throws an exception type not declared in the fn's `throws {}`. Transitivity is enforced: if `loadUser` calls `fetchRow throws { NetworkError }`, `loadUser` must also declare `throws { NetworkError }`. | Add the missing type(s) to `throws {}`, or add a `match` / `unsafe` to suppress the propagation. | | THR002 | (0.9+) A fn body directly constructs `err(TypeName(...))`, `err(new TypeName(...))`, or `err(TypeName)` where `TypeName` (CapCase) is not declared in the fn's own `throws {}` clause. Producer-side complement to THR001. | Add `TypeName` to the fn's `throws {}`, or change the error construction to use a declared type. | | THR003 | (0.9+) A callback parameter declares `throws { … }` types not covered by the outer fn's `throws {}`. Same structural rule as THR001 applied to callback parameters. | Add the missing type(s) to the outer fn's `throws {}`, or narrow the callback annotation. | diff --git a/README.md b/README.md index 1bf8d55c..b1cbc6ec 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,7 @@ claude mcp add botscript -- npx -y @mbfarias/botscript-mcp | ----------- | -------------------------------------- | --------------------------------------------------------------------------------------------------- | | `primer` | (no args) | The canonical language primer (same text the `?primer` directive emits). | | `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 (`ALI001`, `ALI002`, `ALI003`, `BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`, `DEP002`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`–`MAT004`, `RES001`, `RES002`, `SYN001`, `SYN002`, `SYN003`, `SYN005`, `SYN006`, `THR001`–`THR004`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. | +| `explain` | `{ code: string }` | Long-form explanation for any stable diagnostic code (`ALI001`, `ALI002`, `ALI003`, `BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`–`DEP004`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`–`MAT004`, `RES001`, `RES002`, `SYN001`, `SYN002`, `SYN003`, `SYN005`, `SYN006`, `THR001`–`THR004`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. | A bot's loop becomes deterministic: `transform` → if `ok=false`, read `diagnostics[0].code` → `explain(code)` → apply `rewrite` → `transform` again. diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index aa8aa867..d331091d 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -669,6 +669,54 @@ const E: Record = { "fn helper(id: string) -> string = \"ok\"\n" + "fn load(id: string) -> string = helper(id)", }, + DEP003: { + code: "DEP003", + title: "fn declares reads {} label not justified by any tracked callee (warning)", + rule: + "a declared reads {} label must be justified by at least one tracked callee declaring the same label; " + + "DEP003 fires when the pass can resolve all same-file callees and none of them (nor any moduleEffects entry) " + + "transitively declares reads { x }; the pass does not scan fn bodies for direct resource access — " + + "it is a call-graph heuristic, not a body scanner; suppressed when the fn has any opaque/untracked external call", + idiom: + "remove the stale label from the reads {} clause when no tracked callee propagates it; " + + "if the label is live through a cross-module call, the opaque-call suppression prevents a false positive; " + + "leaf fns and fns with opaque external calls are excluded — the warning only fires when the pass can fully resolve the call graph", + rewrite: + "fn name(...) reads { …remaining } -> ... // remove label not propagated by any callee", + example: + "// before — getUser calls helper() but helper() does not read userDb\n" + + "?bs 0.9\n" + + "fn helper(id: string) -> string = \"Alice\"\n" + + "fn getUser(id: string) reads { userDb } -> string { helper(id) } // DEP003\n\n" + + "// after — remove stale label\n" + + "?bs 0.9\n" + + "fn helper(id: string) -> string = \"Alice\"\n" + + "fn getUser(id: string) -> string { helper(id) }", + }, + DEP004: { + code: "DEP004", + title: "fn declares writes {} label not justified by any tracked callee (warning)", + rule: + "a declared writes {} label must be justified by at least one tracked callee declaring the same label; " + + "DEP004 fires when the pass can resolve all same-file callees and none of them (nor any moduleEffects entry) " + + "transitively declares writes { x }; the pass does not scan fn bodies for direct resource access — " + + "it is a call-graph heuristic, not a body scanner; suppressed when the fn has any opaque/untracked external call", + idiom: + "remove the stale label from the writes {} clause when no tracked callee propagates it; " + + "if the label is live through a cross-module call, the opaque-call suppression prevents a false positive; " + + "leaf fns and fns with opaque external calls are excluded — the warning only fires when the pass can fully resolve the call graph", + rewrite: + "fn name(...) writes { …remaining } -> ... // remove label not propagated by any callee", + example: + "// before — logEvent calls save() but save() does not write auditLog\n" + + "?bs 0.9\n" + + "fn save(msg: string) -> void { }\n" + + "fn logEvent(msg: string) writes { auditLog } -> void { save(msg) } // DEP004\n\n" + + "// after — remove stale label\n" + + "?bs 0.9\n" + + "fn save(msg: string) -> void { }\n" + + "fn logEvent(msg: string) -> void { save(msg) }", + }, THR003: { code: "THR003", title: "outer fn declares narrower throws than a callback parameter", diff --git a/packages/compiler/src/module-effects.ts b/packages/compiler/src/module-effects.ts index f541b2fd..4dadf07c 100644 --- a/packages/compiler/src/module-effects.ts +++ b/packages/compiler/src/module-effects.ts @@ -375,11 +375,12 @@ export function buildModuleEffects(sources: readonly string[]): ModuleEffects { if (decl.reads?.length) surface.reads = decl.reads; if (decl.writes?.length) surface.writes = decl.writes; if (decl.throws?.length) surface.throws = decl.throws; - if (Object.keys(surface).length > 0) { - effects[decl.name] = Object.hasOwn(effects, decl.name) - ? mergeEffectSurface(effects[decl.name]!, surface) - : surface; - } + // Always include the function, even when surface is empty {}. + // DEP003/DEP004 need to distinguish "known callee with no labels" from + // "unknown callee" — omitting pure helpers causes them to look opaque. + effects[decl.name] = Object.hasOwn(effects, decl.name) + ? mergeEffectSurface(effects[decl.name]!, surface) + : surface; } } return effects; diff --git a/packages/compiler/src/passes/_callgraph.ts b/packages/compiler/src/passes/_callgraph.ts index f38da2e6..e2cc4c05 100644 --- a/packages/compiler/src/passes/_callgraph.ts +++ b/packages/compiler/src/passes/_callgraph.ts @@ -9,6 +9,7 @@ import type { Token } from "../parser/lex.js"; import type { FnDecl } from "../parser/parse-fn.js"; import { STDLIB_VALUE_CALL_NAMES } from "./imports.js"; +import { STDLIB_TO_CAP } from "./_stdlib.js"; /** * Build a map from each FnDecl to all nested (direct and indirect) FnDecls @@ -103,64 +104,296 @@ function isDirectOrOptionalCall(tokens: Token[], identIdx: number): boolean { return false; } -export 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" - ) { +/** + * Botscript stdlib namespace names. Member calls on these (e.g. `time.now()`, + * `http.get()`) are handled by cap-check/uns-check, not by `hasOpaqueCall`. + * + * Derived from `STDLIB_TO_CAP` in `_stdlib.ts` — the single source of truth. + */ +export const STDLIB_NAMESPACES: ReadonlySet = new Set(Object.keys(STDLIB_TO_CAP)); + +/** + * Botscript's lexer only promotes a small set of names to `keyword` tokens + * (see `KEYWORDS` in lex.ts). Most control-flow constructs (`if`, `while`, + * `for`, etc.) are tokenised as plain `ident` and would otherwise be treated + * as opaque function calls by `hasOpaqueCall`. Skip them explicitly. + */ +const CONTROL_FLOW_IDENTS = new Set([ + "if", "else", "while", "for", "do", "switch", "case", "default", + "return", "break", "continue", "throw", "try", "catch", "finally", + "typeof", "void", "delete", "new", "in", "of", "instanceof", +]); + + +/** + * Extract binding names from a destructuring pattern substring (`{ a, b: c }` + * or `[ x, y ]`). An identifier is a binding name if it is NOT immediately + * followed by `:` (which would mark it as a property key in object destructuring). + * Recurses into nested patterns. + */ +function collectDestructuringStringBindings(pattern: string): string[] { + const names: string[] = []; + let i = 0; + let depth = 0; + while (i < pattern.length) { + const c = pattern[i]!; + if (c === "{" || c === "[" || c === "(") { depth++; i++; continue; } + if (c === "}" || c === "]" || c === ")") { depth--; i++; continue; } + if (depth < 1) { i++; continue; } + const m = /^([a-zA-Z_$][a-zA-Z0-9_$]*)/.exec(pattern.slice(i)); + if (m) { + const name = m[1]!; + const after = pattern.slice(i + m[0].length).trimStart(); + // Not a property key if not followed by `:` + if (!after.startsWith(":")) { + names.push(name); + // Skip default value expression to avoid collecting its identifiers as bindings. + // e.g. `{ a = externalDb }` should bind only `a`, not `externalDb`. + if (after.startsWith("=")) { + i += m[0].length; // advance past ident + while (i < pattern.length && (pattern[i] === " " || pattern[i] === "\t")) i++; + i++; // skip `=` + const skipDepth = depth; + while (i < pattern.length) { + const sc = pattern[i]!; + if (sc === "{" || sc === "[" || sc === "(") { depth++; i++; continue; } + if (sc === "}" || sc === "]" || sc === ")") { + if (depth === skipDepth) break; // closing bracket; outer loop handles it + depth--; i++; continue; + } + if (sc === "," && depth === skipDepth) break; + i++; + } + continue; + } + } + i += m[0].length; + continue; + } + i++; + } + return names; +} + +/** + * Parse the top-level parameter names from a fn's `args` string (verbatim, + * including outer parens). Depth-tracks parentheses so names inside nested + * callback type annotations (e.g. `cb: (item: string) -> void`) are excluded. + * + * Used by `hasOpaqueCall` (and exported for callers that also need param names) + * to avoid treating method calls on fn parameters as opaque namespace calls. + */ +export function collectTopLevelParamNames(args: string): Set { + const names = new Set(); + let parenDepth = 0; + // Tracks < > nesting inside type annotations (skipUntilNextParam mode) so + // commas in generic types like `Map` or `Result` are not + // mistaken for top-level parameter separators. + let angleDepth = 0; + let i = 0; + // True once we have consumed the param name for the current segment — skip + // everything until the next top-level comma (type annotation, default value, + // or destructured-binding type). + let skipUntilNextParam = false; + while (i < args.length) { + const c = args[i]!; + if (c === "(") { parenDepth++; i++; continue; } + if (c === ")") { parenDepth--; i++; continue; } + if (parenDepth !== 1) { i++; continue; } + // Only reset on a comma that is not inside a generic type argument list. + if (c === ",") { if (angleDepth === 0) skipUntilNextParam = false; i++; continue; } + if (skipUntilNextParam) { + // Track `<...>` angle-bracket groups so commas inside generic type args + // (e.g. `x: Map<{ a: string }, T>`) do not end the skip prematurely. + if (c === "<") { angleDepth++; i++; continue; } + if (c === ">") { if (angleDepth > 0) angleDepth--; i++; continue; } + if (angleDepth > 0) { i++; continue; } + // Skip entire `{...}` and `[...]` groups so commas inside type annotations + // like `x: { a: string, b: number }` are not mistaken for param separators. + if (c === "{" || c === "[") { + const closeChar = c === "{" ? "}" : "]"; + let depth = 1; + i++; + while (i < args.length && depth > 0) { + if (args[i] === c) depth++; + else if (args[i] === closeChar) depth--; + i++; + } + continue; + } i++; continue; } - return i; + + // Destructuring parameter: `{ a, b: c }` or `[ x ]` + if ((c === "{" || c === "[") && !skipUntilNextParam) { + // Find the matching close bracket by tracking depth + const openChar = c; + const closeChar = c === "{" ? "}" : "]"; + let depth = 0; + let j = i; + while (j < args.length) { + if (args[j] === openChar) depth++; + else if (args[j] === closeChar) { depth--; if (depth === 0) { j++; break; } } + j++; + } + const pattern = args.slice(i, j); + for (const name of collectDestructuringStringBindings(pattern)) names.add(name); + skipUntilNextParam = true; + i = j; + continue; + } + + // Try typed or optional-typed: `name:` or `name?:` + const typedM = /^([a-zA-Z_$][a-zA-Z0-9_$]*)\s*\??\s*:/.exec(args.slice(i)); + if (typedM) { + names.add(typedM[1]!); + skipUntilNextParam = true; + i += typedM[0].length; + continue; + } + // Try untyped or default param: bare identifier before `,`, `)`, or `=` + const untypedM = /^([a-zA-Z_$][a-zA-Z0-9_$]*)(?=\s*[,)=])/.exec(args.slice(i)); + if (untypedM) { + names.add(untypedM[1]!); + skipUntilNextParam = true; + i += untypedM[0].length; + continue; + } + i++; } - return i; + return names; } -export function prevSignificant(tokens: Token[], start: number): number { - let i = start; - while (i >= 0) { - const t = tokens[i]; - if (!t) return i; - if ( - t.kind === "whitespace" || - t.kind === "newline" || - t.kind === "lineComment" || - t.kind === "blockComment" - ) { - i--; +/** + * Collect all binding names from a destructuring pattern token range + * `[openIdx, closeIdx]` (inclusive). Works for both object (`{}`) and array + * (`[]`) destructuring, and recurses into nested patterns. + * + * Rule: an identifier is a binding name unless it is immediately followed by + * `:` at the same depth — that form marks a property key, not a binding. + */ +function collectDestructuredTokenBindings( + tokens: Token[], + openIdx: number, + names: Set, +): void { + const openTok = tokens[openIdx]; + if (!openTok || openTok.matchedAt === undefined) return; + const closeIdx = openTok.matchedAt; + for (let j = openIdx + 1; j < closeIdx; j++) { + const t = tokens[j]; + if (!t) continue; + // Recurse into nested destructuring patterns + if (t.kind === "open" && (t.text === "{" || t.text === "[")) { + collectDestructuredTokenBindings(tokens, j, names); + if (t.matchedAt !== undefined) j = t.matchedAt; continue; } - return i; + if (t.kind !== "ident") continue; + // Property key in object destructuring: `key: binding` — `key` is followed by `:` + const nextIdx = nextSignificant(tokens, j + 1); + const nextTok = tokens[nextIdx]; + const isPropertyKey = nextTok && nextTok.kind === "punct" && nextTok.text === ":"; + if (!isPropertyKey) { + names.add(t.text); + // Skip default value expression to avoid collecting its identifiers as bindings. + // e.g. `{ a = externalDb }` should bind only `a`, not `externalDb`. + if (nextTok && nextTok.kind === "eq") { + j = nextIdx + 1; // advance past `=` + let skipDepth = 0; + while (j < closeIdx) { + const st = tokens[j]; + if (!st) { j++; continue; } + if (st.kind === "open") { skipDepth++; j++; continue; } + if (st.kind === "close") { skipDepth--; j++; continue; } + if (skipDepth === 0 && st.kind === "punct" && st.text === ",") break; + j++; + } + j--; // outer for-loop will j++ to resume at `,` or closeIdx + } + } } - return i; } -// Alias imported for opaque-call detection; derived from imports.ts to avoid drift. -const BOTSCRIPT_BUILTIN_CALLS = STDLIB_VALUE_CALL_NAMES; +/** + * Collect names of `const`/`let`/`var` variables declared in `fn`'s body + * (excluding tokens inside nested fn declarations). + * + * Handles both simple bindings (`const name = ...`) and destructuring patterns + * (`const { a, b: c } = ...`, `const [x] = ...`). The result is used as the + * `localNames` set for `hasOpaqueCall` so that method calls on local variables + * (e.g. `name.trim()`, `x.method()`) are not mistaken for opaque import calls. + */ +export function collectFnBodyLocalNames( + tokens: Token[], + fn: FnDecl, + inner: FnDecl[], +): Set { + const names = new Set(); + const open: FnDecl[] = []; + let nextInner = 0; + const start = fn.bodyTokenStart ?? fn.tokenStart; + + for (let i = start; i < fn.tokenEnd; i++) { + while (open.length > 0 && open[open.length - 1]!.tokenEnd <= i) open.pop(); + while (nextInner < inner.length && inner[nextInner]!.tokenStart <= i) { + open.push(inner[nextInner]!); + nextInner++; + } + if (open.length > 0) continue; + + const tok = tokens[i]; + if (!tok || tok.kind !== "ident") continue; + if (tok.text !== "const" && tok.text !== "let" && tok.text !== "var") continue; + + const nameIdx = nextSignificant(tokens, i + 1); + const nameTok = tokens[nameIdx]; + if (!nameTok) continue; + if (nameTok.kind === "ident") { + names.add(nameTok.text); + } else if (nameTok.kind === "open" && (nameTok.text === "{" || nameTok.text === "[")) { + collectDestructuredTokenBindings(tokens, nameIdx, names); + } + } + + return names; +} /** - * Returns true if `fn`'s body contains at least one function call whose callee - * name is NOT in `knownCalleeNames` (i.e. an opaque/external call whose effects - * are unknown to the compiler). + * Returns true if fn's body contains any opaque external call — either a bare + * function call (`ident(`) or a member/namespace call (`obj.method()`) — where + * the callee/receiver is NOT in `knownNames` and is not a compiler-known stdlib + * builtin, control-flow keyword, or CapCase error constructor. * - * A call is detected as `ident(` where the ident is not a property access and - * is not in `knownCalleeNames`. Inner fn declarations are excluded. + * Both patterns can reach external resources: a bare `fetchData()` or a member + * call `db.read()` on an imported object. Suppresses over-declaration warnings + * (DEP003/DEP004, THR004) when the fn has at least one such call whose effects + * are unknown to the compiler. * - * Botscript stdlib value helpers (ok, isOk, some, none, etc.) and CapCase type - * constructors are never treated as opaque — they are compiler-known builtins. + * `localNames` (optional) is a set of parameter or local-variable names that + * should not be treated as opaque namespace/object receivers. For example, + * `name.trim()` where `name` is a string parameter is not an opaque import + * method call and must not trigger suppression. */ export function hasOpaqueCall( tokens: Token[], fn: FnDecl, inner: FnDecl[], - knownCalleeNames: Set, + knownNames: Set, + localNames?: ReadonlySet, ): boolean { + // When localNames is not provided, lazily compute it so that method calls on + // fn parameters and local variables (e.g. `name.trim()`) are not mistaken for + // opaque namespace/object calls. + const effectiveLocalNames: ReadonlySet = + localNames ?? + (() => { + const names = collectTopLevelParamNames(fn.args); + for (const n of collectFnBodyLocalNames(tokens, fn, inner)) names.add(n); + return names; + })(); + const open: FnDecl[] = []; let nextInner = 0; @@ -174,26 +407,143 @@ export function hasOpaqueCall( const tok = tokens[i]; if (!tok || tok.kind !== "ident") continue; + if (tok.text === fn.name) continue; + if (knownNames.has(tok.text)) continue; - // Skip known callee names (same-file fns and listed external fns). - if (knownCalleeNames.has(tok.text)) continue; + // Skip control-flow identifiers that look like calls but aren't. + if (CONTROL_FLOW_IDENTS.has(tok.text)) continue; - // Skip compiler-known builtins (err, ok, some, none, etc.) and CapCase - // identifiers (error-type constructors like `NetworkError(...)`). - if (BOTSCRIPT_BUILTIN_CALLS.has(tok.text) || /^[A-Z]/.test(tok.text)) continue; + // Skip compiler-known stdlib builtins (ok, err, some, none, etc.). + if (STDLIB_VALUE_CALL_NAMES.has(tok.text)) continue; - // Skip property accesses: `obj.helper(...)` or `obj?.helper(...)`. + // Skip method identifiers that are part of a property/member access. const prevIdx = prevSignificant(tokens, i - 1); const prev = tokens[prevIdx]; if (prev && ((prev.kind === "punct" && prev.text === ".") || prev.kind === "questionDot")) continue; - // Must be followed by `(` or `?.(` to be a call. - // `fn?.()` is an optional direct call — effects are still in play. - if (!isDirectOrOptionalCall(tokens, i)) continue; + const nextIdx = nextSignificant(tokens, i + 1); + const next = tokens[nextIdx]; + + // Detect member calls on unknown namespace objects: `db.read()` or `api.helper()`. + // If this ident is followed by `.`, it is used as a namespace/object receiver. + // Stdlib namespaces (time, http, etc.) are handled by cap-check — skip those. + // Local names (fn parameters, local variables) are also excluded: `name.trim()` + // is a method on a known local, not a call to an unknown namespace import. + if (next && ((next.kind === "punct" && next.text === ".") || next.kind === "questionDot")) { + const afterDotIdx = nextSignificant(tokens, nextIdx + 1); + const afterDot = tokens[afterDotIdx]; + + if (afterDot && afterDot.kind === "ident") { + // Member-access call: `db.read(...)` or chained: `obj.a.b()` + if (STDLIB_NAMESPACES.has(tok.text)) continue; + if (effectiveLocalNames.has(tok.text)) continue; + // Walk the full member chain (handles single and multi-segment: obj.m() / obj.a.b()) + let segIdx = afterDotIdx; + let chainEndsInCall = false; + while (true) { + const afterSegIdx = nextSignificant(tokens, segIdx + 1); + const afterSeg = tokens[afterSegIdx]; + if (!afterSeg) break; + // seg(...) — direct call + if (afterSeg.kind === "open" && afterSeg.text === "(") { + chainEndsInCall = true; break; + } + // seg. or seg?. — either optional call seg?.() or chain continuation seg.next / seg?.next + if ((afterSeg.kind === "punct" && afterSeg.text === ".") || afterSeg.kind === "questionDot") { + const nextSegIdx = nextSignificant(tokens, afterSegIdx + 1); + const nextSeg = tokens[nextSegIdx]; + if (nextSeg?.kind === "open" && nextSeg.text === "(") { + chainEndsInCall = true; break; // seg?.() + } + if (nextSeg && nextSeg.kind === "ident") { + segIdx = nextSegIdx; continue; // seg.next or seg?.next — keep walking + } + break; + } + break; // property access without a following call + } + if (chainEndsInCall) return true; + continue; + } + + if (afterDot && afterDot.kind === "open" && afterDot.text === "(") { + // Optional bare call: `fn?.()` — `?.` is immediately followed by `(`. + // Local variables and callback parameters are not opaque external callers. + if (effectiveLocalNames.has(tok.text)) continue; + return true; + } + + continue; // `?.` followed by something other than ident or `(` — not a call + } + + // Must be followed by `(` to be a bare function call. + if (!next || next.kind !== "open" || next.text !== "(") continue; + + // CapCase idents followed by `(` are opaque external calls UNLESS they appear + // inside `err(TypeName...)` or `err(new TypeName...)` — those are error-type + // constructors, not user-defined functions. + if (/^[A-Z]/.test(tok.text)) { + // err(TypeName(...)) — prev is `(`, prevprev is `err` + if (prev && prev.kind === "open" && prev.text === "(") { + const prevPrevIdx = prevSignificant(tokens, prevIdx - 1); + const prevPrev = tokens[prevPrevIdx]; + if (prevPrev && prevPrev.kind === "ident" && prevPrev.text === "err") continue; + } + // err(new TypeName(...)) — prev is `new`, prev-of-prev is `(`, prev3 is `err` + if (prev && prev.kind === "ident" && prev.text === "new") { + const prevNewIdx = prevIdx; + const prevParenIdx = prevSignificant(tokens, prevNewIdx - 1); + const prevParen = tokens[prevParenIdx]; + if (prevParen && prevParen.kind === "open" && prevParen.text === "(") { + const prevErrIdx = prevSignificant(tokens, prevParenIdx - 1); + const prevErr = tokens[prevErrIdx]; + if (prevErr && prevErr.kind === "ident" && prevErr.text === "err") continue; + } + } + } return true; } return false; } + +export 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; +} + +export function prevSignificant(tokens: Token[], start: number): number { + let i = start; + while (i >= 0) { + const t = tokens[i]; + if (!t) return i; + if ( + t.kind === "whitespace" || + t.kind === "newline" || + t.kind === "lineComment" || + t.kind === "blockComment" + ) { + i--; + continue; + } + return i; + } + return i; +} + diff --git a/packages/compiler/src/passes/dep-check.ts b/packages/compiler/src/passes/dep-check.ts index 4c803a26..d3e6638d 100644 --- a/packages/compiler/src/passes/dep-check.ts +++ b/packages/compiler/src/passes/dep-check.ts @@ -21,28 +21,31 @@ * * DEP002 writes under-declared: same for writes { ... }. * + * DEP003 reads over-declared (warning): fn has at least one same-file (or + * moduleEffects) callee but no callee (transitively) declares `reads { x }`. + * The label likely became stale after a refactor removed the callee that + * originally justified it. Leaf fns are excluded — they may be the actual + * access point and the compiler cannot verify the body directly. + * + * DEP004 writes over-declared (warning): same for writes { ... }. + * * Same-file call resolution is performed by default. Cross-file calls are * opaque unless the caller provides a `moduleEffects` map (via * `TransformOptions.moduleEffects`): any function listed there is treated as * if its declaration were in the current file, so a caller that omits its * reads/writes labels fires DEP001/DEP002 exactly as for a same-file callee. * Dynamic dispatch and higher-order function arguments are not tracked. - * - * Over-declaration is intentionally NOT checked here. The reads/writes labels - * are user-defined strings; unlike stdlib capability names, the compiler has - * no way to verify that a declared label is actually accessed in the body. - * Over-declaration is therefore always allowed (conservative declarations are - * harmless and may even be intentional for documentation purposes). */ -import { BotscriptError } from "../diagnostics.js"; +import { BotscriptError, type Diagnostic } from "../diagnostics.js"; import { getErrorCode } from "../error-codes.js"; import { parseProgram } from "../parser/parse.js"; import type { FnDecl } from "../parser/parse-fn.js"; import { atLeast, type VersionInfo } from "./version.js"; import { locationOf } from "./_location.js"; -import { computeNesting, collectCallees } from "./_callgraph.js"; +import { computeNesting, collectCallees, hasOpaqueCall, collectTopLevelParamNames, collectFnBodyLocalNames } from "./_callgraph.js"; import { buildImportAliasMap, type ModuleEffects } from "../module-effects.js"; +import { collectStdlibAliases } from "./_alias.js"; // --------------------------------------------------------------------------- // Types @@ -66,19 +69,28 @@ interface FnRecord { // Entry point // --------------------------------------------------------------------------- +export interface DepCheckResult { + code: string; + warnings: ReadonlyArray; +} + export function passDepCheck( src: string, version: VersionInfo, moduleEffects?: ModuleEffects, -): string { - if (!atLeast(version.resolved, "0.9")) return src; +): DepCheckResult { + if (!atLeast(version.resolved, "0.9")) return { code: src, warnings: [] }; 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; + if (decls.length === 0) return { code: src, warnings: [] }; + + // Module-level stdlib alias names (e.g. `const t = time` → `t`). These are + // excluded from opaque-call detection so `t.now()` does not suppress DEP003/DEP004. + const stdlibAliasNames = new Set(collectStdlibAliases(tokens).keys()); // Resolve import aliases: `import { fetchRow as fetchUser }` means a call // to `fetchUser` in this file should look up `fetchRow` in moduleEffects. @@ -87,8 +99,13 @@ export function passDepCheck( // Build maps for external (cross-file) effect declarations. const extReads = new Map>(); const extWrites = new Map>(); + // All declared names in moduleEffects (regardless of whether they have reads/writes). + // Used by DEP003/DEP004 to distinguish "known callee with no labels" from "unknown callee" + // — only known callees count as non-self callees; unknown ones are opaque. + const knownExternalNames = new Set(); if (moduleEffects) { for (const [name, eff] of Object.entries(moduleEffects)) { + knownExternalNames.add(name); if (eff.reads?.length) extReads.set(name, new Set(eff.reads)); if (eff.writes?.length) extWrites.set(name, new Set(eff.writes)); } @@ -100,11 +117,21 @@ export function passDepCheck( const fnNames = new Set(decls.map((d) => d.name)); // Include external function names (and their local aliases) in the callee - // scan so cross-file calls appear in the callees set. - const aliasedLocalNames = new Set(importAliases.keys()); + // scan so cross-file calls appear in the callees set. knownExternalNames + // covers moduleEffects entries with no reads/writes so calls to them are + // collected and treated as non-self callees by DEP003/DEP004. + // + // Only include aliases whose resolved target is actually present in + // moduleEffects. An alias for an un-listed import is opaque — including it + // would mask the opaque call from hasOpaqueCall and produce false warnings. + const aliasedKnownNames = new Set( + [...importAliases.entries()] + .filter(([, resolved]) => knownExternalNames.has(resolved)) + .map(([alias]) => alias), + ); const allCalleeNames = - extReads.size > 0 || extWrites.size > 0 || aliasedLocalNames.size > 0 - ? new Set([...fnNames, ...extReads.keys(), ...extWrites.keys(), ...aliasedLocalNames]) + knownExternalNames.size > 0 || aliasedKnownNames.size > 0 + ? new Set([...fnNames, ...knownExternalNames, ...aliasedKnownNames]) : fnNames; // Precompute each fn's nested (descendant) decls once via a single sweep, @@ -236,7 +263,92 @@ export function passDepCheck( } } - return src; + // 5. DEP003/DEP004: over-declared (warning-level). + // For each fn that has at least one same-file (or moduleEffects) non-self callee, + // compute which declared reads/writes labels are NOT justified by any callee. + // Leaf fns (no tracked callees) and self-only-recursive fns are excluded — + // they may be the actual access point and the compiler can't verify the body. + // Callback parameter annotations (paramReads/paramWrites) are treated as + // implicit callee justification: a wrapper that receives a reads { db } callback + // and propagates that label upward is not over-declaring. + // Fns that call any opaque (unlisted) function are also excluded — the unknown + // callee may be the actual read/write point that justifies the label. + // + // Justification uses a DFS over the callee graph rooted at rec.decl's direct + // callees, collecting each reachable fn's DECLARED reads/writes. The root fn + // itself is pre-added to `visited` so the DFS stops at it during mutual recursion + // (f calls g, g calls f → DFS skips f when encountered via g, preventing + // f from justifying its own labels through the cycle). + const warnings: Diagnostic[] = []; + for (const rec of records.values()) { + if (rec.callees.size === 0) continue; + + // Seed justification from callback parameter annotations. A wrapper fn that + // accepts a `fn() reads { db } -> T` callback and propagates reads { db } + // upward is not over-declaring, even if regular callees don't read db. + const calleeReads = new Set(rec.decl.paramReads); + const calleeWrites = new Set(rec.decl.paramWrites); + + // DFS from direct callees, collecting their declared reads/writes transitively. + // rec.decl is pre-added so the DFS stops there, preventing circular justification. + const visited = new Set([rec.decl]); + let hasNonSelfCallee = false; + + const dfsStack: string[] = [...rec.callees]; + while (dfsStack.length > 0) { + const calleeName = dfsStack.pop()!; + const calleeDecls = declsByName.get(calleeName); + if (calleeDecls) { + for (const calleeDecl of calleeDecls) { + if (visited.has(calleeDecl)) continue; + visited.add(calleeDecl); + hasNonSelfCallee = true; + const callee = records.get(calleeDecl); + if (!callee) continue; + for (const label of callee.decl.reads ?? []) calleeReads.add(label); + for (const label of callee.decl.writes ?? []) calleeWrites.add(label); + for (const nextCallee of callee.callees) dfsStack.push(nextCallee); + } + continue; + } + const resolvedCallee = importAliases.get(calleeName) ?? calleeName; + if (!knownExternalNames.has(resolvedCallee)) continue; + hasNonSelfCallee = true; + const extR = extReads.get(resolvedCallee); + if (extR) for (const label of extR) calleeReads.add(label); + const extW = extWrites.get(resolvedCallee); + if (extW) for (const label of extW) calleeWrites.add(label); + } + + // Self-only-recursive fns are treated like leaves. + if (!hasNonSelfCallee) continue; + + // Suppress when the fn calls any opaque (unlisted) external function — that + // unknown callee may be the actual read/write point that justifies the label. + // Combine param names and local variable names so member calls on any known + // local (e.g. `str.trim()`, `name.length`) are not mistaken for opaque + // namespace/import method calls and don't suppress DEP003/DEP004. + const inner = innerByDecl.get(rec.decl) ?? []; + const paramNames = collectTopLevelParamNames(rec.decl.args); + const bodyLocals = collectFnBodyLocalNames(tokens, rec.decl, inner); + const localNames = new Set([...paramNames, ...bodyLocals, ...stdlibAliasNames]); + // Include param names in knownNames so bare calls to callback parameters + // (e.g. `loader()`) are not treated as opaque external calls — their effects + // are already captured via paramReads/paramWrites. + const knownWithParams = new Set([...allCalleeNames, ...paramNames]); + if (hasOpaqueCall(tokens, rec.decl, inner, knownWithParams, localNames)) continue; + + const overDeclaredReads = [...rec.declaredReads].filter((l) => !calleeReads.has(l)).sort(); + if (overDeclaredReads.length > 0) { + warnings.push(mkOverDeclaredWarning(src, rec, "reads", overDeclaredReads)); + } + const overDeclaredWrites = [...rec.declaredWrites].filter((l) => !calleeWrites.has(l)).sort(); + if (overDeclaredWrites.length > 0) { + warnings.push(mkOverDeclaredWarning(src, rec, "writes", overDeclaredWrites)); + } + } + + return { code: src, warnings }; } // --------------------------------------------------------------------------- @@ -327,3 +439,48 @@ function mkError( return new BotscriptError([diagnostic]); } + +function mkOverDeclaredWarning( + src: string, + rec: FnRecord, + kind: "reads" | "writes", + overLabels: string[], +): Diagnostic { + const code = kind === "reads" ? "DEP003" : "DEP004"; + const entry = getErrorCode(code)!; + const { line, column } = locationOf(src, rec.decl.fnKeywordStart); + const nameEnd = rec.decl.nameStart + rec.decl.name.length; + + const labelList = overLabels.join(", "); + const firstLabel = overLabels[0]!; + + const notPropagated = + overLabels.length === 1 + ? `'${firstLabel}' is not declared by any tracked callee` + : `[${labelList}] are not declared by any tracked callee`; + const message = + `fn '${rec.decl.name}' declares ${kind} { ${labelList} } but ${notPropagated}; ` + + `annotation may be stale`; + + const proposed = + kind === "reads" + ? [...rec.declaredReads].filter((l) => !overLabels.includes(l)).sort() + : [...rec.declaredWrites].filter((l) => !overLabels.includes(l)).sort(); + + return { + code, + severity: "warning", + file: null, + line, + column, + start: rec.decl.fnKeywordStart, + end: nameEnd, + message, + rule: entry.rule, + idiom: entry.idiom, + rewrite: + proposed.length > 0 + ? `fn ${rec.decl.name}(...) ${kind} { ${proposed.join(", ")} } -> ... // remove stale label: ${labelList}` + : `fn ${rec.decl.name}(...) -> ... // remove stale ${kind} {} clause: ${labelList}`, + }; +} diff --git a/packages/compiler/src/passes/thr-check.ts b/packages/compiler/src/passes/thr-check.ts index 452c5a4f..f3c86e51 100644 --- a/packages/compiler/src/passes/thr-check.ts +++ b/packages/compiler/src/passes/thr-check.ts @@ -37,8 +37,9 @@ import type { FnDecl } from "../parser/parse-fn.js"; import { atLeast, type VersionInfo } from "./version.js"; import { locationOf } from "./_location.js"; import type { Token } from "../parser/lex.js"; -import { computeNesting, collectCallees, hasOpaqueCall, nextSignificant, prevSignificant } from "./_callgraph.js"; +import { computeNesting, collectCallees, hasOpaqueCall, collectFnBodyLocalNames, nextSignificant, prevSignificant } from "./_callgraph.js"; import { buildImportAliasMap, type ModuleEffects } from "../module-effects.js"; +import { collectStdlibAliases } from "./_alias.js"; import { extractResultArgs, splitTopLevelPipe, leadingTypeIdent } from "./_type-parser.js"; // --------------------------------------------------------------------------- @@ -74,6 +75,10 @@ export function passThrCheck( if (decls.length === 0) return { code: src, warnings: [] }; + // Stdlib alias names (e.g. `const t = time`): collected once so hasOpaqueCall + // can treat `t.now()` as a known local, not an opaque external member call. + const stdlibAliasNames = new Set(collectStdlibAliases(tokens).keys()); + // Resolve import aliases: `import { fetchRow as fetchUser }` means a call // to `fetchUser` should look up `fetchRow` in moduleEffects. const importAliases = moduleEffects ? buildImportAliasMap(tokens) : new Map(); @@ -101,6 +106,21 @@ export function passThrCheck( ? new Set([...fnNames, ...knownExternalNames, ...aliasedLocalNames]) : fnNames; + // For opaque-call detection, only import aliases whose resolved target is + // tracked in moduleEffects count as "known". An alias to an untracked + // external (not in moduleEffects) is itself an opaque call — including it + // in knownForOpaque would suppress THR004 for fns that genuinely have + // unknown external dependencies. + const trackedAliasNames = new Set( + [...importAliases.entries()] + .filter(([, resolved]) => knownExternalNames.has(resolved)) + .map(([alias]) => alias), + ); + const opaqueKnownBase = + knownExternalNames.size > 0 || trackedAliasNames.size > 0 + ? new Set([...fnNames, ...knownExternalNames, ...trackedAliasNames]) + : fnNames; + const innerByDecl = computeNesting(decls); for (const decl of decls) { @@ -238,12 +258,16 @@ export function passThrCheck( // Fn parameter names are excluded from the opaque-call check because their // effect surface is already captured by `paramThrows` — calling a typed // callback param is not an unknown external call. + // Stdlib aliases (e.g. `const t = time`) are also excluded so that `t.now()` + // is not mistaken for an opaque external member call. const inner = innerByDecl.get(rec.decl) ?? []; const paramNames = collectParamNames(rec.decl); const knownForOpaque = paramNames.size > 0 - ? new Set([...allCalleeNames, ...paramNames]) - : allCalleeNames; - if (hasOpaqueCall(tokens, rec.decl, inner, knownForOpaque)) continue; + ? new Set([...opaqueKnownBase, ...paramNames]) + : opaqueKnownBase; + const bodyLocals = collectFnBodyLocalNames(tokens, rec.decl, inner); + const localNames = new Set([...paramNames, ...bodyLocals, ...stdlibAliasNames]); + if (hasOpaqueCall(tokens, rec.decl, inner, knownForOpaque, localNames)) continue; const overDeclared = [...rec.declaredThrows].filter((l) => !justifiedThrows.has(l)).sort(); if (overDeclared.length > 0) { diff --git a/packages/compiler/tests/dep-check.test.ts b/packages/compiler/tests/dep-check.test.ts index 80b66e79..a2fd5595 100644 --- a/packages/compiler/tests/dep-check.test.ts +++ b/packages/compiler/tests/dep-check.test.ts @@ -3,6 +3,8 @@ * * DEP001: fn A calls fn B which reads { x }, but A doesn't declare reads { x }. * DEP002: fn A calls fn B which writes { x }, but A doesn't declare writes { x }. + * DEP003: fn declares reads { x } but no callee (transitively) reads { x }. + * DEP004: fn declares writes { x } but no callee (transitively) writes { x }. */ import { describe, expect, it } from "vitest"; @@ -270,3 +272,547 @@ describe("DEP001/DEP002: optional direct call syntax fn?.()", () => { expect(() => compile(src)).not.toThrow(); }); }); + +// --------------------------------------------------------------------------- +// DEP003: reads over-declared +// --------------------------------------------------------------------------- + +describe("DEP003: reads over-declared (0.9+)", () => { + it("fires when a fn declares reads { x } but no callee declares reads { x }", () => { + const src = + "?bs 0.9\n" + + "fn formatName(s: string) -> string = s\n" + + "fn getUserName(id: string) reads { userDb } -> string = formatName(\"Alice\")\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + expect(result.warnings.find((w) => w.code === "DEP003")!.message).toContain("userDb"); + }); + + it("does NOT fire when the declared label is justified by a same-file callee", () => { + const src = + "?bs 0.9\n" + + "fn getUser(id: string) reads { userDb } -> string = id\n" + + "fn getUserName(id: string) reads { userDb } -> string = getUser(id)\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("does NOT fire when the declared label is justified transitively", () => { + const src = + "?bs 0.9\n" + + "fn readDb(id: string) reads { userDb } -> string = id\n" + + "fn getUser(id: string) reads { userDb } -> string = readDb(id)\n" + + "fn getUserName(id: string) reads { userDb } -> string = getUser(id)\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("fires a single warning listing all over-declared labels when multiple are stale", () => { + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f() reads { db, cache } -> string = helper()\n"; + const result = transform(src); + const dep3warns = result.warnings.filter((w) => w.code === "DEP003"); + expect(dep3warns).toHaveLength(1); + expect(dep3warns[0]!.message).toContain("db"); + expect(dep3warns[0]!.message).toContain("cache"); + }); + + it("fires warning with severity 'warning'", () => { + const src = + "?bs 0.9\n" + + "fn noop() -> string = \"ok\"\n" + + "fn f() reads { x } -> string = noop()\n"; + const result = transform(src); + const w = result.warnings.find((w) => w.code === "DEP003"); + expect(w).toBeDefined(); + expect(w!.severity).toBe("warning"); + }); + + it("does not fire below ?bs 0.9", () => { + const src = + "?bs 0.8\n" + + "fn helper() -> string = \"ok\"\n" + + "fn f() reads { x } -> string = helper()\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("does not throw — DEP003 is non-blocking", () => { + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"Alice\"\n" + + "fn f() reads { userDb } -> string = helper()\n"; + expect(() => compile(src)).not.toThrow(); + }); +}); + +// --------------------------------------------------------------------------- +// DEP004: writes over-declared +// --------------------------------------------------------------------------- + +describe("DEP004: writes over-declared (0.9+)", () => { + it("fires when a fn declares writes { x } but no callee declares writes { x }", () => { + const src = + "?bs 0.9\n" + + "fn noop(msg: string) -> void { }\n" + + "fn logEvent(msg: string) writes { auditLog } -> void { noop(msg); }\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP004")).toBe(true); + expect(result.warnings.find((w) => w.code === "DEP004")!.message).toContain("auditLog"); + }); + + it("does NOT fire when the declared label is justified by a same-file callee", () => { + const src = + "?bs 0.9\n" + + "fn writeAudit(msg: string) writes { auditLog } -> void { }\n" + + "fn logEvent(msg: string) writes { auditLog } -> void { writeAudit(msg) }\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP004")).toBe(false); + }); + + it("fires with severity 'warning' and does not throw", () => { + const src = + "?bs 0.9\n" + + "fn noop(msg: string) -> void { }\n" + + "fn logEvent(msg: string) writes { auditLog } -> void { noop(msg); }\n"; + expect(() => compile(src)).not.toThrow(); + const result = transform(src); + expect(result.warnings.find((w) => w.code === "DEP004")!.severity).toBe("warning"); + }); + + it("does not fire below ?bs 0.9", () => { + const src = + "?bs 0.8\n" + + "fn noop(msg: string) -> void { }\n" + + "fn logEvent(msg: string) writes { auditLog } -> void { noop(msg); }\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP004")).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// DEP003/DEP004: self-recursive fns and callback parameter justification +// --------------------------------------------------------------------------- + +describe("DEP003/DEP004: transitive callee justification", () => { + it("does NOT fire DEP003 when a multi-hop callee chain justifies the label", () => { + // f reads { db } calls g, g calls h, h reads { db }. + // The DFS must reach h through g to find the justification. + const src = + "?bs 0.9\n" + + "fn h() reads { db } -> string = \"row\"\n" + + "fn g() reads { db } -> string = h()\n" + + "fn f() reads { db } -> string = g()\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("does NOT fire DEP003 when a callee (leaf) declares the same label", () => { + // f reads { db } calls g, g reads { db } (leaf). f is justified by g. + const src = + "?bs 0.9\n" + + "fn g() reads { db } -> string = \"row\"\n" + + "fn f() reads { db } -> string = g()\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); +}); + +describe("DEP003/DEP004: self-recursive fn exclusion", () => { + it("does not fire DEP003 for a self-recursive fn — treated as leaf/access-point", () => { + const src = + "?bs 0.9\n" + + "fn countdown(n: number) reads { store } -> void {\n" + + " if (n > 0) countdown(n - 1);\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("does not fire DEP004 for a self-recursive fn — treated as leaf/access-point", () => { + const src = + "?bs 0.9\n" + + "fn accumulate(n: number) writes { store } -> void {\n" + + " if (n > 0) accumulate(n - 1);\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP004")).toBe(false); + }); +}); + +describe("DEP003/DEP004: callback parameter justification", () => { + it("does not fire DEP003 when reads label is justified by a callback parameter annotation", () => { + const src = + "?bs 0.9\n" + + "fn noop() -> void { }\n" + + "fn withCache(loader: () reads { cache } -> string) reads { cache } -> string {\n" + + " noop();\n" + + " loader()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("does not fire DEP004 when writes label is justified by a callback parameter annotation", () => { + const src = + "?bs 0.9\n" + + "fn noop() -> void { }\n" + + "fn withAudit(cb: () writes { auditLog } -> void) writes { auditLog } -> void {\n" + + " noop();\n" + + " cb()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP004")).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// DEP003/DEP004: opaque external call suppression +// --------------------------------------------------------------------------- + +describe("DEP003/DEP004: opaque external call suppression", () => { + it("does not fire DEP003 when fn also calls an unlisted external helper", () => { + // localHelper is tracked (same-file), unknownHelper is NOT in allCalleeNames. + // DEP003 must be suppressed because unknownHelper may be the actual reader. + const src = + "?bs 0.9\n" + + "fn localHelper() -> string = \"x\"\n" + + "fn f() reads { db } -> string { localHelper(); unknownHelper() }\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("does not fire DEP004 when fn also calls an unlisted external helper", () => { + const src = + "?bs 0.9\n" + + "fn localHelper() -> void { }\n" + + "fn f() writes { db } -> void { localHelper(); unknownHelper() }\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP004")).toBe(false); + }); + + it("fires DEP003 when all callees are tracked and none declare the label", () => { + const src = + "?bs 0.9\n" + + "fn localHelper() -> string = \"x\"\n" + + "fn f() reads { db } -> string = localHelper()\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("fires DEP003 even when fn body contains if/while control flow", () => { + // `if (cond)` must not be treated as an opaque function call — it's + // control flow, not an external callee. + const src = + "?bs 0.9\n" + + "fn localHelper(x: number) -> string = \"ok\"\n" + + "fn f(flag: bool) reads { db } -> string {\n" + + " if (flag) localHelper(1);\n" + + " localHelper(2)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("fires DEP004 even when fn body contains while control flow", () => { + const src = + "?bs 0.9\n" + + "fn step(n: number) -> void { }\n" + + "fn f(n: number) writes { log } -> void {\n" + + " while (n > 0) step(n);\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP004")).toBe(true); + }); + + it("does not fire DEP003 when fn calls an unknown namespace object method", () => { + // `dbClient.query()` is a member call on an unknown namespace import. + // hasOpaqueCall must treat it as opaque so DEP003 is suppressed. + const src = + "?bs 0.9\n" + + "fn helper() -> void { }\n" + + "fn f() reads { db } -> string {\n" + + " helper();\n" + + " dbClient.query()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("does not fire DEP004 when fn calls an unknown namespace object method", () => { + const src = + "?bs 0.9\n" + + "fn step() -> void { }\n" + + "fn f() writes { log } -> void {\n" + + " step();\n" + + " logger.write()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP004")).toBe(false); + }); + + it("does not suppress DEP003 for stdlib namespace method calls (time.now)", () => { + // Stdlib namespaces are excluded from opaque detection — they are handled + // by cap-check, not by DEP003 suppression. + // Use `unsafe` to suppress UNS005 so the DEP003 check is reached. + const src = + "?bs 0.9\n" + + "fn helper() -> void { }\n" + + "fn f() uses { time } reads { db } -> number {\n" + + " helper();\n" + + " unsafe \"known\" { time.now() }\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("fires DEP003 even when fn calls a method on a string parameter (name.trim())", () => { + // `name.trim()` is a method call on a fn parameter — it is NOT an opaque + // external import. hasOpaqueCall must exclude parameter names from the + // namespace-receiver check so DEP003 is not incorrectly suppressed. + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f(name: string) reads { db } -> string {\n" + + " helper();\n" + + " name.trim()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("fires DEP004 even when fn calls a method on an array parameter (items.map())", () => { + const src = + "?bs 0.9\n" + + "fn step() -> void { }\n" + + "fn f(items: string[]) writes { log } -> void {\n" + + " step();\n" + + " items.map(step)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP004")).toBe(true); + }); + + it("fires DEP003 even when fn calls a method on an untyped parameter (name.trim())", () => { + // Untyped params like `fn f(name)` must be collected by collectTopLevelParamNames + // so that `name.trim()` is not treated as an opaque external call. + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f(name) reads { db } -> string {\n" + + " helper();\n" + + " name.trim()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("fires DEP003 even when fn calls a method on a local var binding (x.trim())", () => { + // `var x = ...` bindings must be collected alongside const/let so that + // `x.method()` is not mistaken for an opaque namespace call. + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f() reads { db } -> string {\n" + + " var name = helper();\n" + + " name.trim()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("fires DEP003 even when fn calls a method on a local const binding (name.trim())", () => { + // Local `const name = ...` followed by `name.trim()` is NOT an opaque external call. + // Without collectFnBodyLocalNames, `name` is absent from localNames and name.trim() + // incorrectly suppresses DEP003. + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f() reads { db } -> string {\n" + + " const name = helper();\n" + + " name.trim()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("fires DEP003 even when fn constructs an error type (err(NetworkError{...})) as an arg", () => { + // CapCase error-type constructors inside err(...) must NOT suppress DEP003. + // Standalone CapCase calls like `LoadUser()` SHOULD suppress (opaque external). + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f() reads { db } -> string {\n" + + " helper();\n" + + " err(NotFoundError { msg: \"x\" })\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("suppresses DEP003 when fn calls an unknown CapCase external function (LoadUser())", () => { + // A CapCase function call that is NOT inside err(...) is a genuine opaque + // external and should suppress DEP003 — the external may perform the read. + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f() reads { db } -> string {\n" + + " helper();\n" + + " LoadUser()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("suppresses DEP003 when param has object type annotation — inner idents are not param names", () => { + // `opts: { dbClient: string }` — `dbClient` is a property name in the type + // annotation, not an actual fn parameter. Before the brace-depth fix, + // collectTopLevelParamNames would add `dbClient` to localNames, causing + // `dbClient.query()` in the body to look like a local param method call + // rather than an opaque external call, which incorrectly allowed DEP003 to fire. + const src = + "?bs 0.9\n" + + "fn helper() -> void { }\n" + + "fn f(opts: { dbClient: string }) reads { db } -> void {\n" + + " helper();\n" + + " dbClient.query()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("fires DEP003 when param has generic type with object type arg — inner idents not param names", () => { + // `x: Map` — the comma inside `<...>` is NOT a param separator. + // Without the angle-bracket fix, collectTopLevelParamNames resets skipUntilNextParam + // on the `,` inside ``, then parses `{ a }` as a destructuring *parameter*, + // falsely adding `a` to the param-names set. That causes `a.method()` in the body to + // be classified as a local-param call rather than an opaque external call, so + // DEP003 would fire when it should be suppressed by the opaque call. + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f(x: Map) reads { db } -> string {\n" + + " helper();\n" + + " a.method()\n" + + "}\n"; + const result = transform(src); + // `a.method()` is an opaque external call — DEP003 must be suppressed + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("suppresses DEP003 when fn uses optional bare call to unknown external fn?.()", () => { + // `externalLib?.()` is an optional call to a function not declared in this file — opaque, + // so DEP003 must be suppressed (the external may perform the read). + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f() reads { db } -> void {\n" + + " helper();\n" + + " externalLib?.()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("suppresses DEP003 when fn calls an unknown namespace via optional member call obj.method?.()", () => { + // `db.read?.()` — optional method call on an unknown namespace object. + // hasOpaqueCall must detect the call pattern even with the `?.` operator. + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f() reads { db } -> void {\n" + + " helper();\n" + + " externalDb.read?.()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("suppresses DEP003 when fn calls a chained member call obj.a.b()", () => { + // `client.connection.query()` — two-segment chain on an unknown namespace import. + // hasOpaqueCall must detect the call even with a multi-segment receiver (obj.a.b()). + const src = + "?bs 0.9\n" + + "fn helper() -> void { }\n" + + "fn f() reads { db } -> void {\n" + + " helper();\n" + + " client.connection.query()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("suppresses DEP004 when fn calls a chained optional member call obj.a?.b()", () => { + // `store.cache?.write()` — optional chain on second segment, still opaque. + const src = + "?bs 0.9\n" + + "fn step() -> void { }\n" + + "fn f() writes { log } -> void {\n" + + " step();\n" + + " store.cache?.write()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP004")).toBe(false); + }); + + it("fires DEP003 when fn uses err(new TypeName(...)) — new-form error construction is not opaque", () => { + // `err(new NetworkError(...))` is the botscript `new`-form error constructor. + // It must NOT suppress DEP003 — the error construction is not an external read. + // (throws { NetworkError } declared so THR002 does not fire independently) + const src = + "?bs 0.9\n" + + "fn helper() -> string = \"x\"\n" + + "fn f() reads { db } throws { NetworkError } -> Result {\n" + + " helper();\n" + + " return err(new NetworkError(\"failed\"))\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("fires DEP003 when fn has destructured object param whose member is called — destructured name is local, not opaque", () => { + // `{ name }: User` — `name` is a local binding from the destructured param. + // `name.trim()` must NOT be treated as an opaque external call; DEP003 must still fire. + const src = + "?bs 0.9\n" + + "fn helper() -> void { }\n" + + "fn f({ name }: { name: string }) reads { db } -> string {\n" + + " helper();\n" + + " return name.trim()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("fires DEP003 when fn has destructured const local whose member is called — destructured local is not opaque", () => { + // `const { key } = obj` — `key` is a locally-bound name from destructuring. + // `key.trim()` must NOT suppress DEP003 as an opaque external call. + const src = + "?bs 0.9\n" + + "fn helper() -> void { }\n" + + "fn f(obj: { key: string }) reads { db } -> string {\n" + + " helper();\n" + + " const { key } = obj\n" + + " return key.trim()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("fires DEP003 when fn uses a module-level stdlib alias — alias is not an opaque external", () => { + // `const t = time` at module scope; `t.now()` in a fn body should NOT be treated as an + // opaque external call — it resolves to the `time` stdlib namespace. DEP003 must still fire. + const src = + "?bs 0.9\n" + + "const t = time\n" + + "fn helper() -> void { }\n" + + "fn f() uses { time } reads { db } -> number {\n" + + " helper();\n" + + ' return unsafe "reading wall clock" { t.now() }\n' + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "DEP003")).toBe(true); + }); +}); diff --git a/packages/compiler/tests/error-codes.test.ts b/packages/compiler/tests/error-codes.test.ts index 0eb75b0a..8864a5d4 100644 --- a/packages/compiler/tests/error-codes.test.ts +++ b/packages/compiler/tests/error-codes.test.ts @@ -9,7 +9,7 @@ describe("error-code registry", () => { const expected = [ "ALI001", "ALI002", "ALI003", "CAP001", "CAP002", "CAP003", - "DEP001", "DEP002", + "DEP001", "DEP002", "DEP003", "DEP004", "EFF002", "EFF003", "EFF004", "FMT001", "INT001", "INT002", "INT003", "INT004", "INT005", diff --git a/packages/compiler/tests/module-effects.test.ts b/packages/compiler/tests/module-effects.test.ts index ba1e776e..8d64972b 100644 --- a/packages/compiler/tests/module-effects.test.ts +++ b/packages/compiler/tests/module-effects.test.ts @@ -16,6 +16,10 @@ function compile(src: string, moduleEffects?: ModuleEffects): string { return transform(src, { moduleEffects }).code; } +function compileWarnings(src: string, moduleEffects?: ModuleEffects) { + return transform(src, { moduleEffects }).warnings; +} + // --------------------------------------------------------------------------- // DEP001: cross-file reads transitivity // --------------------------------------------------------------------------- @@ -310,11 +314,14 @@ describe("buildModuleEffects builder", () => { expect(Object.hasOwn(effects, "__proto__")).toBe(true); }); - it("omits fns with no declared effects", () => { + it("includes fns with no declared effects as empty entries", () => { + // Pure helpers (no reads/writes/throws) are included so DEP003/DEP004 can + // distinguish "known callee with no labels" from "unknown opaque callee". const effects = buildModuleEffects([ - "?bs 0.9\nfn pure(x: string) -> string = x\n", + "?bs 0.9\nfn helper(x: string) -> string = x\n", ]); - expect(Object.keys(effects)).toEqual([]); + expect(Object.hasOwn(effects, "helper")).toBe(true); + expect(effects.helper).toEqual({}); }); it("includes only exported fns when the source has export statements", () => { @@ -564,3 +571,41 @@ describe("CAP001: cross-file capability transitivity via moduleEffects", () => { expect(() => compile(src, moduleEffects)).not.toThrow(); }); }); + +// --------------------------------------------------------------------------- +// DEP003/DEP004: cross-file callee with no labels counts as non-self callee +// --------------------------------------------------------------------------- + +describe("DEP003/DEP004: moduleEffects entry with no reads/writes is a known callee", () => { + it("fires DEP003 when fn calls a moduleEffects entry with no reads and declares reads { x }", () => { + // `helper` is a known external callee ({} — no declared reads) so the + // caller's `reads { userDb }` is over-declared. Before the fix, + // `helper` was absent from allCalleeNames and the fn was treated as a + // leaf, suppressing the warning. + const src = + "?bs 0.9\n" + + "fn load(id: string) reads { userDb } -> string = helper(id)\n"; + const mods: ModuleEffects = { helper: {} }; + const warnings = compileWarnings(src, mods); + expect(warnings.some((w) => w.code === "DEP003")).toBe(true); + }); + + it("does NOT fire DEP003 when the external callee is unknown (not in moduleEffects)", () => { + // Unknown callees are opaque: we cannot tell whether they read anything, + // so we must not warn about the caller's annotation being stale. + const src = + "?bs 0.9\n" + + "fn load(id: string) reads { userDb } -> string = helper(id)\n"; + const warnings = compileWarnings(src); + expect(warnings.some((w) => w.code === "DEP003")).toBe(false); + }); + + it("fires DEP004 when fn calls a moduleEffects entry with no writes and declares writes { x }", () => { + const src = + "?bs 0.9\n" + + "fn persist(id: string) writes { auditLog } -> string = helper(id)\n"; + const mods: ModuleEffects = { helper: {} }; + const warnings = compileWarnings(src, mods); + expect(warnings.some((w) => w.code === "DEP004")).toBe(true); + }); +}); diff --git a/packages/compiler/tests/thr-check.test.ts b/packages/compiler/tests/thr-check.test.ts index b4baa375..3c03b000 100644 --- a/packages/compiler/tests/thr-check.test.ts +++ b/packages/compiler/tests/thr-check.test.ts @@ -490,4 +490,21 @@ describe("THR004: over-declared throws — Result error-position symmetry", () = const result = transform(src); expect(result.warnings.some((w) => w.code === "THR004" && w.message.includes("parseUser"))).toBe(true); }); + + it("fires THR004 when fn uses a module-level stdlib alias — alias is not an opaque external", () => { + // `const t = time` at module scope; `t.now()` in a fn body should NOT be treated as an + // opaque external call suppressing THR004. The alias resolves to the `time` stdlib namespace + // which is a known, tracked surface, not an unknown external callee. + const src = + "?bs 0.9\n" + + "const t = time\n" + + "fn helper(x: string) -> string = x\n" + + "fn run(id: string) uses { time } throws { ParseError } -> Result {\n" + + ' unsafe "reading wall clock" { t.now() }\n' + + " helper(id)\n" + + " return err(ParseError(\"invalid\"))\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "THR004" && w.message.includes("run"))).toBe(true); + }); }); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index fe445593..ea3797b3 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -822,9 +822,9 @@ export const EXPLANATIONS: Readonly> = { "`reads { y }`, then both B and A must declare `reads { y }`.\n\n" + "The purpose is completeness: reading A's header should tell you every resource " + "category A (or anything it calls) touches, without tracing through the call graph.\n\n" + - "Over-declaration is always allowed — declaring more than the minimum is conservative " + - "and harmless. DEP001 only fires on under-declaration (a label that is reachable but " + - "not declared).", + "Over-declaration may be warned about from `?bs 0.9` (see DEP003) for non-leaf fns with " + + "tracked same-file callees and no opaque external calls. DEP001 only fires on " + + "under-declaration (a label that is reachable but not declared).", example: { fails: "?bs 0.9\n" + @@ -1024,7 +1024,8 @@ export const EXPLANATIONS: Readonly> = { "`writes { x }`. The rule extends to any depth.\n\n" + "The purpose is completeness: reading A's header should tell you every resource " + "category A (or anything it calls) writes to, without tracing through the call graph.\n\n" + - "Over-declaration is always allowed. DEP002 only fires on under-declaration.", + "Over-declaration may be warned about from `?bs 0.9` (see DEP004) for non-leaf fns with " + + "tracked same-file callees and no opaque external calls. DEP002 only fires on under-declaration.", example: { fails: "?bs 0.9\n" + @@ -1036,6 +1037,58 @@ export const EXPLANATIONS: Readonly> = { "fn recordEvent(id: string) writes { metrics } -> void { updateMetrics(id); }\n", }, }, + DEP003: { + code: "DEP003", + title: "fn declares reads {} label not justified by any tracked callee (warning)", + body: + "From `?bs 0.9`, DEP003 fires as a **warning** (not an error) when fn A has same-file " + + "callees but no callee (direct or transitive) declares `reads { x }` that A also declares. " + + "The label is not justified by the call graph — a common sign of a refactor that removed " + + "the callee that originally justified the annotation.\n\n" + + "**Scope:** only fires for fns with at least one same-file (or moduleEffects) callee. " + + "Leaf fns are excluded — they may be the actual access point, and the compiler cannot " + + "scan the body for direct resource accesses (reads {} labels are user-defined strings, " + + "not stdlib references).\n\n" + + "DEP003 is also suppressed when the function body contains any opaque or untracked external " + + "call (a call to a function not visible to the same-file call graph) — the unknown callee may " + + "be the actual read site, so the warning is withheld to avoid false positives.\n\n" + + "DEP003 is gated on `?bs 0.9`. The symmetrical under-declaration check is DEP001.", + example: { + fails: + "?bs 0.9\n" + + "fn helper(id: string) -> string = id\n" + + "fn getUserName(id: string) reads { userDb } -> string = helper(id)\n", + passes: + "?bs 0.9\n" + + "fn getUser(id: string) reads { userDb } -> string = id\n" + + "fn getUserName(id: string) reads { userDb } -> string = getUser(id)\n", + }, + }, + DEP004: { + code: "DEP004", + title: "fn declares writes {} label not justified by any tracked callee (warning)", + body: + "From `?bs 0.9`, DEP004 fires as a **warning** (not an error) when fn A has same-file " + + "callees but no callee (direct or transitive) declares `writes { x }` that A also declares. " + + "The label is not justified by the call graph and may be stale.\n\n" + + "**Scope:** only fires for fns with at least one same-file (or moduleEffects) callee. " + + "Leaf fns are excluded — the compiler cannot scan the body for direct resource modifications " + + "(writes {} labels are user-defined strings).\n\n" + + "DEP004 is also suppressed when the function body contains any opaque or untracked external " + + "call (a call to a function not visible to the same-file call graph) — the unknown callee may " + + "be the actual write site, so the warning is withheld to avoid false positives.\n\n" + + "DEP004 is gated on `?bs 0.9`. The symmetrical under-declaration check is DEP002.", + example: { + fails: + "?bs 0.9\n" + + "fn helper(msg: string) -> void { }\n" + + "fn logEvent(msg: string) writes { auditLog } -> void { helper(msg) }\n", + passes: + "?bs 0.9\n" + + "fn writeAudit(msg: string) writes { auditLog } -> void { }\n" + + "fn logEvent(msg: string) writes { auditLog } -> void { writeAudit(msg) }\n", + }, + }, THR001: { code: "THR001", title: "fn transitively throws an exception type not declared in its header", diff --git a/packages/mcp/tests/server.test.ts b/packages/mcp/tests/server.test.ts index 57e87d71..0cec4df6 100644 --- a/packages/mcp/tests/server.test.ts +++ b/packages/mcp/tests/server.test.ts @@ -55,6 +55,8 @@ describe("botscript-mcp explanations", () => { "CAP003", "DEP001", "DEP002", + "DEP003", + "DEP004", "EFF002", "EFF003", "EFF004", From 796145e47feff3628ec527648d761e6a026885ee Mon Sep 17 00:00:00 2001 From: Marcelo Bukowski de Farias Date: Thu, 11 Jun 2026 23:33:26 -0300 Subject: [PATCH 09/11] =?UTF-8?q?feat(compiler):=20SYN004=20=E2=80=94=20wa?= =?UTF-8?q?rn=20on=20eval()=20and=20new=20Function()=20calls=20in=20fn=20b?= =?UTF-8?q?odies=20(=3Fbs=200.7+)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(compiler): SYN004 — warn on eval() and new Function() calls in fn bodies (?bs 0.7+) eval() and new Function() execute strings as code at runtime, bypassing every static check botscript provides: CAP001/CAP002 cannot see capability calls hidden in the string, reads/writes labels are unenforceable, and SYN002/SYN003 checks can be routed around entirely. This is the universal bypass — a fn could use eval('process.env.KEY') with no SYN005, or eval('http.get(...)') with no CAP001. Detection (token-based, same suppression model as SYN002/SYN003): - eval(...) — 'eval' ident not preceded by ./ ?., followed by ( - new Function(...) — 'new' followed by 'Function' followed by ( - .eval() (method call on a local) and Function.* member accesses: excluded - Suppressed inside unsafe {} blocks and unsafe fn bodies Closes #144 (filed as SYN006; implemented as SYN004 — next available code on main, SYN004/SYN005 are still in open PRs). Co-Authored-By: Claude Sonnet 4.6 * fix(syn004): detect bare Function() calls without new; fix SYN005 xref `Function(body)` without `new` is equivalent to `new Function(body)` at runtime and was an easy bypass of SYN004's intent. Now both forms fire. Also fixes Copilot's comment about the explanations.ts cross-reference to SYN005 — reworded to not depend on SYN005 existing on main yet. Co-Authored-By: Claude Sonnet 4.6 * fix(syn004): detect Function?.() optional call; align docs/tests with bare Function() detection - Add `Function?.(` optional call detection in syn-check.ts (mirrors eval?.( handling) - Update error-codes.ts, MCP explanations, AGENTS.md, and test header/describe label to mention bare `Function(...)` / `Function?.(...)` alongside `new Function(...)` — the previous commit added bare Function() detection but left docs still saying only new Function() - Fix "behaviour" → "behavior" in MCP explanation (American English consistency) - Add test for Function?.() optional call form Co-Authored-By: Claude Sonnet 4.6 * fix(syn004): exclude declaration contexts from eval/Function detection; fix message text - eval and Function detection now skip when the paren group is followed by `{`, `:`, or `=>` — catches `function eval(x) {}` and method shorthands that would otherwise false-positive - Warning messages use concrete unsafe block examples (`{ eval(src) }`, `{ new Function(body) }`) instead of the confusing empty `{ }` - error-codes.ts title: "call bypasses" → "calls bypass" (plural) - explanations.ts: clarify Function constructor bullet — not SYN003/Result contract specifically, but all static checks - Two new not-fires tests covering the declaration exclusion Co-Authored-By: Claude Sonnet 4.6 * fix(compiler): SYN004 Function warning suggests correct unsafe-block syntax When the flagged call is bare Function(...) (no `new`), the suppression example in the warning message now matches the detected form. Co-Authored-By: Claude Sonnet 4.6 * fix(syn004): detect TypeScript instantiation forms eval() and Function() Co-Authored-By: Claude Sonnet 4.6 * fix(syn004): remove colon from declaration-exclusion check; add ternary regression tests The ): exclusion was causing false negatives: cond ? eval(x) : y was not firing SYN004 because the ternary colon after eval(x) matched the `:` (return-type annotation) exclusion. The `{` and `=>` exclusions correctly handle all real declaration contexts (method shorthands and function declarations). Removes the `:` case from both eval and Function detection and adds two regression tests for ternary branches. Co-Authored-By: Claude Sonnet 4.6 * fix(syn004): handle return-type-annotated declarations; guard Function ternary then-branch - Add `:` to declaration-exclusion check in both eval and Function blocks, so `function eval(x): T {}` and `{ eval(x): T {} }` are not flagged. - Guard the `:` check with `isTernaryConsequent` in both blocks to avoid suppressing `? eval(x) : y` and `? Function(x) : y` (both are real calls). - Fix grammar in explanations.ts: "bypasses" → "bypass" (plural subject). - Add 4 regression tests: return-type-annotated eval/Function declarations must not fire; Function() in ternary then-branch must fire. Co-Authored-By: Claude Sonnet 4.6 * fix(syn004): add eval?.() test; clarify SYN005 AGENTS.md description - Add missing eval?.(…) optional-call test case (SYN004 fires on this form but it was untested — Function?.() had a test, eval?.() did not) - Clarify SYN005 AGENTS.md row: rephrase from the ambiguous "no declaration depends on deployment config" to the accurate "no capability covers it; fn has an undeclared dependency on deployment configuration" Co-Authored-By: Claude Sonnet 4.6 * fix(syn004): idiom shows content in unsafe block; MCP explanation covers instantiation forms Co-Authored-By: Claude Sonnet 4.6 * fix(syn004): fire on new Function() in ternary then-branch — check token before new When `? new Function(body) : other`, prev4 is `new` (not `?`), so the old isTernaryConsequent4 check evaluated to false and the `:` guard skipped the warning as if it were a return-type annotation. Now also check the token before `new` for `?`. Add regression test. --------- Co-authored-by: Marcelo Farias Co-authored-by: Claude Sonnet 4.6 --- AGENTS.md | 3 +- README.md | 2 +- packages/compiler/src/error-codes.ts | 31 ++ packages/compiler/src/passes/syn-check.ts | 183 +++++++++++- packages/compiler/tests/error-codes.test.ts | 2 +- packages/compiler/tests/syn004-check.test.ts | 288 +++++++++++++++++++ packages/mcp/src/explanations.ts | 37 +++ packages/mcp/tests/server.test.ts | 1 + 8 files changed, 542 insertions(+), 5 deletions(-) create mode 100644 packages/compiler/tests/syn004-check.test.ts diff --git a/AGENTS.md b/AGENTS.md index 4663a03a..c3cf4214 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -198,7 +198,8 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | 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. | | SYN002 | (0.7+, warning) A fn body contains a native `throw` statement. Native throws bypass botscript's Result-based error contract: callers using `?` unwrap or `match` on Result will not observe exceptions raised via `throw`. | Replace `throw new ErrorType(...)` with `return err(new ErrorType(...))` and update the return type to `Result`. | | SYN003 | (0.7+, warning) A fn body contains a `console.*` call (console.log, console.error, etc.). Direct console output bypasses the `stdout`/`stderr` capability model — the compiler cannot enforce or surface the output declaration for callers. | Replace `console.log(...)` with `stdout.write(...)` and add `uses { stdout }` to the fn header; replace `console.error(...)` with `stderr.write(...)` and add `uses { stderr }`. | -| SYN005 | (0.7+, warning) A fn body accesses `process.env`. `process.env` is a global deployment-environment namespace — access is invisible to callers, there is no capability or resource declaration that covers it, and the fn has an undeclared dependency on deployment configuration. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `env`. `obj.process.env` (member access on a local), `unsafe {}` blocks, and `unsafe "reason" fn` bodies are excluded. | Pass config and secrets as explicit fn parameters so the dependency is visible in the call signature; if env access is required at the load site, wrap in `unsafe "reads deployment env" { }`. | +| SYN004 | (0.7+, warning) A fn body calls `eval(...)` / `eval?.(...)` (global eval not preceded by `.`/`?.`) or calls `Function(...)` / `Function?.(...)` / `new Function(...)` (Function constructor not preceded by `.`/`?.`). All forms execute strings as code at runtime — every static capability check (CAP001/CAP002), resource declaration (reads/writes), and safety check (SYN002/SYN003) can be bypassed by routing any unsafe pattern through eval or the Function constructor. Suppressed inside `unsafe {}` blocks and `unsafe fn` bodies. `.eval(...)` (method call on a local) and `Function.*` member accesses are excluded. | Refactor the eval-based pattern to use explicit code paths. If eval is genuinely required (e.g. sandboxed interpreter), wrap in `unsafe "" { eval(...) }`. | +| SYN005 | (0.7+, warning) A fn body accesses `process.env`. `process.env` is a global deployment-environment namespace — access is invisible to callers and to static analysis; no capability or resource declaration covers it, so the fn has an undeclared dependency on deployment configuration. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `env`. `obj.process.env` (member access on a local), `unsafe {}` blocks, and `unsafe "reason" fn` bodies are excluded. | Pass config and secrets as explicit fn parameters so the dependency is visible in the call signature; if env access is required at the load site, wrap in `unsafe "reads deployment env" { }`. | | SYN006 | (0.7+, warning) A fn body calls `process.exit()`, `process?.exit()`, or `process.exit?.()`. All forms terminate the entire host process — not just the fn, not just the bot. They produce no return value, bypass `Result` propagation, `throws {}`, `match`, and any caller recovery path. No capability declaration covers them. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `exit` then `(` or `?.(`. `obj.process.exit(...)`, `process.exit` without `(`, and `process.exitCode` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Return `err(...)` and let the caller decide whether to terminate. If `process.exit` is genuinely required at a bootstrap entry point, wrap in `unsafe "exits on invalid config" { process.exit(1) }`. | | 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. | | INT003 | (0.7+) A fn declares `intent: "idempotent"` but also has `uses { random }` or `uses { time }`. Both capabilities produce different values on each call, making the function non-idempotent. Only `random` and `time` are flagged; other capabilities are not structurally flagged by this check (INT003 is a narrow heuristic, not a proof of idempotence). | Remove `random`/`time` from `uses {}`, or change the intent. | diff --git a/README.md b/README.md index b1cbc6ec..2ce0d9b1 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,7 @@ claude mcp add botscript -- npx -y @mbfarias/botscript-mcp | ----------- | -------------------------------------- | --------------------------------------------------------------------------------------------------- | | `primer` | (no args) | The canonical language primer (same text the `?primer` directive emits). | | `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 (`ALI001`, `ALI002`, `ALI003`, `BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`–`DEP004`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`–`MAT004`, `RES001`, `RES002`, `SYN001`, `SYN002`, `SYN003`, `SYN005`, `SYN006`, `THR001`–`THR004`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. | +| `explain` | `{ code: string }` | Long-form explanation for any stable diagnostic code (`ALI001`, `ALI002`, `ALI003`, `BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`–`DEP004`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`–`MAT004`, `RES001`, `RES002`, `SYN001`, `SYN002`, `SYN003`, `SYN004`, `SYN005`, `SYN006`, `THR001`–`THR004`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. | A bot's loop becomes deterministic: `transform` → if `ok=false`, read `diagnostics[0].code` → `explain(code)` → apply `rewrite` → `transform` again. diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index d331091d..a1c7c21a 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -544,6 +544,37 @@ const E: Record = { " unsafe \"stdout.write returns void\" { stdout.write(`Hello, ${name}`) }\n" + "}", }, + SYN004: { + code: "SYN004", + title: "eval() or Function() / new Function() calls bypass all static capability and syntax checks", + rule: + "`eval(...)`, `Function(...)`, and `new Function(...)` execute strings as code at runtime — " + + "no static analysis can see what they do; every capability check (CAP001/CAP002), " + + "resource declaration (reads/writes), and safety check (SYN002/SYN003) can be bypassed " + + "by routing the unsafe pattern through eval or the Function constructor", + idiom: + "refactor eval-based patterns to use explicit code paths or config parameters; " + + "if eval is unavoidable (e.g. a sandboxed interpreter or intentional scripting surface), " + + "wrap in `unsafe \"\" { eval(...) }` to make the escape hatch visible in the diff", + rewrite: + "// before — eval hides config key access from static analysis\n" + + "fn getConfig(key: string) -> string {\n" + + " return eval('process.env.' + key) // SYN004\n" + + "}\n\n" + + "// after — explicit parameter, no eval\n" + + "fn getConfig(value: string) -> string {\n" + + " return value\n" + + "}", + example: + "// SYN004: eval bypasses all static checks\n" + + "fn run(code: string) -> string {\n" + + " return eval(code)\n" + + "}\n\n" + + "// fix: suppress with unsafe if eval is genuinely needed\n" + + "fn run(code: string) -> string {\n" + + ' return unsafe "evaluates user-provided script in sandbox" { eval(code) }\n' + + "}", + }, SYN005: { code: "SYN005", title: "process.env access is an undeclared deployment environment dependency", diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index eb1c94ed..6e0320bd 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -53,10 +53,11 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const warnings: Diagnostic[] = []; const syn002 = getErrorCode("SYN002")!; const syn003 = getErrorCode("SYN003")!; + const syn004 = getErrorCode("SYN004")!; const syn005 = getErrorCode("SYN005")!; const syn006 = getErrorCode("SYN006")!; - // Collect char-offset ranges where SYN002/SYN003/SYN005/SYN006 are suppressed: + // Collect char-offset ranges where SYN002/SYN003/SYN004/SYN005/SYN006 are suppressed: // 1. `unsafe "reason" { ... }` expression blocks — explicit acknowledgment. // 2. `unsafe "reason" fn` bodies — the entire body is exempt, including any // non-unsafe nested fns declared inside it (matching uns-check's pattern). @@ -70,7 +71,7 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const nesting = computeNesting(program.fns.map((f) => f.decl)); for (const { decl } of program.fns) { - // An `unsafe "reason" fn` body is an explicit acknowledgment — skip SYN002/SYN003/SYN005. + // An `unsafe "reason" fn` body is an explicit acknowledgment — skip SYN002/SYN003/SYN004/SYN005. // The range-based suppression above also covers nested non-unsafe fns within it, // so this early-continue is kept purely as an optimisation. if (decl.unsafeReason !== undefined) continue; @@ -265,6 +266,184 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult }); } + // SYN004: eval() and Function() / new Function() call detection. + // Fires on: + // eval(...) — global eval not preceded by `.`/`?.`, followed by `(` + // Function(…) — bare Function call not preceded by `.`/`?.`, followed by `(` + // new Function(…) — same as bare Function call, `new` prefix only affects message + // Suppressed inside `unsafe { }` blocks and `unsafe fn` bodies. + // `.eval(...)` (method call on a local) and `Function.*` member accesses are NOT flagged. + let nextInner4 = 0; + const open4: typeof inner = []; + for (let i = bodyStart; i < decl.tokenEnd; i++) { + while (open4.length > 0 && open4[open4.length - 1]!.tokenEnd <= i) open4.pop(); + while (nextInner4 < inner.length && inner[nextInner4]!.tokenStart <= i) { + open4.push(inner[nextInner4]!); + nextInner4++; + } + if (open4.length > 0) continue; + + const tok4 = tokens[i]; + if (!tok4 || tok4.kind !== "ident") continue; + + // --- eval(...) detection --- + if (tok4.text === "eval") { + // Exclude: `obj.eval(...)` — preceded by `.` or `?.` + const prevIdx4 = prevSignificant(tokens, i - 1); + const prev4 = tokens[prevIdx4]; + if (prev4 && ((prev4.kind === "punct" && prev4.text === ".") || prev4.kind === "questionDot")) + continue; + + // Must be followed by `(` (direct call), `?.(` (optional call), or `(` (TS instantiation). + const nextIdx4 = nextSignificant(tokens, i + 1); + const next4 = tokens[nextIdx4]; + let isOptEval = false; + let callIdx4 = nextIdx4; + if (next4 && next4.kind === "questionDot") { + isOptEval = true; + callIdx4 = nextSignificant(tokens, nextIdx4 + 1); + } else if (next4 && next4.kind === "operator" && next4.text === "<") { + // TypeScript instantiation form: eval(...) + let depth = 1; + let j = nextIdx4 + 1; + while (j < tokens.length && depth > 0) { + const t = tokens[j]; + if (!t) break; + if (t.kind === "operator" && t.text === "<") depth++; + else if (t.kind === "operator" && (t.text === ">" || t.text === ">>" || t.text === ">>>")) + depth -= t.text.length; + j++; + } + const afterGenericIdx4 = nextSignificant(tokens, j); + const afterGeneric4 = tokens[afterGenericIdx4]; + if (afterGeneric4 && afterGeneric4.kind === "open" && afterGeneric4.text === "(") + callIdx4 = afterGenericIdx4; + } + const callTok4 = tokens[callIdx4]; + if (!callTok4 || !(callTok4.kind === "open" && callTok4.text === "(")) continue; + + // Exclude declarations: `function eval(params) {}`, `{ eval(params) {} }`, and + // return-type-annotated forms `function eval(params): T {}` / `{ eval(params): T {} }`. + // The `:` check is guarded: in a ternary (`? eval(x) : y`), the token before `eval` + // is `?` (question) — that is a call, not a declaration, so `:` is skipped there. + if (callTok4.matchedAt !== undefined) { + const afterCloseIdx = nextSignificant(tokens, callTok4.matchedAt + 1); + const afterClose = tokens[afterCloseIdx]; + const isTernaryConsequent = prev4 && prev4.kind === "question"; + if (afterClose && ( + (afterClose.kind === "open" && afterClose.text === "{") || + afterClose.kind === "fatArrow" || + (!isTernaryConsequent && afterClose.kind === "punct" && afterClose.text === ":") + )) continue; + } + + if (isInsideRange(tok4.start, unsafeRanges)) continue; + + const callSep4 = isOptEval ? "?." : ""; + const loc4 = locationOf(src, tok4.start); + warnings.push({ + code: "SYN004", + severity: "warning", + file: null, + line: loc4.line, + column: loc4.column, + start: tok4.start, + end: callTok4.start + 1, + message: + `fn '${decl.name}' calls eval${callSep4}() — ` + + `eval executes a string as code and bypasses all static capability, ` + + `resource, and safety checks; refactor to explicit code or wrap in unsafe "reason" { eval(src) }`, + rule: syn004.rule, + idiom: syn004.idiom, + rewrite: syn004.rewrite, + }); + continue; + } + + // --- new Function(...) / Function(...) detection --- + // Both `new Function(body)` and bare `Function(body)` execute a string as + // code at runtime and are equivalent bypasses. + if (tok4.text === "Function") { + const prevIdx4 = prevSignificant(tokens, i - 1); + const prev4 = tokens[prevIdx4]; + + // Exclude: `obj.Function(...)` — preceded by `.` or `?.` + if (prev4 && ((prev4.kind === "punct" && prev4.text === ".") || prev4.kind === "questionDot")) + continue; + + // Exclude: `Function.prototype.*` — followed by `.` (member access, not a call) + // Must be followed by `(` (direct call), `?.(` (optional call), or `(` (TS instantiation). + const nextIdx4 = nextSignificant(tokens, i + 1); + const next4 = tokens[nextIdx4]; + let isOptFunc = false; + let callIdx4 = nextIdx4; + if (next4 && next4.kind === "questionDot") { + isOptFunc = true; + callIdx4 = nextSignificant(tokens, nextIdx4 + 1); + } else if (next4 && next4.kind === "operator" && next4.text === "<") { + // TypeScript instantiation form: Function(...) / new Function(...) + let depth = 1; + let j = nextIdx4 + 1; + while (j < tokens.length && depth > 0) { + const t = tokens[j]; + if (!t) break; + if (t.kind === "operator" && t.text === "<") depth++; + else if (t.kind === "operator" && (t.text === ">" || t.text === ">>" || t.text === ">>>")) + depth -= t.text.length; + j++; + } + const afterGenericIdx4 = nextSignificant(tokens, j); + const afterGeneric4 = tokens[afterGenericIdx4]; + if (afterGeneric4 && afterGeneric4.kind === "open" && afterGeneric4.text === "(") + callIdx4 = afterGenericIdx4; + } + const callTok4 = tokens[callIdx4]; + if (!callTok4 || !(callTok4.kind === "open" && callTok4.text === "(")) continue; + + // Exclude declarations: `function Function(params) {}`, method shorthands, and + // return-type-annotated forms `function Function(params): T {}` / `{ Function(params): T {} }`. + // Guard `:` against ternary then-branch: `? Function(body) :` has `?` before Function. + // `? new Function(body) :` has `new` before Function but `?` before `new` — check both. + if (callTok4.matchedAt !== undefined) { + const afterCloseIdx = nextSignificant(tokens, callTok4.matchedAt + 1); + const afterClose = tokens[afterCloseIdx]; + const prevBeforeNew4 = (prev4 && prev4.kind === "ident" && prev4.text === "new") + ? tokens[prevSignificant(tokens, prevIdx4 - 1)] + : undefined; + const isTernaryConsequent4 = (prev4 && prev4.kind === "question") || + (prevBeforeNew4 !== undefined && prevBeforeNew4 !== null && prevBeforeNew4.kind === "question"); + if (afterClose && ( + (afterClose.kind === "open" && afterClose.text === "{") || + afterClose.kind === "fatArrow" || + (!isTernaryConsequent4 && afterClose.kind === "punct" && afterClose.text === ":") + )) continue; + } + + if (isInsideRange(tok4.start, unsafeRanges)) continue; + + const hasNew = prev4 && prev4.kind === "ident" && prev4.text === "new"; + const funcCallSep = isOptFunc ? "?." : ""; + const warnStart = hasNew ? prev4!.start : tok4.start; + const loc4 = locationOf(src, warnStart); + warnings.push({ + code: "SYN004", + severity: "warning", + file: null, + line: loc4.line, + column: loc4.column, + start: warnStart, + end: callTok4.start + 1, + message: + `fn '${decl.name}' constructs ${hasNew ? "new " : ""}Function${funcCallSep}() — ` + + `the Function constructor executes a string as code and bypasses all static checks; ` + + `refactor to explicit code or wrap in unsafe "reason" { ${hasNew ? "new Function(body)" : "Function(body)"} }`, + rule: syn004.rule, + idiom: syn004.idiom, + rewrite: syn004.rewrite, + }); + } + } + // SYN005: process.env access detection. // Fires on any `process.env` access (read or write) — not just calls. // Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `env`. diff --git a/packages/compiler/tests/error-codes.test.ts b/packages/compiler/tests/error-codes.test.ts index 8864a5d4..5b57aae2 100644 --- a/packages/compiler/tests/error-codes.test.ts +++ b/packages/compiler/tests/error-codes.test.ts @@ -15,7 +15,7 @@ describe("error-code registry", () => { "INT001", "INT002", "INT003", "INT004", "INT005", "MAT001", "MAT002", "MAT003", "MAT004", "RES001", "RES002", - "SYN001", "SYN002", "SYN003", "SYN005", "SYN006", + "SYN001", "SYN002", "SYN003", "SYN004", "SYN005", "SYN006", "THR001", "THR002", "THR003", "THR004", "UNS001", "UNS002", "UNS003", "UNS004", "UNS005", "VER001", "VER002", "VER003", diff --git a/packages/compiler/tests/syn004-check.test.ts b/packages/compiler/tests/syn004-check.test.ts new file mode 100644 index 00000000..f58d5ecb --- /dev/null +++ b/packages/compiler/tests/syn004-check.test.ts @@ -0,0 +1,288 @@ +/** + * Tests for SYN004: eval(), Function(), and new Function() call detection (?bs 0.7+). + */ + +import { describe, expect, it } from "vitest"; +import { transform } from "../src/transform.js"; + +describe("SYN004: eval(), Function(), and new Function() checks (0.7+)", () => { + it("fires on eval() call in fn body", () => { + const src = + "?bs 0.7\n" + + "fn run(code: string) -> string {\n" + + " return eval(code)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("fires on eval() with a string literal", () => { + const src = + "?bs 0.7\n" + + "fn run() -> number {\n" + + " return eval('1 + 2')\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("fires on new Function() call", () => { + const src = + "?bs 0.7\n" + + "fn build(body: string) -> Function {\n" + + " return new Function(body)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("fires on new Function() with multiple args", () => { + const src = + "?bs 0.7\n" + + "fn build(a: string, body: string) -> Function {\n" + + " return new Function(a, body)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("does not fire below ?bs 0.7", () => { + const src = + "?bs 0.2\n" + + "fn run(code) {\n" + + " return eval(code)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("does not fire when eval is inside an unsafe block", () => { + const src = + "?bs 0.7\n" + + "fn run(code: string) -> string {\n" + + ' return unsafe "evaluates user script in sandbox" { eval(code) }\n' + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("does not fire when new Function is inside an unsafe block", () => { + const src = + "?bs 0.7\n" + + "fn build(body: string) -> Function {\n" + + ' return unsafe "trusted function factory" { new Function(body) }\n' + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("does not fire when eval is inside an unsafe fn body", () => { + const src = + "?bs 0.7\n" + + 'unsafe "runs untrusted user scripts" fn sandbox(code: string) -> string {\n' + + " return eval(code)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("does not fire on .eval() — method call on a local object", () => { + const src = + "?bs 0.7\n" + + "fn run(vm: { eval: (s: string) -> string }) -> string {\n" + + " return vm.eval('code')\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("does not fire on Function.prototype.call — not new Function()", () => { + const src = + "?bs 0.7\n" + + "fn run(f: Function) -> void {\n" + + " Function.prototype.call(f)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("fires on bare Function() call without new — equivalent runtime bypass", () => { + const src = + "?bs 0.7\n" + + "fn build(body: string) -> unknown {\n" + + " return Function(body)()\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("fires on eval?.() optional call form in fn body", () => { + const src = + "?bs 0.7\n" + + "fn run(code: string) -> unknown {\n" + + " return eval?.(code)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("fires on Function?.() optional call form in fn body", () => { + const src = + "?bs 0.7\n" + + "fn build(body: string) -> unknown {\n" + + " return Function?.(body)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("does not fire on bare Function reference — not called", () => { + const src = + "?bs 0.7\n" + + "fn check(f: unknown) -> boolean {\n" + + " return typeof Function !== 'undefined'\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("does not fire on function declaration named eval — declaration not a call", () => { + const src = + "?bs 0.7\n" + + "fn run(code: string) -> string {\n" + + " function eval(src: string) { return src }\n" + + " return eval(code)\n" + + "}\n"; + const result = transform(src); + // Only one SYN004: the call `eval(code)`, not the declaration `function eval(src) {...}` + const warnings = result.warnings.filter((w) => w.code === "SYN004"); + expect(warnings.length).toBe(1); + }); + + it("does not fire on method shorthand named Function — declaration not a call", () => { + const src = + "?bs 0.7\n" + + "fn run() -> unknown {\n" + + " const obj = { Function(body: string) { return body } }\n" + + " return obj\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("does not fire on function declaration named eval with return-type annotation", () => { + const src = + "?bs 0.7\n" + + "fn run(code: string) -> string {\n" + + " function eval(src: string): string { return src }\n" + + " return code\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("does not fire on method shorthand named eval with return-type annotation", () => { + const src = + "?bs 0.7\n" + + "fn run() -> unknown {\n" + + " const obj = { eval(src: string): string { return src } }\n" + + " return obj\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("does not fire on method shorthand named Function with return-type annotation", () => { + const src = + "?bs 0.7\n" + + "fn run() -> unknown {\n" + + " const obj = { Function(body: string): Function { return () => body } }\n" + + " return obj\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(false); + }); + + it("fires on eval() in a ternary then-branch — not confused with a method signature", () => { + const src = + "?bs 0.7\n" + + "fn run(flag: boolean, code: string) -> string {\n" + + " return flag ? eval(code) : code\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("fires on Function() in a ternary else-branch — not confused with a method signature", () => { + const src = + "?bs 0.7\n" + + "fn run(flag: boolean, body: string) -> unknown {\n" + + " return flag ? body : Function(body)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("fires on Function() in a ternary then-branch — not suppressed by ternary colon", () => { + const src = + "?bs 0.7\n" + + "fn run(flag: boolean, body: string) -> unknown {\n" + + " return flag ? Function(body) : body\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("fires on new Function() in a ternary then-branch — prev is new, not ? directly", () => { + const src = + "?bs 0.7\n" + + "fn run(flag: boolean, body: string) -> unknown {\n" + + " return flag ? new Function(body) : body\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("has severity 'warning' (non-blocking — transform must not throw)", () => { + const src = + "?bs 0.7\n" + + "fn run(code: string) -> string {\n" + + " return eval(code)\n" + + "}\n"; + let result: ReturnType; + expect(() => { result = transform(src); }).not.toThrow(); + const w = result!.warnings.find((w) => w.code === "SYN004"); + expect(w).toBeDefined(); + expect(w!.severity).toBe("warning"); + }); + + it("fires on eval() TypeScript instantiation form", () => { + const src = + "?bs 0.7\n" + + "fn run(code: string) -> unknown {\n" + + " return eval(code)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("fires on new Function() TypeScript instantiation form", () => { + const src = + "?bs 0.7\n" + + "fn build(body: string) -> unknown {\n" + + " return new Function(body)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); + + it("fires on Function() bare instantiation form", () => { + const src = + "?bs 0.7\n" + + "fn build(body: string) -> unknown {\n" + + " return Function(body)\n" + + "}\n"; + const result = transform(src); + expect(result.warnings.some((w) => w.code === "SYN004")).toBe(true); + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index ea3797b3..7191efac 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -739,6 +739,43 @@ export const EXPLANATIONS: Readonly> = { "}\n", }, }, + SYN004: { + code: "SYN004", + title: "eval() or Function() / new Function() bypass all static capability and syntax checks", + body: + "Botscript's safety model relies on static analysis: capability declarations, resource " + + "labels, and syntax checks (SYN002/SYN003) all operate on visible, unchanging source text. " + + "`eval(...)`, `Function(...)`, and `new Function(...)` shatter that foundation — they " + + "execute arbitrary strings as code at runtime, and no static pass can see what those strings will do.\n\n" + + "The risk in bot code is concrete:\n" + + "- `eval('process.env.' + key)` hides env dependencies from callers (invisible to static analysis)\n" + + "- `eval('http.get(...)')` bypasses CAP001 (capability claim not in the fn's header)\n" + + "- `new Function('return process.exit(1)')()` hides arbitrary effects from all static checks (capability, Result contract, and every SYN diagnostic)\n\n" + + "Every other SYN check is weakened by eval: a bot could route any unsafe pattern through " + + "`eval` or the Function constructor to avoid static detection. The capability manifest hash " + + "proves the *source* hasn't changed, not that runtime behavior is bounded.\n\n" + + "**Fix:** refactor the eval-based pattern to use explicit code paths. If the use case " + + "genuinely requires eval-level dynamism (e.g. a sandboxed user-script interpreter), " + + "wrap the call in `unsafe \"\" { eval(src) }` to make the escape hatch visible " + + "in the diff and in code review.\n\n" + + "SYN004 fires at `?bs 0.7+` as a non-blocking warning. Detection is token-based: " + + "`eval` not preceded by `.`/`?.` followed by `(`, `?.(`, or `(`; bare `Function(...)` / " + + "`Function?.(...)` / `new Function(...)` — including TypeScript instantiation forms " + + "`eval(...)`, `Function(...)`, and `new Function(...)` — not preceded by `.`/`?.`. " + + "`.eval(...)` (method call on a local object) and `Function.*` member accesses are excluded.", + example: { + fails: + "?bs 0.7\n" + + "fn run(code: string) -> string {\n" + + " return eval(code)\n" + + "}\n", + passes: + "?bs 0.7\n" + + "fn run(code: string) -> string {\n" + + " return unsafe \"evaluates user-provided script in sandbox\" { eval(code) }\n" + + "}\n", + }, + }, SYN005: { code: "SYN005", title: "process.env access is an undeclared deployment environment dependency", diff --git a/packages/mcp/tests/server.test.ts b/packages/mcp/tests/server.test.ts index 0cec4df6..4f4d3622 100644 --- a/packages/mcp/tests/server.test.ts +++ b/packages/mcp/tests/server.test.ts @@ -75,6 +75,7 @@ describe("botscript-mcp explanations", () => { "SYN001", "SYN002", "SYN003", + "SYN004", "SYN005", "SYN006", "THR001", From d2537cf5aa04ba5777bbd4527cd37ec94097bedf Mon Sep 17 00:00:00 2001 From: Marcelo Bukowski de Farias Date: Thu, 11 Jun 2026 23:35:35 -0300 Subject: [PATCH 10/11] =?UTF-8?q?feat(compiler):=20SYN010=20=E2=80=94=20wa?= =?UTF-8?q?rn=20on=20setTimeout/setInterval/queueMicrotask=20calls=20in=20?= =?UTF-8?q?fn=20bodies=20(=3Fbs=200.7+)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(compiler): SYN010 — warn on setTimeout/setInterval/queueMicrotask calls in fn bodies (?bs 0.7+) - Adds **SYN010**: a new warning that fires when a fn body calls `setTimeout(...)`, `setInterval(...)`, or `queueMicrotask(...)` at `?bs 0.7+` - These globals schedule callbacks that run after the fn returns — any effects inside those callbacks are invisible to callers: no capability declaration, no `writes {}` label, and no `throws {}` entry can cover them - Same bypass class as `fetch` (SYN007) and `WebSocket` (SYN008): real side effects that sidestep the declared capability surface, but deferred rather than immediate - Suppressed inside `unsafe "reason" { }` blocks and `unsafe "reason" fn` bodies - Member calls (`obj.setTimeout(...)`) and bare references (without `(`) are excluded - Note: should be merged after PR #150 (SYN009) lands; SYN010 is the next available code Changes: - error-codes.ts: add SYN010 entry with rule/idiom/rewrite/example - syn-check.ts: add SYN010 detection; add SYN006 + SYN010 to module header comment; fix outdated unsafe-fn skip comment - tests/syn010-check.test.ts: 12 tests covering all three globals, optional-call form, severity, version gating, unsafe suppression, member-call exclusion, bare reference - tests/error-codes.test.ts: add SYN010 to exhaustive allowlist - mcp/src/explanations.ts: add SYN010 long-form explanation with fails/passes examples - mcp/tests/server.test.ts: add SYN010 to KNOWN_CODES list - AGENTS.md: add SYN010 row to diagnostic table - README.md: add SYN010 to explain tool code list * fix(syn010): address Copilot review — hoist TIMER_GLOBALS, fix param name and reason string - Hoist TIMER_GLOBALS to module scope (was re-created per fn, wasteful in hot pass) - Rename `fn` parameter to `callback` in unsafe-fn test (fn is a reserved keyword) - Fix unsafe reason string: "suspends execution" → "schedules deferred effect" (consistent with idiom) Co-Authored-By: Claude Sonnet 4.6 * fix(syn010): address remaining Copilot feedback — false positive on declarations and explanation codes - syn-check: exclude function declarations named setTimeout/setInterval/queueMicrotask (prev significant token is `function`) — these are definitions, not calls - syn-check: exclude object/class method shorthands where after closing `)` is `{` or `:` - tests: add two tests covering function-declaration and object-method-shorthand exclusions - explanations: remove SYN007/SYN008 code references in SYN010 body; reference by name instead to avoid dangling code cross-references until those PRs land Co-Authored-By: Claude Sonnet 4.6 * fix(compiler): address SYN010 Copilot review feedback — exclude fn keyword declarations - syn-check.ts: also skip `fn setTimeout(...) -> void {}` botscript-style nested function declarations (prev token is `keyword`/`fn`), not just JS `function` declarations — prevents false positives on nested fn declarations inside fn bodies - syn010-check.test.ts: add test asserting SYN010 does NOT fire on `fn setTimeout(delay: number) -> void { }` inside a fn body Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Marcelo Farias Co-authored-by: Claude Sonnet 4.6 --- AGENTS.md | 1 + README.md | 2 +- packages/compiler/src/error-codes.ts | 34 ++++ packages/compiler/src/passes/syn-check.ts | 94 ++++++++++- packages/compiler/tests/error-codes.test.ts | 2 +- packages/compiler/tests/syn010-check.test.ts | 168 +++++++++++++++++++ packages/mcp/src/explanations.ts | 38 +++++ packages/mcp/tests/server.test.ts | 1 + 8 files changed, 336 insertions(+), 4 deletions(-) create mode 100644 packages/compiler/tests/syn010-check.test.ts diff --git a/AGENTS.md b/AGENTS.md index c3cf4214..17e7d2e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -201,6 +201,7 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | SYN004 | (0.7+, warning) A fn body calls `eval(...)` / `eval?.(...)` (global eval not preceded by `.`/`?.`) or calls `Function(...)` / `Function?.(...)` / `new Function(...)` (Function constructor not preceded by `.`/`?.`). All forms execute strings as code at runtime — every static capability check (CAP001/CAP002), resource declaration (reads/writes), and safety check (SYN002/SYN003) can be bypassed by routing any unsafe pattern through eval or the Function constructor. Suppressed inside `unsafe {}` blocks and `unsafe fn` bodies. `.eval(...)` (method call on a local) and `Function.*` member accesses are excluded. | Refactor the eval-based pattern to use explicit code paths. If eval is genuinely required (e.g. sandboxed interpreter), wrap in `unsafe "" { eval(...) }`. | | SYN005 | (0.7+, warning) A fn body accesses `process.env`. `process.env` is a global deployment-environment namespace — access is invisible to callers and to static analysis; no capability or resource declaration covers it, so the fn has an undeclared dependency on deployment configuration. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `env`. `obj.process.env` (member access on a local), `unsafe {}` blocks, and `unsafe "reason" fn` bodies are excluded. | Pass config and secrets as explicit fn parameters so the dependency is visible in the call signature; if env access is required at the load site, wrap in `unsafe "reads deployment env" { }`. | | SYN006 | (0.7+, warning) A fn body calls `process.exit()`, `process?.exit()`, or `process.exit?.()`. All forms terminate the entire host process — not just the fn, not just the bot. They produce no return value, bypass `Result` propagation, `throws {}`, `match`, and any caller recovery path. No capability declaration covers them. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `exit` then `(` or `?.(`. `obj.process.exit(...)`, `process.exit` without `(`, and `process.exitCode` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Return `err(...)` and let the caller decide whether to terminate. If `process.exit` is genuinely required at a bootstrap entry point, wrap in `unsafe "exits on invalid config" { process.exit(1) }`. | +| SYN010 | (0.7+, warning) A fn body calls `setTimeout(...)`, `setInterval(...)`, or `queueMicrotask(...)`. These globals schedule callbacks that run after the fn returns — any effects inside those callbacks are invisible to callers: no capability declaration, no `writes {}` label, and no `throws {}` entry can cover them. Detection: identifier not preceded by `.`/`?.`, followed by `(` or `?.(`. Member calls (`obj.setTimeout(...)`) and bare references (without `(`) are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Make the timing explicit: return a `Promise` the caller awaits, or return a teardown function so the caller controls the lifecycle. If a timer is genuinely required, wrap in `unsafe "schedules deferred effect" { setTimeout(...) }`. | | 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. | | INT003 | (0.7+) A fn declares `intent: "idempotent"` but also has `uses { random }` or `uses { time }`. Both capabilities produce different values on each call, making the function non-idempotent. Only `random` and `time` are flagged; other capabilities are not structurally flagged by this check (INT003 is a narrow heuristic, not a proof of idempotence). | Remove `random`/`time` from `uses {}`, or change the intent. | | INT004 | (0.7+) A fn declares `intent: "idempotent"` but its body directly references `random` or `time` without declaring them. Under-declaration variant of INT003 — fires when INT003 does not. | Remove the non-idempotent call from the body, or declare the capability and remove the idempotent intent. | diff --git a/README.md b/README.md index 2ce0d9b1..2901cca9 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,7 @@ claude mcp add botscript -- npx -y @mbfarias/botscript-mcp | ----------- | -------------------------------------- | --------------------------------------------------------------------------------------------------- | | `primer` | (no args) | The canonical language primer (same text the `?primer` directive emits). | | `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 (`ALI001`, `ALI002`, `ALI003`, `BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`–`DEP004`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`–`MAT004`, `RES001`, `RES002`, `SYN001`, `SYN002`, `SYN003`, `SYN004`, `SYN005`, `SYN006`, `THR001`–`THR004`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. | +| `explain` | `{ code: string }` | Long-form explanation for any stable diagnostic code (`ALI001`, `ALI002`, `ALI003`, `BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`–`DEP004`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`–`MAT004`, `RES001`, `RES002`, `SYN001`, `SYN002`, `SYN003`, `SYN004`, `SYN005`, `SYN006`, `SYN010`, `THR001`–`THR004`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. | A bot's loop becomes deterministic: `transform` → if `ok=false`, read `diagnostics[0].code` → `explain(code)` → apply `rewrite` → `transform` again. diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index a1c7c21a..c155911b 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -642,6 +642,40 @@ const E: Record = { " return ok(undefined)\n" + "}", }, + SYN010: { + code: "SYN010", + title: "setTimeout / setInterval / queueMicrotask defers side effects outside the fn's capability surface", + rule: + "`setTimeout(fn, ms)`, `setInterval(fn, ms)`, and `queueMicrotask(fn)` schedule callbacks that run " + + "after the current fn returns — any effects inside those callbacks are invisible to the caller: " + + "no capability declaration, no `writes {}` label, no `throws {}` entry can reflect them. " + + "Callers see a fn that returns normally; the real work happens later, in a different call frame, " + + "with no signal in the fn header.", + idiom: + "pass the delay and callback to the caller as a return value so the timing is visible (e.g. return a Promise " + + "the caller awaits); if a timer is genuinely required here, wrap in " + + "`unsafe \"schedules deferred effect\" { setTimeout(...) }`", + rewrite: + "// before — deferred effect invisible to callers\n" + + "fn scheduleRetry(fn: () -> void, ms: number) -> void {\n" + + " setTimeout(fn, ms) // SYN010\n" + + "}\n\n" + + "// after — caller controls the timing\n" + + "async fn scheduleRetry(fn: () -> void, ms: number) -> Promise {\n" + + " await new Promise(resolve => unsafe \"schedules deferred effect\" { setTimeout(resolve, ms) })\n" + + " fn()\n" + + "}", + example: + "// SYN010: deferred callback hides a network effect from callers\n" + + "fn pollStatus(url: string) uses { net } -> void {\n" + + " setInterval(() => http.get(url), 5000) // SYN010\n" + + "}\n\n" + + "// fix: return a teardown fn so the polling is visible at the call site\n" + + "fn pollStatus(url: string) uses { net } -> () -> void {\n" + + " const id = setInterval(() => http.get(url), 5000)\n" + + " return () => clearInterval(id)\n" + + "}", + }, DEP001: { code: "DEP001", title: "fn transitively reads a resource category not declared in its header", diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index 6e0320bd..f50da198 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -23,6 +23,21 @@ * depends on runtime deployment values that callers cannot see, * audit, or mock in tests. The idiomatic fix is to pass config * and secrets as explicit fn parameters. + * + * SYN006 A `process.exit()` call was detected in a fn body (?bs 0.7+). + * `process.exit()` terminates the entire host process — not just the + * fn, not just the bot. It produces no return value and bypasses + * Result propagation, throws {}, match, and any caller recovery + * path. The idiomatic fix is `return err(...)` so the caller can + * decide whether to terminate. + * + * SYN010 A `setTimeout(...)`, `setInterval(...)`, or `queueMicrotask(...)` + * call was detected in a fn body (?bs 0.7+). These globals schedule + * callbacks to run after the current fn returns — any effects inside + * those callbacks are invisible to callers: no capability declaration, + * no `writes {}` label, and no `throws {}` entry covers them. + * Excluded: member calls (`obj.setTimeout`), function declarations + * named `setTimeout`, and object/class method shorthands. */ import type { Diagnostic } from "../diagnostics.js"; @@ -44,6 +59,8 @@ const CONSOLE_OUTPUT_METHODS = new Set([ "table", "trace", "group", "groupCollapsed", "groupEnd", ]); +const TIMER_GLOBALS = new Set(["setTimeout", "setInterval", "queueMicrotask"]); + export function passSynCheck(src: string, version: VersionInfo): SynCheckResult { if (!atLeast(version.resolved, "0.7")) return { code: src, warnings: [] }; @@ -56,8 +73,9 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const syn004 = getErrorCode("SYN004")!; const syn005 = getErrorCode("SYN005")!; const syn006 = getErrorCode("SYN006")!; + const syn010 = getErrorCode("SYN010")!; - // Collect char-offset ranges where SYN002/SYN003/SYN004/SYN005/SYN006 are suppressed: + // Collect char-offset ranges where all SYN checks are suppressed: // 1. `unsafe "reason" { ... }` expression blocks — explicit acknowledgment. // 2. `unsafe "reason" fn` bodies — the entire body is exempt, including any // non-unsafe nested fns declared inside it (matching uns-check's pattern). @@ -71,7 +89,7 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const nesting = computeNesting(program.fns.map((f) => f.decl)); for (const { decl } of program.fns) { - // An `unsafe "reason" fn` body is an explicit acknowledgment — skip SYN002/SYN003/SYN004/SYN005. + // An `unsafe "reason" fn` body is an explicit acknowledgment — all SYN checks are skipped. // The range-based suppression above also covers nested non-unsafe fns within it, // so this early-continue is kept purely as an optimisation. if (decl.unsafeReason !== undefined) continue; @@ -575,6 +593,78 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult rewrite: syn006.rewrite, }); } + + // SYN010: setTimeout / setInterval / queueMicrotask detection. + // Fires when a fn body calls any of these timer/microtask globals, which schedule + // callbacks to run after the fn returns. Any side effects inside those callbacks are + // invisible to callers: no capability, writes {}, or throws {} can reflect them. + // Suppressed inside `unsafe "reason" { }` blocks and `unsafe "reason" fn` bodies. + nextInner = 0; + const open010: typeof inner = []; + for (let i = bodyStart; i < decl.tokenEnd; i++) { + while (open010.length > 0 && open010[open010.length - 1]!.tokenEnd <= i) open010.pop(); + while (nextInner < inner.length && inner[nextInner]!.tokenStart <= i) { + open010.push(inner[nextInner]!); + nextInner++; + } + if (open010.length > 0) continue; + + const tok10 = tokens[i]; + if (!tok10 || tok10.kind !== "ident" || !TIMER_GLOBALS.has(tok10.text)) continue; + + // Exclude property accesses: obj.setTimeout(...) + const prevIdx10 = prevSignificant(tokens, i - 1); + const prev10 = tokens[prevIdx10]; + if (prev10 && ((prev10.kind === "punct" && prev10.text === ".") || prev10.kind === "questionDot")) + continue; + + // Exclude function declarations: function setTimeout(fn, ms) {} or fn setTimeout(...) -> void {} + if (prev10 && prev10.kind === "ident" && prev10.text === "function") continue; + if (prev10 && prev10.kind === "keyword" && prev10.text === "fn") continue; + + // Must be followed by `(` or `?.(` — confirming this is a call, not a reference. + let afterIdx10 = nextSignificant(tokens, i + 1); + let afterTok10 = tokens[afterIdx10]; + if (afterTok10 && afterTok10.kind === "questionDot") { + afterIdx10 = nextSignificant(tokens, afterIdx10 + 1); + afterTok10 = tokens[afterIdx10]; + } + if (!afterTok10 || !(afterTok10.kind === "open" && afterTok10.text === "(")) continue; + + // Exclude method shorthands and class methods: { setTimeout(fn) { ... } } + // When after the closing `)` is `{` (method body) or `:` (return type), it's a definition. + const closeParenIdx10 = afterTok10.matchedAt; + if (closeParenIdx10 !== undefined) { + const afterParenIdx10 = nextSignificant(tokens, closeParenIdx10 + 1); + const afterParen10 = tokens[afterParenIdx10]; + if ( + afterParen10 && + ((afterParen10.kind === "open" && afterParen10.text === "{") || + (afterParen10.kind === "punct" && afterParen10.text === ":")) + ) continue; + } + + if (isInsideRange(tok10.start, unsafeRanges)) continue; + + const loc10 = locationOf(src, tok10.start); + warnings.push({ + code: "SYN010", + severity: "warning", + file: null, + line: loc10.line, + column: loc10.column, + start: tok10.start, + end: tok10.end, + message: + `fn '${decl.name}' calls ${tok10.text}() — ` + + `${tok10.text} schedules a callback that runs after the fn returns; ` + + `any effects inside that callback are invisible to callers and cannot be declared in the fn header; ` + + `wrap in unsafe "schedules deferred effect" { ${tok10.text}(...) }`, + rule: syn010.rule, + idiom: syn010.idiom, + rewrite: syn010.rewrite, + }); + } } return { code: src, warnings }; diff --git a/packages/compiler/tests/error-codes.test.ts b/packages/compiler/tests/error-codes.test.ts index 5b57aae2..b2f61184 100644 --- a/packages/compiler/tests/error-codes.test.ts +++ b/packages/compiler/tests/error-codes.test.ts @@ -15,7 +15,7 @@ describe("error-code registry", () => { "INT001", "INT002", "INT003", "INT004", "INT005", "MAT001", "MAT002", "MAT003", "MAT004", "RES001", "RES002", - "SYN001", "SYN002", "SYN003", "SYN004", "SYN005", "SYN006", + "SYN001", "SYN002", "SYN003", "SYN004", "SYN005", "SYN006", "SYN010", "THR001", "THR002", "THR003", "THR004", "UNS001", "UNS002", "UNS003", "UNS004", "UNS005", "VER001", "VER002", "VER003", diff --git a/packages/compiler/tests/syn010-check.test.ts b/packages/compiler/tests/syn010-check.test.ts new file mode 100644 index 00000000..0e771edf --- /dev/null +++ b/packages/compiler/tests/syn010-check.test.ts @@ -0,0 +1,168 @@ +/** + * Tests for SYN010: setTimeout / setInterval / queueMicrotask call detection (?bs 0.7+). + * + * SYN010 is a non-blocking warning — transform must not throw. + */ + +import { describe, it, expect } from "vitest"; +import { transform } from "../src/index.js"; + +function compile(src: string) { + return transform(src, {}); +} + +describe("SYN010: timer / microtask global call detection", () => { + it("fires on setTimeout() inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + " setTimeout(fn, 1000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(true); + }); + + it("fires on setInterval() inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn poll(fn: () -> void) -> void {\n" + + " setInterval(fn, 5000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(true); + }); + + it("fires on queueMicrotask() inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn defer(fn: () -> void) -> void {\n" + + " queueMicrotask(fn)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(true); + }); + + it("fires on optional-call form setTimeout?.()", () => { + const src = + "?bs 0.7\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + " setTimeout?.(fn, 1000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(true); + }); + + it("produces a warning-severity diagnostic", () => { + const src = + "?bs 0.7\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + " setTimeout(fn, 1000)\n" + + "}\n"; + const result = compile(src); + const w = result.warnings.find((w) => w.code === "SYN010"); + expect(w?.severity).toBe("warning"); + }); + + it("diagnostic message names the timer global", () => { + const src = + "?bs 0.7\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + " setInterval(fn, 5000)\n" + + "}\n"; + const result = compile(src); + const w = result.warnings.find((w) => w.code === "SYN010"); + expect(w?.message).toContain("setInterval"); + }); + + it("does NOT fire below ?bs 0.7", () => { + const src = + "?bs 0.6\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + " setTimeout(fn, 1000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire inside an unsafe block", () => { + const src = + "?bs 0.7\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + ' unsafe "schedules deferred effect" { setTimeout(fn, 1000) }\n' + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire inside an unsafe fn body", () => { + const src = + "?bs 0.7\n" + + 'unsafe "uses timers" fn scheduleRetry(callback: () -> void) -> void {\n' + + " setTimeout(callback, 1000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire on obj.setTimeout() (member call on a local)", () => { + const src = + "?bs 0.7\n" + + "fn test(ctx: any) -> void {\n" + + " ctx.setTimeout(fn, 1000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire on bare setTimeout reference (not called)", () => { + const src = + "?bs 0.7\n" + + "fn test() -> any {\n" + + " return setTimeout\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("fires once per distinct timer call", () => { + const src = + "?bs 0.7\n" + + "fn scheduleAll(fn: () -> void) -> void {\n" + + " setTimeout(fn, 100)\n" + + " setInterval(fn, 1000)\n" + + " queueMicrotask(fn)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.filter((w) => w.code === "SYN010").length).toBe(3); + }); + + it("does NOT fire on function declarations named setTimeout inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn test(callback: () -> void, ms: number) -> void {\n" + + " function setTimeout(cb: () -> void, delay: number) -> void {}\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire on botscript fn declarations named setTimeout inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn test(callback: () -> void, ms: number) -> void {\n" + + " fn setTimeout(delay: number) -> void { }\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire on object method shorthands named setTimeout", () => { + const src = + "?bs 0.7\n" + + "fn test(callback: () -> void) -> any {\n" + + " return { setTimeout(cb: () -> void, ms: number) { callback() } }\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 7191efac..1aa2c9d7 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -849,6 +849,44 @@ export const EXPLANATIONS: Readonly> = { "}\n", }, }, + SYN010: { + code: "SYN010", + title: "setTimeout / setInterval / queueMicrotask defers side effects outside the fn's capability surface", + body: + "Botscript's capability model is static: the compiler reads the fn header and infers what that fn " + + "may do — network access, resource reads/writes, error types. Timer and microtask globals break this " + + "contract by scheduling work to run *after* the fn returns.\n\n" + + "When a fn calls `setTimeout(() => http.get(url), 5000)`, it has a live network dependency — but " + + "that dependency runs in a callback that fires in a future event-loop tick. The fn returns `void` " + + "(or a timer ID) immediately. No `uses { net }` declaration in the header can cover it, because the " + + "capability lives in the deferred callback, not in the fn's direct call graph.\n\n" + + "The impact in bot code is concrete:\n" + + "- `setTimeout(() => db.write(state), 0)` — a write that runs after the fn returns; callers cannot observe it\n" + + "- `setInterval(() => pollApi(), 60_000)` — a recurring effect started by the fn; the caller has no teardown handle\n" + + "- `queueMicrotask(() => emitEvent())` — a microtask-queued side effect, invisible to the static analysis\n\n" + + "This is the same bypass class as `fetch()` and `WebSocket` globals: real effects that " + + "sidestep the declared capability surface, but deferred rather than immediate.\n\n" + + "**Fix:** make the timing explicit. If the fn needs to delay work, return a `Promise` the caller " + + "awaits, or return a teardown function the caller can control. If a timer is genuinely required, " + + "wrap in `unsafe \"schedules deferred effect\" { setTimeout(...) }` to make the escape hatch visible.\n\n" + + "SYN010 fires at `?bs 0.7+` as a non-blocking warning. Detection is token-based: `setTimeout`, " + + "`setInterval`, or `queueMicrotask` not preceded by `.`/`?.`, followed by `(` or `?.(`. " + + "Member calls (`obj.setTimeout(...)`) are excluded. " + + "Calls inside `unsafe { }` blocks or `unsafe \"reason\" fn` bodies are suppressed.", + example: { + fails: + "?bs 0.7\n" + + "fn pollStatus(url: string) uses { net } -> void {\n" + + " setInterval(() => http.get(url), 5000)\n" + + "}\n", + passes: + "?bs 0.7\n" + + "fn pollStatus(url: string) uses { net } -> () -> void {\n" + + " const id = unsafe \"schedules polling\" { setInterval(() => http.get(url), 5000) }\n" + + " return () => clearInterval(id)\n" + + "}\n", + }, + }, DEP001: { code: "DEP001", title: "fn transitively reads a resource category not declared in its header", diff --git a/packages/mcp/tests/server.test.ts b/packages/mcp/tests/server.test.ts index 4f4d3622..41991681 100644 --- a/packages/mcp/tests/server.test.ts +++ b/packages/mcp/tests/server.test.ts @@ -78,6 +78,7 @@ describe("botscript-mcp explanations", () => { "SYN004", "SYN005", "SYN006", + "SYN010", "THR001", "THR002", "THR003", From 6e214389875c12d2b1210b70d9bc22b953da6852 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Fri, 12 Jun 2026 03:38:44 -0300 Subject: [PATCH 11/11] docs(syn-check): add SYN004 to module header comment SYN004 (eval/new Function detection) was implemented in this pass but omitted from the top-of-file diagnostic table. Add the missing entry so the header accurately lists all SYN checks performed by this pass. Co-Authored-By: Botkowski --- packages/compiler/src/passes/syn-check.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index d1820d02..3f7c7c8b 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -16,6 +16,13 @@ * is explicit in the fn's `uses { stdout }` / `uses { stderr }` * clause and visible to callers. * + * SYN004 An `eval()` or `new Function()` call was detected in a fn body (?bs 0.7+). + * `eval` and `Function` construct and execute arbitrary code at runtime, + * bypassing every static guarantee botscript provides: capability checks, + * result typing, throws declarations, and the entire diagnostic model. + * The idiomatic fix is to restructure code to avoid dynamic evaluation; + * if the raw call is genuinely required, wrap in `unsafe`. + * * SYN005 A `process.env` access was detected in a fn body (?bs 0.7+). * `process.env` is a global deployment-environment namespace. Reads * and writes to it are invisible to callers — no capability or