diff --git a/compiler/src/codegen/collect.ts b/compiler/src/codegen/collect.ts index 280fd010a..e8e0b35bd 100644 --- a/compiler/src/codegen/collect.ts +++ b/compiler/src/codegen/collect.ts @@ -187,6 +187,32 @@ export function collectFunctions(fileAST: FileAST): Node[] { result.push(node); } if (Array.isArray(node.children)) visit(node.children); + // §17.1.1 if-chain — THE FUNCTION-DEFINITION LIMB, and the one that + // decides whether a function declared inside a branch EXISTS at all. + // + // `collapseIfChains` (ast-builder.js) rewrites an `if=`/`else-if=`/`else` + // chain WITH an else arm into `{kind:"if-chain", branches:[{condition, + // element}], elseBranch}`. The `node.children` descent above reaches + // neither: `branches` holds `{condition, element}` RECORDS, and + // `elseBranch` is a single object. So a `${ function helper() {…} }` + // inside a branch was never collected, emit-functions.ts emitted ZERO + // definitions, and every call site shipped as a BARE `helper()` — + // `ReferenceError` at exit 0 with zero diagnostics. The lone-`if=` twin of + // the same source emitted one `_scrml_helper_N` definition and four + // mangled calls. + // + // ⛑ THIS WALK IS DOWNSTREAM OF ROUTE INFERENCE, AND THAT ORDER IS LOAD- + // BEARING. `analyze.ts` hands this result to BOTH emit-functions.ts (the + // client emitter) and emit-server.ts. The client emitter omits a + // `server fn` body only because RI claimed the function as an endpoint — + // and RI's own walk (`route-inference.ts` `collectFileFunctions`) was + // blind to this same shape. Closing THIS walk while that one stayed blind + // shipped the `server fn` BODY into `client.js` with no `server.js` at + // all: measured body-in-client 1 with this hunk alone, 0 without it, 0 on + // base. Both walks are closed together and + // `g-if-chain-branch-cell-never-wired.test.js`'s LEAK GUARD is what keeps + // them that way. Child SHAPE via `ifChainChildNodes` (ast-if-chain.js). + for (const branchBody of ifChainChildNodes(node)) visit([branchBody]); } } visit(nodes); diff --git a/compiler/src/route-inference.ts b/compiler/src/route-inference.ts index 52c433985..688723c81 100644 --- a/compiler/src/route-inference.ts +++ b/compiler/src/route-inference.ts @@ -89,6 +89,7 @@ import { filePrintBuiltinsShadowed } from "./codegen/log-loc.ts"; // name-extractor rather than re-hand-rolling it. Cycle-safe: type-system's // direct deps do not import route-inference. import { isDestructurePattern, iterDestructuredNames } from "./type-system.ts"; +import { ifChainChildNodes } from "./ast-if-chain.js"; // --------------------------------------------------------------------------- // RI-internal types @@ -1107,6 +1108,33 @@ export function collectFileFunctions(fileAST: FileAST): FunctionDeclNode[] { if ("children" in node && Array.isArray((node as any).children)) { visitNodes((node as any).children); } + // §17.1.1 if-chain — THE SERVER-BOUNDARY LIMB, and the one that decides + // whether a `server fn` declared inside a branch ever becomes a ROUTE. + // + // `collapseIfChains` (ast-builder.js) rewrites an `if=`/`else-if=`/`else` + // chain WITH an else arm into `{kind:"if-chain", branches:[{condition, + // element}], elseBranch}`. The `children` descent above reaches neither: + // `branches` holds `{condition, element}` RECORDS, and `elseBranch` is a + // single object. So RI never saw the declaration and never built a route + // for it — which is what leaves `route.boundary` unset, and it is + // `route.boundary === "server"` (checked in `codegen/emit-functions.ts`) + // that keeps a `server fn` body out of `client.js`. NOTE: this is NOT + // `endpointClientSkipIds` — that set is built solely from `` + // private-arm reachability and carries no `server fn` placement path. + // (An earlier draft of this comment named it; a reader who follows that + // pointer finds no `server fn` path there and wrongly concludes this walk + // is not the cause.) + // + // ⛑ THIS WALK MUST STAY AHEAD OF `codegen/collect.ts`'s + // `collectFunctions`. The client function emitter skips a `server fn` + // body ONLY because RI claimed it here. Teaching the CLIENT collector to + // see branch-declared functions while THIS walk stays blind emits the + // `server fn` BODY into `client.js` and produces no `server.js` at all — + // measured, and the reason the two halves land together + // (g-collect-functions-branch-decl-vs-server-boundary-routing, HIGH, + // security-gated). Child SHAPE via `ifChainChildNodes` + // (ast-if-chain.js), the one module that owns the fact. + for (const branchBody of ifChainChildNodes(node)) visitNodes([branchBody as ASTNode]); } } @@ -1159,6 +1187,18 @@ function collectWorkerBodyFunctionIds(fileAST: FileAST): Set { if ("children" in node && Array.isArray((node as any).children)) { visitNodes((node as any).children, enteringWorker); } + // §17.1.1 if-chain — this SUPPRESSION set must stay in lockstep with + // `collectFileFunctions` above. That walk now descends branch bodies, so a + // `server fn` declared inside an `if=`/`else` branch of a + // `` worker body is now COLLECTED; if this set stayed + // blind, the same function would be missing from the worker-body + // suppression and E-ROUTE-001 would false-fire on it. Closing one descent + // without the other converts a silent miss into a spurious error. + // `enteringWorker` threads through unchanged — the branch body is + // lexically inside whatever `` encloses the chain. + for (const branchBody of ifChainChildNodes(node)) { + visitNodes([branchBody as ASTNode], enteringWorker); + } } } diff --git a/compiler/tests/unit/g-if-chain-branch-cell-never-wired.test.js b/compiler/tests/unit/g-if-chain-branch-cell-never-wired.test.js index 9b32f8ae9..e815c7a71 100644 --- a/compiler/tests/unit/g-if-chain-branch-cell-never-wired.test.js +++ b/compiler/tests/unit/g-if-chain-branch-cell-never-wired.test.js @@ -36,16 +36,34 @@ * file". The accepting half is right — SPEC §6.1.1 + the §6.1.2 Read bullet say * so — but it is only right WITH this. * - * ⚠ THE TRAP THIS TEST ALSO PINS, because it was walked into and backed out of. - * `collect.ts`'s `collectFunctions` has the SAME blind spot, and closing it is - * NOT the same edit. It feeds the CLIENT function emitter, and the walk that - * routes a `server fn` to the server bundle is blind in its own separate way — - * so collecting branch-declared functions there emits a `server fn`'s BODY into - * `client.js` and produces no `server.js` at all. MEASURED: with that hunk the - * body shipped to the client; without it, it did not. A loud `ReferenceError` - * traded for a silent server-body leak is a strictly worse trade, so - * `collectFunctions` is deliberately NOT closed here and the leak guard below - * is what keeps it that way. + * ⚠ THE TRAP THIS TEST PINS, because it was walked into and backed out of once + * before it was closed properly. `collect.ts`'s `collectFunctions` had the SAME + * blind spot, and closing it is NOT the same edit. `analyze.ts` hands its result + * to BOTH the client function emitter (`emit-functions.ts`) and the server + * emitter (`emit-server.ts`) — but the client emitter omits a `server fn` body + * ONLY because route inference claimed the function as an endpoint, and RI's own + * walk (`route-inference.ts` `collectFileFunctions`) was blind to this same + * shape. So collecting branch-declared functions on the codegen side ALONE + * emitted the `server fn`'s BODY into `client.js` and produced no `server.js` + * at all. A loud `ReferenceError` traded for a silent server-body leak is a + * strictly worse trade. + * + * BOTH walks are now closed, ROUTING FIRST, and this is the three-way control + * that decided the landing order (`41 + 1` = the server fn's body): + * + * | | body in client | server.js bytes | + * |----------------------------------|----------------|-----------------| + * | base (neither walk) | 0 | 0 | + * | codegen half ALONE (the trap) | 1 | 0 | + * | RI half ALONE | 0 | 0 | + * | BOTH (shipped) | 0 | 2308 | + * | lone-`if=` oracle | 0 | 2308 | + * + * The leak guard below is what keeps the two halves together. It is written to + * RED against the codegen-half-alone column — verified by re-applying exactly + * that hunk and running this file — and it asserts the POSITIVE limb too, + * because an absence-only guard is also satisfied by the base column, where the + * function vanished entirely. * * VALUE-asserting (R26): compiles real .scrml end-to-end and asserts on the * EMITTED artifact, because the defect is invisible in the diagnostic stream by @@ -81,6 +99,14 @@ function compileSource(source) { const count = (hay, needle) => hay.split(needle).length - 1; +// Strip ALL whitespace before asserting on emitted-code SHAPE. +// +// ⚠ WHY: `expect(clientJs).not.toContain("41 + 1")` — this guard's original and +// only biting assertion — is whitespace-SENSITIVE. An emitter that spaced the +// same leaked body as `41+1`, or wrapped it across a line, passed it while +// leaking. A guard that a formatting change can silence is not a guard. +const squash = (s) => s.replace(/\s+/g, ""); + // The declaration under test, plus a read INSIDE the branch and one OUTSIDE it. const DECL_BRANCH = `
${D} : number = 7 }

