diff --git a/bynk-emit/src/emitter.rs b/bynk-emit/src/emitter.rs index 9542cc291..24fe62ea0 100644 --- a/bynk-emit/src/emitter.rs +++ b/bynk-emit/src/emitter.rs @@ -713,7 +713,7 @@ pub(crate) fn block_uses_send(b: &Block) -> bool { b.statements.iter().any(stmt) || expr(&b.tail) } -/// Events track, slice 0 (spine #936): does this block contain an +/// Events track, slice 0 (spine #936): does this block contain a real /// `Events.emit[...]` call anywhere — including nested branches, match arms, /// lambdas, and any other expression position (a `Paren`, an `Ok`/`Err` /// wrapper, a `Call`/`RecordConstruction` argument, a `BinOp` operand, …)? @@ -731,23 +731,32 @@ pub(crate) fn block_uses_send(b: &Block) -> bool { /// can't drift from the lowering again: a new `ExprKind` variant fails to /// compile here until `walk_exprs` itself is taught to visit it. /// -/// Syntactic, like `block_uses_send`: matches a bare-`Events`-receiver -/// `.emit` call by name, not by resolving the receiver against `given` — a -/// locally-shadowed `Events` would be a false positive, an accepted -/// approximation matching `block_uses_send`'s own precedent (it doesn't -/// verify `~>`'s target either). -pub(crate) fn block_uses_emit(b: &Block) -> bool { - fn is_events_emit_call(receiver: &Expr, method: &Ident) -> bool { - matches!(&receiver.kind, ExprKind::Ident(id) if id.name == "Events") - && method.name == "emit" - } +/// #1187's slice 6 plumbing (review of #1202): reads the checker's own +/// already-resolved `Callee::Capability{cap:"Events",op:"emit"}` for each +/// visited call site instead of a bare-`Ident("Events")`-receiver name +/// match. Was deliberately syntactic before this — this function's own +/// prior doc comment named the locally-shadowed-`Events` false positive an +/// "accepted approximation," matching `block_uses_send`'s own precedent — +/// but that approximation stopped being harmless once `crate::project:: +/// unit_table_uses_emit` (the project-wide compose-gating twin this +/// function's own callers must agree with) became precise first: the two +/// disagreeing on exactly the shadowed case produces a real `tsc` type +/// error (a `deps.__eventsDispatch` call site with nothing supplying it), +/// not just an unused interface field. `block_uses_send` needs no matching +/// fix — a `~>` send is a real `Statement::Send` AST variant, not a method +/// call that could be shadowed, so it was never approximate to begin with. +pub(crate) fn block_uses_emit( + b: &Block, + callees: &HashMap, +) -> bool { let mut found = false; walk_block_exprs(b, &mut |e| { if !found - && let ExprKind::MethodCall { - receiver, method, .. - } = &e.kind - && is_events_emit_call(receiver, method) + && matches!( + callees.get(&e.id), + Some(bynk_check::checker::Callee::Capability { cap, op }) + if cap == "Events" && op == "emit" + ) { found = true; } @@ -2477,7 +2486,10 @@ fn write_header(out: &mut String, commons: &TypedCommons, ctx: &EmitProjectCtx) // provider does. let has_agent_uses_emit = workers && commons.commons.items.iter().any(|i| match i { - CommonsItem::Agent(a) => a.handlers.iter().any(|h| block_uses_emit(&h.body)), + CommonsItem::Agent(a) => a + .handlers + .iter() + .any(|h| block_uses_emit(&h.body, &commons.callees)), _ => false, }); if has_agent_uses_emit { diff --git a/bynk-emit/src/emitter/emit.rs b/bynk-emit/src/emitter/emit.rs index 816738618..9e8a57d21 100644 --- a/bynk-emit/src/emitter/emit.rs +++ b/bynk-emit/src/emitter/emit.rs @@ -1613,7 +1613,7 @@ pub(crate) fn emit_service( // `Events` rides the same path since it's declared as an ordinary // `given Events` on the agent handler. let needs_events_dispatch = cx.is_first_party_events() - && (crate::emitter::block_uses_emit(&handler.body) + && (crate::emitter::block_uses_emit(&handler.body, &commons.callees) || cx .agent_given_caps_used() .is_some_and(|m| m.contains_key("Events"))); @@ -1652,7 +1652,7 @@ pub(crate) fn emit_service( // `__eventsDispatch` to an agent it calls has nothing of its own to // buffer or flush, so it keeps byte-identical output, mirroring // `__exec`'s gate on `block_uses_send`. - let body_emits_directly = crate::emitter::block_uses_emit(&handler.body); + let body_emits_directly = crate::emitter::block_uses_emit(&handler.body, &commons.callees); if body_emits_directly { writeln!( out, @@ -1708,11 +1708,11 @@ pub(crate) fn commons_uses_emit(commons: &TypedCommons) -> bool { CommonsItem::Service(s) => s .handlers .iter() - .any(|h| crate::emitter::block_uses_emit(&h.body)), + .any(|h| crate::emitter::block_uses_emit(&h.body, &commons.callees)), CommonsItem::Agent(a) => a .handlers .iter() - .any(|h| crate::emitter::block_uses_emit(&h.body)), + .any(|h| crate::emitter::block_uses_emit(&h.body, &commons.callees)), _ => false, }) } @@ -2988,7 +2988,7 @@ pub(crate) fn emit_agent( let agent_uses_emit = a .handlers .iter() - .any(|h| crate::emitter::block_uses_emit(&h.body)); + .any(|h| crate::emitter::block_uses_emit(&h.body, &commons.callees)); let needs_env_ctor = given_deps_expr.is_some() || agent_uses_emit; if needs_env_ctor { writeln!(out, " private __env: unknown;").unwrap(); @@ -3240,7 +3240,7 @@ pub(crate) fn emit_agent( // that calls another local agent method which itself emits, gets // the same compose-supplied `__eventsDispatch` callback. let needs_events_dispatch = cx.is_first_party_events() - && (crate::emitter::block_uses_emit(&h.body) + && (crate::emitter::block_uses_emit(&h.body, &commons.callees) || cx .agent_given_caps_used() .is_some_and(|m| m.contains_key("Events"))); @@ -3313,7 +3313,7 @@ pub(crate) fn emit_agent( // a handler that only *forwards* `__eventsDispatch` to another // local agent it calls has nothing of its own to buffer or flush; // `deps` (typed with the field) simply passes through unchanged. - let body_emits_directly = crate::emitter::block_uses_emit(&h.body); + let body_emits_directly = crate::emitter::block_uses_emit(&h.body, &commons.callees); let events_decl = format!( " const __events: Array<{}> = [];", crate::emitter::EVENTS_WIRE_EVENT_TS_TYPE diff --git a/bynk-emit/src/ir/lower.rs b/bynk-emit/src/ir/lower.rs index 8b3d621e6..8e32ec201 100644 --- a/bynk-emit/src/ir/lower.rs +++ b/bynk-emit/src/ir/lower.rs @@ -405,7 +405,7 @@ pub(crate) fn lower_handler_ir( ); let cx = LowerIrCtx::new(program, HashSet::new()); let (params, given, ret, effectful) = lower_handler_signature_ir(h, &cx); - let emits = block_uses_emit(&h.body); + let emits = block_uses_emit(&h.body, &program.program().callees); let commit = lower_commit_shape_ir(&h.body, invariants, transitions, emits, program); let body = lower_handler_body_ir(h, store_cells, state_ty, program); IrHandler { @@ -695,7 +695,7 @@ pub(crate) fn lower_service_handler_ir( } _ => None, }; - let emits = block_uses_emit(&h.body); + let emits = block_uses_emit(&h.body, &program.program().callees); let commit = lower_commit_shape_ir(&h.body, &[], &[], emits, program); let body = lower_service_handler_body_ir(h, binder.as_ref(), connection.as_ref(), program); IrHandler { @@ -5670,7 +5670,7 @@ agent Widget { fn commit_shape_of(program: &CheckedProgram, handler_name: &str) -> CommitShape { let agent = find_agent(program, "Widget"); let handler = find_handler(agent, handler_name); - let emits = block_uses_emit(&handler.body); + let emits = block_uses_emit(&handler.body, &program.program().callees); lower_commit_shape_ir(&handler.body, &[], &[], emits, program) } diff --git a/bynk-emit/src/project.rs b/bynk-emit/src/project.rs index 8f7b12196..e2231eb3b 100644 --- a/bynk-emit/src/project.rs +++ b/bynk-emit/src/project.rs @@ -2798,29 +2798,14 @@ pub(crate) fn instantiate_provider_expr( /// that violates it fails loudly in tests rather than shipping a publishing /// context that silently drops every emitted event. /// -/// **Known follow-on, found while adding a regression fixture for this PR, -/// deliberately not attempted here:** `emitter::block_uses_emit` — the -/// still-AST-driven, per-*handler* twin this function used to share its own -/// name-matching logic with — decides `emit_service`/`emit_agent`'s own -/// `deps.__eventsDispatch` *parameter* threading (`emit.rs`'s -/// `needs_events_dispatch`/`body_emits_directly`), a genuinely different -/// call site from this project-wide compose-gating one. Before this PR both -/// checks used the same syntactic bare-`Ident("Events")` match, so a -/// locally-declared type also named `Events` with its own static `emit` -/// (legal, resolves to `Callee::Static`) fooled both identically — -/// needlessly wiring up real event-fanout machinery for a call that has -/// nothing to do with the capability, but *consistently*, so the emitted -/// TypeScript still type-checked. Now that this function reads the -/// checker's own resolved `Callee` and `block_uses_emit` still does not, -/// the two can disagree on exactly that shadowed-name case: this function -/// correctly says "no real emit here" (skips compose/fan-out generation), -/// while a handler's own body still gets a `deps.__eventsDispatch` call -/// site with nothing left to supply it — a `tsc` type error, confirmed by -/// hand (a fixture hitting this shape was written for review of #1202, -/// found to fail `tsc --strict`, and removed rather than landed broken). -/// Closing this needs `block_uses_emit`'s own several call sites -/// (`emit.rs`, `emitter.rs`, `ir/lower.rs`) converted too — a real, larger, -/// separately-scoped slice, not attempted here. +/// `emitter::block_uses_emit` — the per-*handler* twin deciding +/// `emit_service`/`emit_agent`'s own `deps.__eventsDispatch` *parameter* +/// threading — reads the same resolved `Callee` now too (its own doc +/// comment has the story: the two checks briefly disagreed on a +/// locally-shadowed `Events` type between this function converting and +/// that one following, confirmed by a fixture that failed `tsc --strict` in +/// between, `1204_events_emit_shadowed_by_local_type`), so the two stay in +/// agreement on every input, not just the ones existing fixtures cover. pub(crate) fn unit_table_uses_emit( table: &UnitTable, callees: Option<&HashMap>, diff --git a/bynkc/tests/fixtures/positive/1204_events_emit_shadowed_by_local_type/expected/demo.ts b/bynkc/tests/fixtures/positive/1204_events_emit_shadowed_by_local_type/expected/demo.ts new file mode 100644 index 000000000..0b46c7103 --- /dev/null +++ b/bynkc/tests/fixtures/positive/1204_events_emit_shadowed_by_local_type/expected/demo.ts @@ -0,0 +1,31 @@ +// Generated by bynkc — do not edit by hand. +// context demo + +import { Ok, Err, Some, None, type Result, type Option, type ValidationError } from "./runtime.js"; + +export interface Events { +} + +export const Events = { + emit(e: number): Promise { + return Promise.resolve(undefined); + }, +}; + +export const pinger = { + async call(deps: {}): Promise { + return Events.emit(1); + }, +}; + +export interface DemoDeps { +} + +export function makeSurface(deps: DemoDeps) { + return { + async pinger(): Promise { + return pinger.call(deps); + }, + }; +} + diff --git a/bynkc/tests/fixtures/positive/1204_events_emit_shadowed_by_local_type/src/demo.bynk b/bynkc/tests/fixtures/positive/1204_events_emit_shadowed_by_local_type/src/demo.bynk new file mode 100644 index 000000000..31f50fbbe --- /dev/null +++ b/bynkc/tests/fixtures/positive/1204_events_emit_shadowed_by_local_type/src/demo.bynk @@ -0,0 +1,27 @@ +context demo + +-- Regression (review of #1202, closed by the emitter.rs::block_uses_emit +-- follow-on it named): a locally-declared type named `Events` with its own +-- static `emit` method must not be mistaken for the real `bynk.Events` +-- capability's `emit` op, at *either* granularity this compiler checks it +-- at. `unit_table_uses_emit` (project-wide, project.rs) and +-- `block_uses_emit` (per-handler, emitter.rs) both now read the checker's +-- own resolved `Callee` — `Callee::Static`, not +-- `Callee::Capability{cap:"Events",op:"emit"}`, for this call — so neither +-- wires up compose/fan-out machinery nor threads a `deps.__eventsDispatch` +-- parameter for a context that never actually uses the real Events +-- capability. Before both were converted, a first version of this fixture +-- (review of #1202) failed `tsc --strict`: the project-level check had +-- already gone Callee-based while the per-handler one was still +-- name-matching, so the two disagreed on exactly this shape. +type Events = { } + +fn Events.emit(e: Int) -> Effect[()] { + Effect.pure(()) +} + +service pinger { + on call() -> Effect[()] { + Events.emit(1) + } +} diff --git a/design/pending/p6-block-uses-emit-callee.md b/design/pending/p6-block-uses-emit-callee.md new file mode 100644 index 000000000..467f4590a --- /dev/null +++ b/design/pending/p6-block-uses-emit-callee.md @@ -0,0 +1,4 @@ +--- +level: patch +changelog: "emitter::block_uses_emit now reads the checker's own resolved Callee classification instead of a bare-Ident(\"Events\") receiver name match, closing a real disagreement with project.rs's own unit_table_uses_emit (#1202) on a locally-shadowed Events type that could previously produce TypeScript failing tsc --strict" +---