From 9f4e7168747919d942eec83d1bae2faab52ce8ae Mon Sep 17 00:00:00 2001 From: addiplus Date: Thu, 13 Aug 2026 00:20:44 -0700 Subject: [PATCH 1/3] fix(typescript): escape reserved words in generated declaration names `sdkgen_typescript_shared` wrote class, enum and type-alias names raw, so a BAML `enum import` emitted `export enum import {` and the whole generated file failed to parse (TS1359). `is_js_reserved` and `JS_RESERVED` already existed but were applied only to parameters, free functions and child-namespace re-exports. Add `safe_decl_name` beside `safe_param_name` and apply it at the two places that name a binding: `emit::build_emitted`, where an IR name becomes an emitted symbol, and `translate_ty::render_name_ref`, which re-derives every cross-reference from the IR rather than reading the declaration back. Escaping one side alone would turn a parse error into a dangling reference. Type parameters take the same escape in `generic_decl`. Escaping at `build_emitted` also covers `_typemap.ts`, which indexes the module namespace through a `Record` cast and so would have resolved to `undefined` at runtime with no compile-time signal. Only the TypeScript identifier moves. Wire identity is unchanged: dispatch still uses `baml_fqn`, the type map is still keyed on the raw BAML FQN, and enum member values, marshalling parameter names and the `$generic` / `typeParams` arrays all keep their source spelling. --- .../sdkgen_typescript_shared/src/emit/mod.rs | 13 +++- .../sdkgen_typescript_shared/src/leaf.rs | 60 ++++++++++++++- .../sdkgen_typescript_shared/src/lib.rs | 48 ++++++++++++ .../src/translate_ty.rs | 74 ++++++++++++++++++- 4 files changed, 186 insertions(+), 9 deletions(-) diff --git a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/mod.rs b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/mod.rs index e724df61e8a..ed45850a94d 100644 --- a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/mod.rs +++ b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/mod.rs @@ -22,6 +22,7 @@ use crate::{ method::{MethodKind, OptionalArg, RequiredArg, TypeScriptMethodBinding}, type_alias::TypeScriptTypeAlias, }, + leaf::safe_decl_name, routing::{LeafPath, route}, }; @@ -55,6 +56,12 @@ pub(crate) fn build_emitted(pool: &SymbolPool) -> Vec<(LeafPath, EmittedSymbol, // identifier char, so a `$stream` companion class is emitted as // e.g. `Resume$stream` (not stripped to `Resume`). Non-stream // symbols are unaffected (their name carries no `$stream`). + // + // `bare` stays raw. The class / enum / type-alias arms below wrap it in + // `safe_decl_name` because those three names bind a module-scope + // identifier, where a reserved word is a parse error. Every wire-facing + // field on the emitted symbol (`source`, `baml_fqn`, enum member + // `value`) keeps the raw spelling. let bare = key.name().as_str().to_string(); match symbol { @@ -82,7 +89,7 @@ pub(crate) fn build_emitted(pool: &SymbolPool) -> Vec<(LeafPath, EmittedSymbol, out.push(( leaf, EmittedSymbol::Class(TypeScriptClass { - name: bare, + name: safe_decl_name(&bare), source: key.clone(), generic_params, docstring: c.docstring.clone(), @@ -107,7 +114,7 @@ pub(crate) fn build_emitted(pool: &SymbolPool) -> Vec<(LeafPath, EmittedSymbol, out.push(( leaf, EmittedSymbol::Enum(TypeScriptEnum { - name: bare, + name: safe_decl_name(&bare), source: key.clone(), variants, docstring: e.docstring.clone(), @@ -120,7 +127,7 @@ pub(crate) fn build_emitted(pool: &SymbolPool) -> Vec<(LeafPath, EmittedSymbol, out.push(( leaf, EmittedSymbol::TypeAlias(TypeScriptTypeAlias { - name: bare, + name: safe_decl_name(&bare), source: key.clone(), resolves_to: t.resolves_to.clone(), recursive: t.recursive, diff --git a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs index 89925d14de9..a11b2fc7722 100644 --- a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs +++ b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs @@ -301,12 +301,17 @@ fn write_enum_doc(out: &mut String, e: &TypeScriptEnum) { out.push_str(" */\n"); } -/// `` generic-parameter list, or empty. +/// `` generic-parameter list, or empty. A type-parameter name is a +/// binding identifier, so it takes the same declaration escape as a class or +/// enum name; `translate_ty` re-applies that escape at every `Ty::TypeVar` use +/// site. The raw spellings still reach the runtime through the `$generic` +/// array and the `typeParams` factory argument, which are string literals. fn generic_decl(params: &[String]) -> String { if params.is_empty() { String::new() } else { - format!("<{}>", params.join(", ")) + let escaped: Vec = params.iter().map(|p| safe_decl_name(p.as_str())).collect(); + format!("<{}>", escaped.join(", ")) } } @@ -322,6 +327,31 @@ pub(crate) fn safe_param_name(name: &str) -> String { } } +/// A DECLARATION name binds an identifier in module scope, so unlike an enum +/// member or a class property (both `IdentifierName`, where reserved words are +/// legal) it cannot be a reserved word. `export enum import {}` is TS1359, and +/// because it is a parse error it kills the whole generated file rather than +/// just that one symbol. Append `_`, the same rule [`safe_param_name`] uses and +/// the same rule the Python SDK's `escape_python_keyword` uses, so a BAML +/// `enum import` renders as `export enum import_ {}`. +/// +/// The escape is deliberately STATELESS. A class / enum / type-alias name has +/// to be reproducible from the bare BAML `Name` alone at a distance, because +/// `translate_ty::render_name_ref` re-derives the reference from the IR at +/// every cross-reference rather than reading the emitted declaration back. +/// +/// Only the TypeScript identifier moves. Wire identity travels on separate +/// channels and is untouched: `defineFunction` is handed `baml_fqn`, +/// `_typemap.ts` is keyed on `TypeScriptClass::source`, and enum member values +/// are emitted verbatim, so dispatch and marshalling still target `import`. +pub(crate) fn safe_decl_name(name: &str) -> String { + if is_js_reserved(name) { + format!("{name}_") + } else { + name.to_string() + } +} + pub(crate) fn safe_required_param_names( names: &[&str], reserve_options_param: bool, @@ -1097,6 +1127,32 @@ mod tests { }) } + #[test] + fn safe_decl_name_escapes_only_reserved_words() { + assert_eq!(safe_decl_name("import"), "import_"); + assert_eq!(safe_decl_name("class"), "class_"); + assert_eq!(safe_decl_name("interface"), "interface_"); + assert_eq!(safe_decl_name("Resume"), "Resume"); + // Companion suffixes survive: `$` is a legal TS identifier character. + assert_eq!(safe_decl_name("Resume$stream"), "Resume$stream"); + // Python keywords that are not reserved in TypeScript stay put, so the + // shared `reserved_keywords` fixture keeps per-language spellings + // rather than converging on one escaped set. + assert_eq!(safe_decl_name("None"), "None"); + assert_eq!(safe_decl_name("pass"), "pass"); + assert_eq!(safe_decl_name("lambda"), "lambda"); + } + + #[test] + fn generic_decl_escapes_reserved_type_parameters() { + assert_eq!(generic_decl(&[]), ""); + assert_eq!(generic_decl(&["T".to_string()]), ""); + assert_eq!( + generic_decl(&["package".to_string(), "T".to_string()]), + "" + ); + } + #[test] fn class_renders_real_body() { let b = body( diff --git a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/lib.rs b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/lib.rs index e60a50830d5..7cc0318351f 100644 --- a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/lib.rs +++ b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/lib.rs @@ -392,6 +392,54 @@ mod tests { assert!(leaf.contains("POSITIVE = \"POSITIVE\",")); } + #[test] + fn reserved_declaration_name_is_escaped_and_wire_identity_is_preserved() { + let mut pool = SymbolPool::new(); + let enum_name = name("user", &["lorem"], "import"); + pool.insert(enum_name.clone(), enum_sym(&enum_name, 0)); + let fn_name = name("user", &["lorem"], "pick"); + pool.insert( + fn_name, + Symbol::Function(Function { + name: BaseName::new("pick"), + generic_params: Vec::new(), + docstring: None, + arguments: Vec::new(), + return_type: Ty::Enum(enum_name, baml_base::TyAttr::EMPTY), + throws: None, + watchers: Vec::new(), + origin: origin(1), + }), + ); + + let out = emit_sdk(&pool); + let leaf = &out[&PathBuf::from("lorem/index.ts")]; + // `export enum import {` is TS1359, so the declaration takes the + // trailing-underscore escape ... + assert!(leaf.contains("export enum import_ {"), "{leaf}"); + assert!(!leaf.contains("export enum import {"), "{leaf}"); + // ... the member name is an `IdentifierName` and keeps its source + // spelling, as does its wire value ... + assert!(leaf.contains("POSITIVE = \"POSITIVE\","), "{leaf}"); + // ... and the reference re-derives the same escaped spelling from the + // IR rather than dangling on the raw name. + assert!(leaf.contains("=> import_;"), "{leaf}"); + + // Wire identity is untouched: dispatch still names the raw BAML FQN. + assert!( + leaf.contains("defineFunction(\"user.lorem.pick\", \"sync\", [])"), + "{leaf}" + ); + // The type map is still keyed on the raw FQN; only the module-scope + // attribute it resolves through moved. + let typemap = &out[&PathBuf::from("_typemap.ts")]; + let entry = typemap + .lines() + .find(|line| line.contains("\"user.lorem.import\":")) + .expect("type map keys the enum on its raw BAML FQN"); + assert!(entry.ends_with("[\"import_\"],"), "{entry}"); + } + #[test] fn function_fans_out_sync_and_async() { let mut pool = SymbolPool::new(); diff --git a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/translate_ty.rs b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/translate_ty.rs index 7ff6b18a399..aaca45af334 100644 --- a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/translate_ty.rs +++ b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/translate_ty.rs @@ -22,6 +22,7 @@ use baml_base::{Literal, MediaKind}; use baml_codegen_types::{CodegenFunctionParamMode, Name, Ty}; use crate::{ + leaf::safe_decl_name, routing::{LeafPath, route_class_ref}, ts_string, }; @@ -153,11 +154,17 @@ pub(crate) fn translate_ty(ty: &Ty, ctx: &TranslateCtx) -> TranslatedType { Ty::EnumVariant(name, variant, _) => { let mut result = render_name_ref(name, ctx); result.expr.push('.'); + // The MEMBER stays raw. An enum member name is an `IdentifierName`, + // so a reserved word is legal both in the declaration + // (`{ import = "import" }`) and in this member access + // (`Kw.import`); escaping it here would dangle. result.expr.push_str(variant.as_str()); result } Ty::TypeAlias(name, _) => render_name_ref(name, ctx), - Ty::TypeVar(name, _) => TranslatedType::bare(name.as_str().to_string()), + // A type parameter is a binding identifier at its declaration site + // (`leaf::generic_decl`), so the use site takes the same escape. + Ty::TypeVar(name, _) => TranslatedType::bare(safe_decl_name(name.as_str())), Ty::Function { params, ret, .. } => { let ret_t = translate_ty(ret, ctx); @@ -241,7 +248,8 @@ pub(crate) const ROOT_ALIAS: &str = "_bamlRoot"; /// The emitted identifier is the BAML name verbatim — including any /// `$stream` suffix (spec2: `$` is a valid TS identifier char, and stream /// companions live beside their base type rather than in a `stream_types/` -/// namespace). +/// namespace), except that a reserved word takes the same trailing-underscore +/// escape [`crate::leaf::safe_decl_name`] applies to the declaration. /// /// - Same leaf → bare name, no import. /// - Root-namespace symbol referenced from a non-root leaf → `_bamlRoot.Name` @@ -251,9 +259,13 @@ pub(crate) const ROOT_ALIAS: &str = "_bamlRoot"; /// `LeafPath` in `imports`. fn render_name_ref(name: &Name, ctx: &TranslateCtx) -> TranslatedType { let routed = route_class_ref(name); - let ident = name.name().as_str(); + // Re-derived from the IR `Name`, never read back off the emitted + // declaration, so it has to apply the identical stateless escape that + // `emit::build_emitted` applied when it named the declaration. Escaping one + // side alone produces a dangling reference instead of a fixed file. + let ident = safe_decl_name(name.name().as_str()); if routed == ctx.current_leaf { - return TranslatedType::bare(ident.to_string()); + return TranslatedType::bare(ident); } let mut imports = BTreeSet::new(); if routed.segments.is_empty() { @@ -1010,4 +1022,58 @@ mod tests { assert_ty(case); } } + + #[test] + fn reserved_declaration_names_are_re_escaped_at_reference_sites() { + // Same leaf: the bare identifier, escaped exactly as the declaration + // was escaped in `emit::build_emitted`. + assert_eq!( + translate_ty( + &enum_ty(name("user", &["lorem"], "import")), + &ctx(&["lorem"]) + ) + .expr, + "import_" + ); + // Cross leaf: only the identifier moves; the routed namespace path is + // untouched, because a module path segment is not a binding site. + assert_eq!( + translate_ty(&enum_ty(name("user", &["lorem"], "import")), &ctx(&[])).expr, + "lorem.import_" + ); + // Root-namespace symbol reached from a nested leaf. + assert_eq!( + translate_ty( + &class_ty(name("user", &[], "class"), vec![]), + &ctx(&["lorem"]) + ) + .expr, + "_bamlRoot.class_" + ); + // The enum MEMBER keeps its source spelling: it is an `IdentifierName` + // in both the declaration and the member access. + assert_eq!( + translate_ty( + &enum_variant_ty(name("user", &["lorem"], "import"), "default"), + &ctx(&["lorem"]) + ) + .expr, + "import_.default" + ); + // Type parameters are binding identifiers too. + assert_eq!( + translate_ty(&type_var(BaseName::new("package")), &ctx(&[])).expr, + "package_" + ); + // Non-reserved names are untouched, so keyword-free schemas emit + // byte-identical output to before this change. + assert_eq!( + translate_ty( + &alias_ty(name("user", &["lorem"], "Json")), + &ctx(&["lorem"]) + ) + .expr, + "Json" + ); + } } From 0fffdf306da1b5ff425331a5ed230e520d1d70df Mon Sep 17 00:00:00 2001 From: addiplus Date: Fri, 14 Aug 2026 20:49:52 -0700 Subject: [PATCH 2/3] fix(typescript): allocate collision-safe binders for generic type parameters The TypeScript generator wrote a type-parameter list by joining the raw BAML names, so a reserved word in that list was a parse error that took the whole generated file with it rather than just the one symbol. A BAML class `Pair` renders as `export class Pair {`, which tsc rejects with TS1213. Escaping each name in isolation does not fix it. Appending an underscore to a reserved word makes `package` and a sibling `package_` both render as `package_`, and the file fails with TS2300 instead. Either way the generated file does not compile. This mirrors the Python SDK's TypeVar allocator into the TypeScript generator, narrowed to TypeScript's scoping rules. `allocate_binders` allocates the emitted identifier for every type parameter of one generic scope against a reservation set and returns the scope's full raw-to-emitted map. A Python TypeVar is a module-level assignment, so the Python allocator reserves leaf-globally; a TypeScript type parameter is scoped to its own declaration, so the allocation unit here is the scope, not the leaf. `TranslateCtx` carries the active scope's map and `Ty::TypeVar` resolves every use site through it, so a reference lands on exactly the identifier its declaration allocated instead of re-deriving a colliding escape. The map is keyed by the RAW name, so a `{package, package_}` twin can never collapse onto one identifier. A non-reserved raw name maps to itself, unconditionally: the reservation set is only ever consulted for a name that is a JavaScript reserved word, and bumping only appends an underscore. Every keyword-free schema therefore renders byte-identically to today's output. Wire identity is untouched, because the raw spellings still reach the runtime through the `$generic` array and the `typeParams` factory argument, both string literals. Two bounds are deliberately not reserved against, and both are stated in the allocator's doc comment rather than papered over. The cross-leaf `import type * as ` aliases are routing-sanitized module path segments that are not known until the leaf's bodies have been rendered, and shadowing one inside a type-parameter list is a resolution change rather than a parse error. An inner scope's non-reserved parameter is likewise not reserved against: `Box` allocates `package_`, and an instance method that declares a parameter literally named `package_` re-binds that identifier for the whole method. Shadowing a type parameter is legal TypeScript and compiles clean. Widening the bump to cover either case would destroy the unconditional non-reserved-maps-to-itself guarantee that keeps keyword-free output byte-identical. --- .../sdkgen_typescript_shared/src/leaf.rs | 334 ++++++++++++++++-- .../src/translate_ty.rs | 34 +- 2 files changed, 343 insertions(+), 25 deletions(-) diff --git a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs index a11b2fc7722..9f81b303d80 100644 --- a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs +++ b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs @@ -301,16 +301,145 @@ fn write_enum_doc(out: &mut String, e: &TypeScriptEnum) { out.push_str(" */\n"); } +/// Raw BAML type-parameter name → the TypeScript binder identifier allocated +/// for it in one generic scope. Mirrors the Python SDK's `TypeVarMap`. +pub(crate) type TypeVarMap = BTreeMap; + +/// Resolve one raw type-parameter name to the identifier its declaration +/// allocated. The scope map wins; the stateless [`safe_decl_name`] is the +/// fallback for scopes that bind no type parameters (type-alias bodies, +/// non-generic classes and callables), where no twin can exist. +pub(crate) fn emitted_binder(raw: &str, map: Option<&TypeVarMap>) -> String { + map.and_then(|m| m.get(raw).cloned()) + .unwrap_or_else(|| safe_decl_name(raw)) +} + +/// Allocate the emitted binder identifier for every type parameter of ONE +/// generic scope, against a reservation set, and return the scope's full +/// raw→emitted map (the enclosing scope's entries included). +/// +/// This is the TypeScript counterpart of the Python SDK's +/// `leaf::allocate_leaf_type_vars`, narrowed to TypeScript's scoping rules. The +/// Python allocator reserves LEAF-globally because a Python `TypeVar` is a +/// module-level assignment (`T = typing.TypeVar("T")`), so two scopes really do +/// share one binding. A TypeScript type parameter is scoped to its own +/// declaration, so the allocation unit here is the scope, not the leaf. The +/// collision-resolution rule is otherwise identical. +/// +/// - `raw_params` are this scope's OWN type parameters, in declaration order. +/// - `outer` is the enclosing scope's map (a generic class, for an instance +/// method) or `None`. A static method re-declares the class parameters as its +/// own (TS2302), so it passes them in `raw_params` with no `outer`. +/// - `module_names` are the leaf's module-scope bindings a binder could shadow. +/// +/// Guarantees, matching the Python allocator's: +/// - **A non-reserved raw name maps to itself, unconditionally.** The +/// reservation set is only ever consulted for a raw name that is a JavaScript +/// reserved word, and bumping only appends `_`. Every keyword-free schema +/// therefore renders byte-identically to the stateless escape it replaces. +/// - **A reserved raw name bumps** (`package`→`package_`→`package__`…) past +/// (a) every raw type-parameter name in this scope and its enclosing scope, +/// (b) every binder already allocated in this scope chain, and (c) the leaf's +/// module-scope declaration names and immediate child-namespace names. +/// - The map is keyed by RAW name, so a `{package, package_}` twin can never +/// collapse onto one identifier, and `translate_ty` resolves each use site +/// through the same map. +/// +/// **Deliberately not reserved.** Runtime import names (`defineFunction`, +/// `BamlCallContext`, `_BamlHandle`, `_TYPE_MAP`, `__ns_*`, `__baml_*`) are +/// unreachable by construction: bumping only appends `_` to a reserved word, so +/// an allocated binder is never `_`-leading and never equals one of them. The +/// cross-leaf `import type * as ` aliases are NOT reserved: they are +/// routing-sanitized module path segments, they are not known until the leaf's +/// bodies have been rendered, and shadowing one inside a type-parameter list is +/// a resolution change rather than a parse error. That bound is stated here +/// rather than papered over. +/// An inner scope's NON-reserved parameter is likewise not reserved against. +/// `Box` allocates `package_`, and an instance method that declares a +/// parameter literally named `package_` re-binds that identifier for the whole +/// method. Shadowing a type parameter is legal TypeScript and compiles clean, +/// so this is a name-resolution change of the same category as the import +/// aliases above, not a parse error. Widening the bump to cover it would +/// destroy the unconditional non-reserved-maps-to-itself guarantee, so it is +/// stated rather than fixed. +fn allocate_binders( + raw_params: &[String], + outer: Option<&TypeVarMap>, + module_names: &BTreeSet, +) -> TypeVarMap { + let mut map: TypeVarMap = outer.cloned().unwrap_or_default(); + if raw_params.is_empty() { + return map; + } + let mut reserved: BTreeSet = BTreeSet::new(); + // (a) every raw type-parameter name in this scope — so `package` cannot bump + // onto a sibling `package_` that maps to itself. + reserved.extend(raw_params.iter().cloned()); + // (b) the enclosing scope's raw names and the binders already allocated for + // them. This is a BUMP target only: it stops a reserved-word inner + // parameter from bumping onto an identifier the enclosing scope is + // already using, which would silently retarget that scope's references. + // It does NOT stop a non-reserved inner parameter from re-binding an + // outer name: that branch maps the raw name to itself unconditionally + // and never reads this set. That case is ordinary TypeScript shadowing, + // legal and compile-clean, and is noted with the other unreserved + // surfaces above. + for (raw, emitted) in &map { + reserved.insert(raw.clone()); + reserved.insert(emitted.clone()); + } + // (c) the leaf's module-scope declaration and child-namespace names. + reserved.extend(module_names.iter().cloned()); + + for raw in raw_params { + if !is_js_reserved(raw) { + map.insert(raw.clone(), raw.clone()); + } else { + let mut candidate = format!("{raw}_"); + while is_js_reserved(&candidate) || reserved.contains(&candidate) { + candidate.push('_'); + } + reserved.insert(candidate.clone()); + map.insert(raw.clone(), candidate); + } + } + map +} + +/// The leaf's module-scope bindings a type-parameter binder could shadow: every +/// emitted declaration name in the body, plus every immediate child-namespace +/// name re-exported by this `index.ts`. +fn module_scope_names(body: &LeafBody, kids: &BTreeSet) -> BTreeSet { + let mut out: BTreeSet = kids.clone(); + for (sym, _) in &body.symbols { + out.insert(symbol_decl_name(sym).to_string()); + } + out +} + +fn symbol_decl_name(sym: &EmittedSymbol) -> &str { + match sym { + EmittedSymbol::Class(c) => &c.name, + EmittedSymbol::Enum(e) => &e.name, + EmittedSymbol::TypeAlias(a) => &a.name, + EmittedSymbol::Function(f) => &f.name, + } +} + /// `` generic-parameter list, or empty. A type-parameter name is a -/// binding identifier, so it takes the same declaration escape as a class or -/// enum name; `translate_ty` re-applies that escape at every `Ty::TypeVar` use -/// site. The raw spellings still reach the runtime through the `$generic` -/// array and the `typeParams` factory argument, which are string literals. -fn generic_decl(params: &[String]) -> String { +/// binding identifier, so it cannot be a reserved word; `map` carries the +/// identifier allocated for each raw name by [`allocate_binders`], and +/// `translate_ty` resolves every `Ty::TypeVar` use site through the same map. +/// The raw spellings still reach the runtime through the `$generic` array and +/// the `typeParams` factory argument, which are string literals. +fn generic_decl(params: &[String], map: &TypeVarMap) -> String { if params.is_empty() { String::new() } else { - let escaped: Vec = params.iter().map(|p| safe_decl_name(p.as_str())).collect(); + let escaped: Vec = params + .iter() + .map(|p| emitted_binder(p.as_str(), Some(map))) + .collect(); format!("<{}>", escaped.join(", ")) } } @@ -395,6 +524,7 @@ fn is_ts_property_identifier(name: &str) -> bool { /// vars; a class type var is already in scope on the enclosing class. fn fn_type_sig( generics: &[String], + generic_map: &TypeVarMap, names: &[&str], tys: &[TranslatedType], defaults: &[Option], @@ -425,7 +555,11 @@ fn fn_type_sig( } else { ret_expr.to_string() }; - format!("{}({}) => {ret}", generic_decl(generics), params.join(", ")) + format!( + "{}({}) => {ret}", + generic_decl(generics, generic_map), + params.join(", ") + ) } // ── Public entry point ── @@ -439,9 +573,13 @@ pub(crate) fn render_index_ts( ) -> String { let ctx = TranslateCtx { current_leaf: body.leaf.clone(), + type_var_map: None, }; let mut state = RenderState::default(); let callable_child_aliases = body.callable_child_aliases(kids); + // Module-scope bindings a generic binder must not shadow. Computed once per + // leaf and only ever consulted for a reserved-word type parameter. + let module_names = module_scope_names(body, kids); // Render symbol bodies first so the import preamble can be computed. let mut body_str = String::new(); @@ -456,6 +594,7 @@ pub(crate) fn render_index_ts( &ctx, &mut state, &callable_child_aliases, + &module_names, runtime_package, ); prev = Some(key); @@ -639,6 +778,7 @@ fn render_symbol_ts( ctx: &TranslateCtx, state: &mut RenderState, callable_child_aliases: &BTreeMap, + module_names: &BTreeSet, runtime_package: &str, ) { match sym { @@ -646,7 +786,7 @@ fn render_symbol_ts( if let Some(rust_name) = runtime_owned_reexport_name(c) { render_media_reexport_ts(out, &c.name, rust_name, runtime_package); } else { - render_class_ts(out, c, ctx, state); + render_class_ts(out, c, ctx, state, module_names); } } EmittedSymbol::Enum(e) => render_enum(out, e), @@ -656,6 +796,7 @@ fn render_symbol_ts( f, ctx, state, + module_names, callable_child_aliases.get(&f.name).map(String::as_str), ), } @@ -701,16 +842,22 @@ fn render_class_ts( c: &TypeScriptClass, ctx: &TranslateCtx, state: &mut RenderState, + module_names: &BTreeSet, ) { write_class_doc(out, c); - let generics = generic_decl(&c.generic_params); + // The class's type parameters are one generic scope: allocate their binders + // once, then render the `<…>` list, every field type, and every instance + // method signature through the same map. + let class_map = std::rc::Rc::new(allocate_binders(&c.generic_params, None, module_names)); + let class_ctx = ctx.with_type_vars(&class_map); + let generics = generic_decl(&c.generic_params, &class_map); // Translate each property type once; reuse for field + constructor. let props: Vec<(&str, TranslatedType)> = c .properties .iter() .map(|p| { - let t = translate_ty(&p.ty, ctx); + let t = translate_ty(&p.ty, &class_ctx); state.merge(&t); (p.name.as_str(), t) }) @@ -771,10 +918,26 @@ fn render_class_ts( // Static + instance method bindings, as class fields. for m in &c.static_methods { - render_method_binding_ts(out, m, &c.generic_params, ctx, state); + render_method_binding_ts( + out, + m, + &c.generic_params, + &class_map, + ctx, + state, + module_names, + ); } for m in &c.instance_methods { - render_method_binding_ts(out, m, &c.generic_params, ctx, state); + render_method_binding_ts( + out, + m, + &c.generic_params, + &class_map, + ctx, + state, + module_names, + ); } out.push_str("}\n"); @@ -844,14 +1007,35 @@ fn render_method_binding_ts( out: &mut String, m: &TypeScriptMethodBinding, class_generics: &[String], + class_map: &TypeVarMap, ctx: &TranslateCtx, state: &mut RenderState, + module_names: &BTreeSet, ) { write_doc_with_raises(out, m.docstring.as_deref(), &m.raises_names); - let (names, tys, defaults, ret) = binding_surface(m, ctx, state); let is_async = m.mode == SyncAsync::Async; let sig_generics = method_sig_generics(m, class_generics); - let sig = fn_type_sig(&sig_generics, &names, &tys, &defaults, &ret.expr, is_async); + // A static method re-declares the class parameters as its own (TS2302), so + // its scope is the flat `sig_generics` list with no enclosing map. An + // instance method binds only its own parameters on top of the class scope, + // so the class map is the outer scope and its binders are reserved against. + // `sig_generics` is exactly the scope's own parameter list in both cases. + let outer = match m.kind { + MethodKind::Static => None, + MethodKind::Instance => Some(class_map), + }; + let sig_map = std::rc::Rc::new(allocate_binders(&sig_generics, outer, module_names)); + let method_ctx = ctx.with_type_vars(&sig_map); + let (names, tys, defaults, ret) = binding_surface(m, &method_ctx, state); + let sig = fn_type_sig( + &sig_generics, + &sig_map, + &names, + &tys, + &defaults, + &ret.expr, + is_async, + ); let required_params = m.runtime_required_names(); let optional_params = m.optional_names(); let required_params_lit = param_names_literal(&required_params); @@ -894,25 +1078,30 @@ fn render_function_ts( f: &TypeScriptFunction, ctx: &TranslateCtx, state: &mut RenderState, + module_names: &BTreeSet, child_namespace_alias: Option<&str>, ) { write_doc_with_raises(out, f.docstring.as_deref(), &f.raises_names); state.uses_define_function = true; + // A free function's type parameters are one generic scope of their own. + let fn_map = std::rc::Rc::new(allocate_binders(&f.generic_params, None, module_names)); + let fn_ctx = ctx.with_type_vars(&fn_map); let tys: Vec = f .arg_tys .iter() .map(|t| { - let tt = translate_ty(t, ctx); + let tt = translate_ty(t, &fn_ctx); state.merge(&tt); tt }) .collect(); - let ret = translate_ty(&f.return_ty, ctx); + let ret = translate_ty(&f.return_ty, &fn_ctx); state.merge(&ret); let names: Vec<&str> = f.param_names.iter().map(String::as_str).collect(); let is_async = f.mode == SyncAsync::Async; let sig = fn_type_sig( &f.generic_params, + &fn_map, &names, &tys, &f.arg_defaults, @@ -1145,11 +1334,114 @@ mod tests { #[test] fn generic_decl_escapes_reserved_type_parameters() { - assert_eq!(generic_decl(&[]), ""); - assert_eq!(generic_decl(&["T".to_string()]), ""); - assert_eq!( - generic_decl(&["package".to_string(), "T".to_string()]), - "" + let decl = |params: &[&str]| { + let raw: Vec = params.iter().map(ToString::to_string).collect(); + let map = allocate_binders(&raw, None, &BTreeSet::new()); + generic_decl(&raw, &map) + }; + assert_eq!(decl(&[]), ""); + assert_eq!(decl(&["T"]), ""); + assert_eq!(decl(&["package", "T"]), ""); + } + + /// The `{package, package_}` twin: the stateless escape maps BOTH raw names + /// onto `package_`, so the binder list would read `` — + /// TS2300 duplicate identifier, which kills the whole generated file. The + /// allocator reserves the sibling raw name, so `package` bumps past it. + #[test] + fn reserved_and_underscore_twin_get_distinct_binders() { + let raw = vec!["package".to_string(), "package_".to_string()]; + // Control: the stateless escape collapses the twin. + assert_eq!(safe_decl_name("package"), safe_decl_name("package_")); + + let map = allocate_binders(&raw, None, &BTreeSet::new()); + assert_eq!(map.get("package").map(String::as_str), Some("package__")); + assert_eq!(map.get("package_").map(String::as_str), Some("package_")); + assert_eq!(generic_decl(&raw, &map), ""); + } + + /// Declaration order does not matter: whichever reserved-word parameter is + /// allocated first still bumps past the sibling that maps to itself. + #[test] + fn twin_binders_are_distinct_in_either_declaration_order() { + let raw = vec!["package_".to_string(), "package".to_string()]; + let map = allocate_binders(&raw, None, &BTreeSet::new()); + assert_eq!(generic_decl(&raw, &map), ""); + } + + /// A binder never lands on a module-scope declaration name in the same leaf. + #[test] + fn binder_does_not_shadow_a_module_scope_declaration() { + let module_names: BTreeSet = ["package_".to_string()].into_iter().collect(); + let raw = vec!["package".to_string()]; + let map = allocate_binders(&raw, None, &module_names); + assert_eq!(generic_decl(&raw, &map), ""); + } + + /// Keyword-free schemas are untouched: a non-reserved raw name maps to + /// itself unconditionally, so the reservation set is never consulted and + /// output stays byte-identical to the stateless escape it replaces. + #[test] + fn non_reserved_binders_are_never_bumped() { + let module_names: BTreeSet = + ["T".to_string(), "U".to_string()].into_iter().collect(); + let raw = vec!["T".to_string(), "U".to_string()]; + let map = allocate_binders(&raw, None, &module_names); + assert_eq!(generic_decl(&raw, &map), ""); + } + + /// End-to-end through `render_index_ts`: a generic class declaring both + /// `package` and `package_` renders distinct binders, and every use site + /// (field types, the constructor init object) resolves to the binder its + /// declaration allocated rather than re-deriving a colliding escape. + #[test] + fn generic_class_with_reserved_twin_renders_distinct_binders() { + let type_var = |n: &str| { + Ty::TypeVar( + baml_codegen_types::ParamTy::new(0, BaseName::new(n)), + baml_base::TyAttr::EMPTY, + ) + }; + let c = TypeScriptClass { + name: "Pair".to_string(), + source: name("user", &["lorem"], "Pair"), + generic_params: vec!["package".to_string(), "package_".to_string()], + docstring: None, + properties: vec![ + crate::emit::class::TypeScriptClassProperty { + name: "first".to_string(), + ty: type_var("package"), + docstring: None, + }, + crate::emit::class::TypeScriptClassProperty { + name: "second".to_string(), + ty: type_var("package_"), + docstring: None, + }, + ], + static_methods: Vec::new(), + instance_methods: Vec::new(), + }; + let b = body(&["lorem"], vec![EmittedSymbol::Class(c)]); + let ts = render_index_ts(&b, &BTreeSet::new(), false, TEST_RUNTIME_PACKAGE); + + assert!( + ts.contains("export class Pair {"), + "binders collided or were not allocated:\n{ts}" + ); + assert!( + ts.contains("first!: package__;"), + "`package` use site did not resolve to its binder:\n{ts}" + ); + assert!( + ts.contains("second!: package_;"), + "`package_` use site did not resolve to its binder:\n{ts}" + ); + // The wire channels stay RAW — the encoder positions `$types` bindings + // by the BAML spelling, not the TypeScript identifier. + assert!( + ts.contains("static readonly $generic = [\"package\", \"package_\"] as const;"), + "raw generic names must survive on the wire channel:\n{ts}" ); } diff --git a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/translate_ty.rs b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/translate_ty.rs index aaca45af334..dbdfe98e22b 100644 --- a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/translate_ty.rs +++ b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/translate_ty.rs @@ -16,13 +16,13 @@ //! type aliases natively, so — unlike the Python port — there is no //! self-ref quoting or `defer_name_refs`. //! -use std::collections::BTreeSet; +use std::{collections::BTreeSet, rc::Rc}; use baml_base::{Literal, MediaKind}; use baml_codegen_types::{CodegenFunctionParamMode, Name, Ty}; use crate::{ - leaf::safe_decl_name, + leaf::{TypeVarMap, emitted_binder, safe_decl_name}, routing::{LeafPath, route_class_ref}, ts_string, }; @@ -30,6 +30,26 @@ use crate::{ #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct TranslateCtx { pub(crate) current_leaf: LeafPath, + /// Raw→emitted type-parameter map for the generic scope whose body is being + /// translated (one class, one free function, or one method). `Ty::TypeVar` + /// resolves through it so a `{package, package_}` twin's references match the + /// distinct `{package__, package_}` binders the declaration allocated. + /// `None` when no generic scope is active — resolution then falls back to the + /// stateless [`crate::leaf::safe_decl_name`], which is exact for a scope that + /// binds no type parameters and so cannot carry a twin (type-alias bodies, + /// non-generic classes and callables). Mirrors the Python SDK's + /// `TranslateCtx::type_var_map`. + pub(crate) type_var_map: Option>, +} + +impl TranslateCtx { + /// The same leaf, with `map` installed as the active generic scope. + pub(crate) fn with_type_vars(&self, map: &Rc) -> Self { + Self { + current_leaf: self.current_leaf.clone(), + type_var_map: Some(Rc::clone(map)), + } + } } #[derive(Debug, Clone, PartialEq, Eq, Default)] @@ -163,8 +183,13 @@ pub(crate) fn translate_ty(ty: &Ty, ctx: &TranslateCtx) -> TranslatedType { } Ty::TypeAlias(name, _) => render_name_ref(name, ctx), // A type parameter is a binding identifier at its declaration site - // (`leaf::generic_decl`), so the use site takes the same escape. - Ty::TypeVar(name, _) => TranslatedType::bare(safe_decl_name(name.as_str())), + // (`leaf::generic_decl`), so the use site must land on exactly the + // identifier that site allocated. The scope map wins; the stateless + // escape is the fallback only for scopes that bind no type parameters, + // where no twin can exist. + Ty::TypeVar(name, _) => { + TranslatedType::bare(emitted_binder(name.as_str(), ctx.type_var_map.as_deref())) + } Ty::Function { params, ret, .. } => { let ret_t = translate_ty(ret, ctx); @@ -308,6 +333,7 @@ mod tests { fn ctx(segments: &[&str]) -> TranslateCtx { TranslateCtx { current_leaf: leaf(segments), + type_var_map: None, } } fn name(pkg: &str, namespace_path: &[&str], bare_name: &str) -> Name { From 6e4d0d4c38c13bf3221e85c56dd94f6577c4539a Mon Sep 17 00:00:00 2001 From: addiplus Date: Fri, 14 Aug 2026 21:41:05 -0700 Subject: [PATCH 3/3] test(typescript): cover method-scope binder allocation `render_method_binding_ts` allocates a method's binders through one of two arms: an instance method passes the class map as the enclosing scope, and a static method passes no enclosing scope because it re-declares the class parameters as its own (TS2302) in the flattened list `method_sig_generics` builds. Neither arm had a test. Every existing binder test calls `allocate_binders` with no enclosing scope, and the one end-to-end generic class test builds a class with empty `static_methods` and `instance_methods`, so the outer-scope reservation loop in `allocate_binders` was never reached from a test at all. `instance_method_binder_bumps_past_the_outer_class_scope` allocates a method scope that re-declares `package` on top of a class map that already binds the `{package, package_}` twin. The method binder has to clear the enclosing scope's raw names and the binders already allocated for them, so it lands on `package___`. Landing on `package_` instead would silently retarget every class-level reference that resolves through that binder, which is a wrong-type error rather than a parse error, so the compiler would not catch it. `static_method_scope_allocates_the_flattened_list_with_no_outer` runs a `MethodKind::Static` binding through `method_sig_generics` on a generic class, checks that the flattened list allocates three distinct binders with no enclosing scope, and checks that re-declaring the class parameters reproduces the class binders exactly. It then allocates the same list WITH the class map as an enclosing scope as a control, which bumps `package` one further and shows what the static arm avoids by passing none. Disabling the outer-scope reservation loop fails both new tests and no existing one. Also states one bound on `safe_decl_name` in its doc comment: a leaf declaring both a reserved word and its underscore-suffixed twin collapses them onto one identifier. That is not a regression, since at base the same leaf emitted `export class import {}` and TS1359 already killed the whole generated file. --- .../sdkgen_typescript_shared/src/leaf.rs | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs index 9f81b303d80..fe475c1a946 100644 --- a/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs +++ b/baml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rs @@ -469,6 +469,12 @@ pub(crate) fn safe_param_name(name: &str) -> String { /// `translate_ty::render_name_ref` re-derives the reference from the IR at /// every cross-reference rather than reading the emitted declaration back. /// +/// One bound follows from that statelessness: a leaf declaring BOTH a reserved +/// word and its `_`-suffixed twin (`import` alongside `import_`) collapses them +/// onto a single identifier, which is not a regression because at base that same +/// leaf emitted `export class import {}`, a TS1359 parse error that already +/// killed the whole generated file. +/// /// Only the TypeScript identifier moves. Wire identity travels on separate /// channels and is untouched: `defineFunction` is handed `baml_fqn`, /// `_typemap.ts` is keyed on `TypeScriptClass::source`, and enum member values @@ -1445,6 +1451,140 @@ mod tests { ); } + /// Instance-method scope, the `Some(class_map)` arm of the `outer` match in + /// `render_method_binding_ts`. A method that re-declares a reserved-word + /// parameter must bump clear of BOTH the class's raw parameter names and the + /// binders the class already allocated for them. Landing on `package_` would + /// silently retarget every class-level reference that resolves through that + /// binder, which is a wrong-type error the compiler cannot catch. + #[test] + fn instance_method_binder_bumps_past_the_outer_class_scope() { + let module_names: BTreeSet = ["Pair".to_string()].into_iter().collect(); + let class_raw = vec!["package".to_string(), "package_".to_string()]; + let class_map = allocate_binders(&class_raw, None, &module_names); + assert_eq!( + class_map.get("package").map(String::as_str), + Some("package__") + ); + assert_eq!( + class_map.get("package_").map(String::as_str), + Some("package_") + ); + + // The instance arm: the method's OWN parameters only, with the class map + // as the enclosing scope. + let method_raw = vec!["package".to_string()]; + let sig_map = allocate_binders(&method_raw, Some(&class_map), &module_names); + + // KILL ASSERTION. Without the outer-scope reservation loop the candidate + // `package_` is unreserved, so the method binder lands exactly on the + // class's own `package_` binder. Reserving the outer raw names AND the + // outer binders is what pushes it to `package___`. + assert_eq!( + sig_map.get("package").map(String::as_str), + Some("package___"), + "method binder did not bump clear of the outer scope: {sig_map:?}" + ); + + // Stated the other way: clear of every outer raw name and every outer + // binder, not just the one this fixture happens to collide with. + let allocated = sig_map.get("package").cloned().unwrap(); + for (raw, emitted) in &class_map { + assert_ne!( + &allocated, raw, + "method binder collided with an outer raw name" + ); + assert_ne!( + &allocated, emitted, + "method binder collided with an outer binder" + ); + } + + // The enclosing scope's entries survive into the method map, so a + // class-level reference inside the method body still resolves. + assert_eq!( + sig_map.get("package_").map(String::as_str), + Some("package_") + ); + assert_eq!(generic_decl(&method_raw, &sig_map), ""); + } + + /// Static-method scope, the `None` arm of the same match. A static member + /// cannot reference the class's type parameters (TS2302), so + /// `method_sig_generics` flattens the class params and the method's own into + /// ONE list that is allocated with no enclosing scope. The flat list must + /// still allocate distinct binders, and re-declaring the class params must + /// reproduce the class's binders exactly. + #[test] + fn static_method_scope_allocates_the_flattened_list_with_no_outer() { + let module_names: BTreeSet = ["Box".to_string()].into_iter().collect(); + let class_raw = vec!["package".to_string(), "package_".to_string()]; + let class_map = allocate_binders(&class_raw, None, &module_names); + + let m = TypeScriptMethodBinding { + name: "build".to_string(), + baml_fqn: "user.lorem.Box.build".to_string(), + mode: SyncAsync::Sync, + kind: MethodKind::Static, + required_args: Vec::new(), + optional_args: Vec::new(), + return_ty: Ty::Int { + attr: baml_base::TyAttr::EMPTY, + }, + generic_params: vec!["T".to_string()], + docstring: None, + raises_names: Vec::new(), + }; + + let sig_generics = method_sig_generics(&m, &class_raw); + assert_eq!( + sig_generics, + vec![ + "package".to_string(), + "package_".to_string(), + "T".to_string() + ], + "static signature must re-declare the class params ahead of its own" + ); + + let sig_map = allocate_binders(&sig_generics, None, &module_names); + assert_eq!( + sig_map.get("package").map(String::as_str), + Some("package__") + ); + assert_eq!( + sig_map.get("package_").map(String::as_str), + Some("package_") + ); + assert_eq!(sig_map.get("T").map(String::as_str), Some("T")); + // Three raw names, three distinct binders. + let distinct: BTreeSet<&String> = sig_map.values().collect(); + assert_eq!(distinct.len(), sig_map.len()); + assert_eq!( + generic_decl(&sig_generics, &sig_map), + "" + ); + // Re-declaring the class params reproduces the class binders exactly. + // That is what passing no enclosing scope buys. + assert_eq!(sig_map.get("package"), class_map.get("package")); + assert_eq!(sig_map.get("package_"), class_map.get("package_")); + + // KILL ASSERTION, as a control on the arm this branch deliberately does + // not take. Allocating the same flat list WITH the class map as an + // enclosing scope reserves the outer binders, so `package` bumps one + // further to `package___` and the static signature drifts off the class + // declaration. If the outer-scope reservation loop were removed, this + // allocation would collapse back onto the no-outer result and both + // assertions below would fail. + let with_outer = allocate_binders(&sig_generics, Some(&class_map), &module_names); + assert_eq!( + with_outer.get("package").map(String::as_str), + Some("package___"), + "outer-scope reservation did not apply: {with_outer:?}" + ); + assert_ne!(with_outer.get("package"), sig_map.get("package")); + } + #[test] fn class_renders_real_body() { let b = body(