in ${D}@n}

`; const ELSE_ARM = `

c

`; @@ -121,16 +147,115 @@ describe("g-if-chain-branch-cell-never-wired", () => { } }); - test("LEAK GUARD: a server fn in a branch never ships its body to the client", () => { - // This is the trap `collectFunctions` sets — see the header. If someone - // closes that walk without first closing the server-boundary routing walk, - // THIS is what reds. - const src = ` = true + // A `server fn` DECLARED INSIDE a branch, called from that branch's markup. + const SERVER_FN_SRC = ` = true
${D} server fn zzload() { return 41 + 1 } }

v ${D}zzload()}

${ELSE_ARM} `; - const r = compileSource(src); - expect(r.clientJs).not.toContain("41 + 1"); - expect(r.clientJs).not.toContain("server fn"); + + test("LEAK GUARD: a server fn in a branch never ships its body to the client", () => { + // This is the trap `collectFunctions` set — see the header. If someone + // re-opens the codegen half without the server-boundary routing walk, THIS + // is what reds. Verified to red against exactly that hunk. + const r = compileSource(SERVER_FN_SRC); + expect(r.errors.length).toBe(0); + + // (a) THE BODY. Whitespace-normalized: the leak shipped + // `function _scrml_zzload_7() { return 41 + 1; }` verbatim into client.js, + // and `41+1` must not survive any reformatting of that. + expect(squash(r.clientJs)).not.toContain("41+1"); + + // (b) THE DEFINITION. Every `function …zzload…` the client is allowed to + // define is the generated FETCH STUB (`_scrml_fetch_zzload_N`) that POSTs to + // the route. A definition WITHOUT `fetch` in its name is the server body + // inlined — the leak shape — and stays caught even if the body itself is + // constant-folded or rewritten past assertion (a). + const clientZzloadDefs = [...r.clientJs.matchAll(/function\s+([A-Za-z0-9_$]*zzload[A-Za-z0-9_$]*)\s*\(/g)] + .map((m) => m[1]); + expect(clientZzloadDefs.every((n) => n.includes("fetch"))).toBe(true); + + // (c) THE HANDLER PRELUDE. Route-handler machinery is server-only material; + // none of it may appear in a client bundle. + // (Replaces the ORIGINAL `not.toContain("server fn")` assertion, which was + // VACUOUS — emitted JS never contains the scrml source keyword regardless of + // where the body lands, so it could not fail for any input.) + for (const serverOnlyMarker of ["_scrml_handler_", "_scrml_validate_csrf", "Set-Cookie"]) { + expect(r.clientJs).not.toContain(serverOnlyMarker); + } + + // (d) THE POSITIVE LIMB — without it, (a)-(c) are ALSO satisfied by the base + // column of the header table, where the function vanished from every bundle + // and the page threw `ReferenceError`. "Absent from the client" only means + // something once the function is PRESENT on the server. + expect(r.serverJs.length).toBeGreaterThan(0); + expect(squash(r.serverJs)).toContain("41+1"); + expect(r.serverJs).toContain("zzload"); + }); + + test("ROUTING PARITY: the branch-declared server fn routes exactly like the lone-if= oracle", () => { + // Same source in the two wrappers whose only difference is the `
` + // sibling — the discriminator that makes `collapseIfChains` fire. + const loneSrc = SERVER_FN_SRC.replace(ELSE_ARM + "\n", ""); + // ⛑ ANTI-TAUTOLOGY GUARD. `String.replace` NO-OPS SILENTLY if `ELSE_ARM + + // "\n"` stops being a literal substring of `SERVER_FN_SRC` — a trailing + // space, a reflow of the template, an attribute added to ELSE_ARM. Without + // this line `loneSrc === SERVER_FN_SRC`, the test compiles the SAME source + // twice, and every assertion below passes trivially with the chain-vs-oracle + // discriminator gone. A gate that cannot fail is indistinguishable from a + // gate that never fails. + expect(loneSrc).not.toBe(SERVER_FN_SRC); + const lone = compileSource(loneSrc); + const chain = compileSource(SERVER_FN_SRC); + + expect(lone.errors.length).toBe(0); + expect(chain.errors.length).toBe(0); + + // The oracle emits a server bundle; so must the chain. + expect(lone.serverJs.length).toBeGreaterThan(0); + expect(chain.serverJs.length).toBeGreaterThan(0); + // NB: byte-length EQUALITY between the two bundles is deliberately NOT + // asserted. The chain source carries an extra `
` node, which + // shifts the node-id counter (this file's own Phase-2 note records + // `_scrml_helper_5` vs `_scrml_helper_8`). Equality holds today only because + // no node id happens to reach `server.js`; the first change that stamps one + // into a server artifact would red this test with no defect behind it. The + // per-marker count loop below carries the real signal. + + // Route identity, handler, and body all match. + for (const marker of ["_scrml_handler_zzload_1", "/_scrml/__ri_route_zzload_1", "41 + 1"]) { + expect(chain.serverJs).toContain(marker); + expect(count(chain.serverJs, marker)).toBe(count(lone.serverJs, marker)); + } + + // And the client side calls it the same way in both: through a fetch stub. + expect(chain.clientJs).toContain("/_scrml/__ri_route_zzload_1"); + expect(squash(chain.clientJs)).not.toContain("41+1"); + expect(squash(lone.clientJs)).not.toContain("41+1"); + }); + + test("a plain function declared in an if=/else branch is DEFINED and called by name", () => { + // The client-side half of the same class: with `collectFunctions` blind, the + // chain emitted ZERO definitions and four BARE `helper()` call sites — + // `ReferenceError` at exit 0 with zero diagnostics. The lone-`if=` oracle + // emitted one mangled definition and four mangled calls. + const decl = `
${D} function zzhelper() { return 1 } }

a ${D}zzhelper()}

b ${D}zzhelper()}

`; + const chain = compileSource(` = true\n${decl}\n${ELSE_ARM}\n`); + const lone = compileSource(` = true\n${decl}\n`); + + expect(chain.errors.length).toBe(0); + + const defs = (js) => (js.match(/function\s+_scrml_zzhelper_\d+\s*\(/g) ?? []).length; + const mangledCalls = (js) => (js.match(/_scrml_zzhelper_\d+\s*\(\s*\)/g) ?? []).length; + // A call site the emitter never resolved to a definition ships as the raw + // source name — that is the `ReferenceError`. + const bareCalls = (js) => (js.match(/(? = true + : string = "none" +} +
+ ${ + function branchHelper() { + return "from-branch" + } + } +

${branchHelper()} +