diff --git a/AGENTS.md b/AGENTS.md index 6bb47fdb..8bbd50ef 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -198,8 +198,11 @@ 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) }`. | +| 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(...) }`. | +| 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. | @@ -213,6 +216,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..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`, `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`, `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 aa8aa867..c1713da1 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", @@ -611,6 +642,81 @@ 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" + + "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" + + "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" + + " }\n" + + "}", + example: + "// SYN007: fetch bypasses the net capability model\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" + + "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" + + " }\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", @@ -669,6 +775,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/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index eb1c94ed..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 @@ -23,6 +30,29 @@ * 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. + * + * 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. + * + * 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 +74,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: [] }; @@ -53,10 +85,13 @@ 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")!; + const syn007 = getErrorCode("SYN007")!; + const syn010 = getErrorCode("SYN010")!; - // Collect char-offset ranges where SYN002/SYN003/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). @@ -70,7 +105,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; @@ -265,6 +300,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`. @@ -396,6 +609,174 @@ 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, + }); + } + + // 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`. + // 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; + + if (afterFetch.kind === "operator" && afterFetch.text === "<") { + // TypeScript instantiation form: `fetch(...)` — skip over `<...>` to find `(` + let anglDepth = 1; + let j = afterFetchIdx + 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; + j++; + } + afterFetchIdx = nextSignificant(tokens, j); + } else if (afterFetch.kind === "questionDot") { + // `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[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 + // tokens[afterFetchIdx] 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 + 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/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..114c475c 100644 --- a/packages/compiler/tests/error-codes.test.ts +++ b/packages/compiler/tests/error-codes.test.ts @@ -9,13 +9,13 @@ 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", "MAT001", "MAT002", "MAT003", "MAT004", "RES001", "RES002", - "SYN001", "SYN002", "SYN003", "SYN005", "SYN006", + "SYN001", "SYN002", "SYN003", "SYN004", "SYN005", "SYN006", "SYN007", "SYN010", "THR001", "THR002", "THR003", "THR004", "UNS001", "UNS002", "UNS003", "UNS004", "UNS005", "VER001", "VER002", "VER003", 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/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/compiler/tests/syn007-check.test.ts b/packages/compiler/tests/syn007-check.test.ts new file mode 100644 index 00000000..ea5d73e1 --- /dev/null +++ b/packages/compiler/tests/syn007-check.test.ts @@ -0,0 +1,186 @@ +/** + * 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("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" + + "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() 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" + + "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"); + }); + + 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 object-literal method with return-type annotation inside a fn body", () => { + 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); + }); + + 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); + }); +}); 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/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..0cb666c9 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", @@ -812,6 +849,86 @@ 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 `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: " + + "`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" + + "async fn loadData(url: string) -> Promise {\n" + + " const res = await fetch(url)\n" + + " return await res.text()\n" + + "}\n", + passes: + "?bs 0.7\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" + + " }\n" + + "}\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", @@ -822,9 +939,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 +1141,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 +1154,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..38ebfe5c 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", @@ -73,8 +75,11 @@ describe("botscript-mcp explanations", () => { "SYN001", "SYN002", "SYN003", + "SYN004", "SYN005", "SYN006", + "SYN007", + "SYN010", "THR001", "THR002", "THR003",