feat(compiler): DEP003/DEP004 — warn when reads/writes labels are over-declared (?bs 0.9+)

AGENTS.md

@@ -205,6 +205,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 same-file callee (direct or transitive) also declares `reads { x }`. The annotation is likely stale — either a refactor removed the callee that justified it, or the fn is itself the access point. 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 same-file callee justifies. 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. |

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 (`BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`, `DEP002`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`, `RES001`, `SYN001`, `THR001`–`THR003`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. |
+| `explain`   | `{ code: string }`                     | Long-form explanation for any stable diagnostic code (`BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`–`DEP004`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`, `RES001`, `SYN001`, `THR001`–`THR003`, `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.

packages/compiler/src/error-codes.ts

@@ -453,6 +453,48 @@ const E: Record<string, ErrorCodeEntry> = {
       "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 callee in the same file (warning)",
+    rule:
+      "a declared reads {} label should reflect a resource the fn or its callees actually access; " +
+      "if no same-file callee (transitively) declares reads { x }, the label may be stale after a refactor",
+    idiom:
+      "remove the stale label from the reads {} clause, or verify that the fn itself directly accesses the resource; " +
+      "leaf fns that are the actual access point can safely declare the label even if no callee propagates it",
+    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 callee in the same file (warning)",
+    rule:
+      "a declared writes {} label should reflect a resource the fn or its callees actually modify; " +
+      "if no same-file callee (transitively) declares writes { x }, the label may be stale after a refactor",
+    idiom:
+      "remove the stale label from the writes {} clause, or verify that the fn itself directly modifies the resource; " +
+      "leaf fns that are the actual write point can safely declare the label even if no callee propagates it",
+    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",

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;

packages/compiler/src/passes/_callgraph.ts

@@ -87,64 +87,133 @@ export function collectCallees(
   return callees;
 }
 
-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`.
+ *
+ * `cap-check.ts` exports `STDLIB_TO_CAP` whose keys must exactly match this set.
+ * Import from here rather than duplicating the list.
+ */
+export const STDLIB_NAMESPACES = new Set(["http", "time", "random", "fs", "stdout", "stderr"]);
+
+/**
+ * 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",
+]);
+
+
+/**
+ * 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<string> {
+  const names = new Set<string>();
+  let parenDepth = 0;
+  let braceDepth = 0;
+  let i = 0;
+  while (i < args.length) {
+    const c = args[i]!;
+    if (c === "(") { parenDepth++; i++; continue; }
+    if (c === ")") { parenDepth--; i++; continue; }
+    if (c === "{") { braceDepth++; i++; continue; }
+    if (c === "}") { braceDepth--; i++; continue; }
+    if (parenDepth !== 1 || braceDepth > 0) { i++; continue; }
+    const m = /^([a-zA-Z_$][a-zA-Z0-9_$]*)\s*:/.exec(args.slice(i));
+    if (m) {
+      names.add(m[1]!);
+      i += m[0].length;
+    } else {
       i++;
-      continue;
     }
-    return 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--;
-      continue;
+/**
+ * Collect names of `const`/`let` simple-binding variables declared in `fn`'s
+ * body (excluding tokens inside nested fn declarations).
+ *
+ * Only plain `const name = ...` and `let name = ...` forms are collected —
+ * destructuring patterns are intentionally skipped. The result is used as
+ * the `localNames` set for `hasOpaqueCall` so that method calls on local
+ * variables (e.g. `name.trim()`) are not mistaken for opaque import calls.
+ */
+export function collectFnBodyLocalNames(
+  tokens: Token[],
+  fn: FnDecl,
+  inner: FnDecl[],
+): Set<string> {
+  const names = new Set<string>();
+  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++;
     }
-    return i;
+    if (open.length > 0) continue;
+
+    const tok = tokens[i];
+    if (!tok || tok.kind !== "ident") continue;
+    if (tok.text !== "const" && tok.text !== "let") continue;
+
+    const nameIdx = nextSignificant(tokens, i + 1);
+    const nameTok = tokens[nameIdx];
+    // Only simple `const name` bindings — skip destructuring (`{`, `[`)
+    if (nameTok && nameTok.kind === "ident") names.add(nameTok.text);
   }
-  return i;
-}
 
-// Alias imported for opaque-call detection; derived from imports.ts to avoid drift.
-const BOTSCRIPT_BUILTIN_CALLS = STDLIB_VALUE_CALL_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<string>,
+  knownNames: Set<string>,
+  localNames?: ReadonlySet<string>,
 ): 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<string> =
+    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;
 
@@ -158,27 +227,126 @@ 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 `(` to be a call.
     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 optional member: `db?.read(...)`
+        if (STDLIB_NAMESPACES.has(tok.text)) continue;
+        if (effectiveLocalNames.has(tok.text)) continue;
+        const afterMethodIdx = nextSignificant(tokens, afterDotIdx + 1);
+        const afterMethod = tokens[afterMethodIdx];
+        // Direct call: `db.read(...)` or optional call: `db.read?.(...)`
+        const isMethodCall =
+          (afterMethod?.kind === "open" && afterMethod.text === "(") ||
+          (afterMethod?.kind === "questionDot" &&
+            tokens[nextSignificant(tokens, afterMethodIdx + 1)]?.kind === "open" &&
+            tokens[nextSignificant(tokens, afterMethodIdx + 1)]?.text === "(");
+        if (isMethodCall) return true; // Opaque namespace/object method call
+        continue; // Property access without a following call — not opaque
+      }
+
+      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;
+}
+