From cdbbdf97af310ebff13ba7b585d3a035b3e694a5 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Tue, 19 May 2026 21:07:09 -0300 Subject: [PATCH 1/2] fix(dep-check): restrict collectCallees scan to body tokens only collectCallees was scanning from fn.tokenStart (the `unsafe`/`fn` keyword) through tokenEnd, so idents in the parameter list and return-type annotation could be mistaken for body calls. A parameter default like: fn caller(x: string = helper()) -> string = x would cause DEP001 to fire on `caller` for missing `reads { cache }` even though `helper()` is evaluated at the call site, not in the body. Fix: add `bodyTokenStart` to FnDecl (the token index of `{` or `=` that opens the body) and start the collectCallees scan there instead of fn.tokenStart. Closes #70. Co-Authored-By: Botkowski --- packages/compiler/src/parser/parse-fn.ts | 8 ++++++++ packages/compiler/src/passes/dep-check.ts | 2 +- packages/compiler/tests/dep-check.test.ts | 24 +++++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/compiler/src/parser/parse-fn.ts b/packages/compiler/src/parser/parse-fn.ts index fbe7ff5..4328872 100644 --- a/packages/compiler/src/parser/parse-fn.ts +++ b/packages/compiler/src/parser/parse-fn.ts @@ -102,6 +102,13 @@ export interface FnDecl { /** Source offset of the unsafe reason string token (UTF-16 code units, inclusive). Used to anchor UNS002 at the right location. */ unsafeReasonStart?: number; returnType: string; + /** + * Token-array index of the first body token (`{` for block bodies, `=` for + * expression bodies). Effect-check passes use this to scan only from the body + * start rather than from `tokenStart`, avoiding false matches on idents in the + * parameter list or return-type annotation. + */ + bodyTokenStart: number; /** Body is a brace block OR a single-expression form (= pure / io / arbitrary). */ body: FnBody; } @@ -489,6 +496,7 @@ export function parseFn( unsafeReason, unsafeReasonStart, returnType, + bodyTokenStart: typeEnd, body, }; } diff --git a/packages/compiler/src/passes/dep-check.ts b/packages/compiler/src/passes/dep-check.ts index 34ceb7b..85120bf 100644 --- a/packages/compiler/src/passes/dep-check.ts +++ b/packages/compiler/src/passes/dep-check.ts @@ -219,7 +219,7 @@ function collectCallees( const open: FnDecl[] = []; let nextInner = 0; - for (let i = fn.tokenStart; i < fn.tokenEnd; i++) { + for (let i = fn.bodyTokenStart; 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]!); diff --git a/packages/compiler/tests/dep-check.test.ts b/packages/compiler/tests/dep-check.test.ts index e706a85..8ad0b6b 100644 --- a/packages/compiler/tests/dep-check.test.ts +++ b/packages/compiler/tests/dep-check.test.ts @@ -191,3 +191,27 @@ describe("recursive fns", () => { expect(() => compile(src)).not.toThrow(); }); }); + +// --------------------------------------------------------------------------- +// Parameter-default / return-type false-positive regression (issue #70) +// --------------------------------------------------------------------------- + +describe("parameter default and return-type exclusion", () => { + it("does not fire DEP001 when callee appears only in a parameter default, not the body", () => { + // `helper` is called in the parameter default of `caller` (evaluated at the + // call site), not in caller's body. collectCallees must not pick it up. + const src = + "?bs 0.9\n" + + "fn helper() reads { cache } -> string = \"x\"\n" + + "fn caller(x: string = helper()) -> string = x\n"; + expect(() => compile(src)).not.toThrow(); + }); + + it("still fires DEP001 when callee is called inside the body", () => { + const src = + "?bs 0.9\n" + + "fn helper() reads { cache } -> string = \"x\"\n" + + "fn caller() -> string { helper() }\n"; + expect(() => compile(src)).toThrow("DEP001"); + }); +}); From f71671ca516dd4ec4cbf7a35f32e17d0019e1ddc Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Wed, 20 May 2026 01:06:13 -0300 Subject: [PATCH 2/2] fix: address Copilot review comments on PR #71 - Make bodyTokenStart optional in FnDecl to avoid breaking downstream code that constructs FnDecl values structurally (mocks, tooling); add ?? fn.tokenStart fallback in collectCallees - Rename test describe block to accurately reflect parameter-default-only coverage; add comment explaining return-type exclusion is implicit Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/parser/parse-fn.ts | 6 +++++- packages/compiler/src/passes/dep-check.ts | 2 +- packages/compiler/tests/dep-check.test.ts | 8 ++++++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/compiler/src/parser/parse-fn.ts b/packages/compiler/src/parser/parse-fn.ts index 4328872..afbda91 100644 --- a/packages/compiler/src/parser/parse-fn.ts +++ b/packages/compiler/src/parser/parse-fn.ts @@ -107,8 +107,12 @@ export interface FnDecl { * expression bodies). Effect-check passes use this to scan only from the body * start rather than from `tokenStart`, avoiding false matches on idents in the * parameter list or return-type annotation. + * + * Optional so that code constructing `FnDecl` values outside the parser (e.g. + * tests, mocks) is not forced to populate this field. Consumers should fall + * back to `tokenStart` when absent: `fn.bodyTokenStart ?? fn.tokenStart`. */ - bodyTokenStart: number; + bodyTokenStart?: number; /** Body is a brace block OR a single-expression form (= pure / io / arbitrary). */ body: FnBody; } diff --git a/packages/compiler/src/passes/dep-check.ts b/packages/compiler/src/passes/dep-check.ts index 85120bf..38487db 100644 --- a/packages/compiler/src/passes/dep-check.ts +++ b/packages/compiler/src/passes/dep-check.ts @@ -219,7 +219,7 @@ function collectCallees( const open: FnDecl[] = []; let nextInner = 0; - for (let i = fn.bodyTokenStart; i < fn.tokenEnd; i++) { + for (let i = fn.bodyTokenStart ?? fn.tokenStart; 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]!); diff --git a/packages/compiler/tests/dep-check.test.ts b/packages/compiler/tests/dep-check.test.ts index 8ad0b6b..688f2cf 100644 --- a/packages/compiler/tests/dep-check.test.ts +++ b/packages/compiler/tests/dep-check.test.ts @@ -193,10 +193,14 @@ describe("recursive fns", () => { }); // --------------------------------------------------------------------------- -// Parameter-default / return-type false-positive regression (issue #70) +// Parameter-default false-positive regression (issue #70) +// collectCallees now starts from bodyTokenStart, skipping both the parameter +// list (including defaults) and the return-type annotation. The return-type +// exclusion is implicitly covered by the same mechanism — botscript return +// types don't support call-syntax idents, so no separate test is needed. // --------------------------------------------------------------------------- -describe("parameter default and return-type exclusion", () => { +describe("parameter-default exclusion (issue #70)", () => { it("does not fire DEP001 when callee appears only in a parameter default, not the body", () => { // `helper` is called in the parameter default of `caller` (evaluated at the // call site), not in caller's body. collectCallees must not pick it up.