diff --git a/src/ast/g.rs b/src/ast/g.rs index 96f0a28d6390..5ab4abcb10e0 100644 --- a/src/ast/g.rs +++ b/src/ast/g.rs @@ -224,7 +224,7 @@ impl Property { Some(value) => Some(value.deep_clone(bump)?), None => None, }, - ts_metadata: self.ts_metadata.clone(), + ts_metadata: self.ts_metadata.deep_clone(bump), }) } } @@ -314,7 +314,7 @@ impl Fn { }, arguments_ref: self.arguments_ref, flags: self.flags, - return_ts_metadata: self.return_ts_metadata.clone(), + return_ts_metadata: self.return_ts_metadata.deep_clone(bump), }) } } @@ -358,7 +358,7 @@ impl Arg { None => None, }, is_typescript_ctor_field: self.is_typescript_ctor_field, - ts_metadata: self.ts_metadata.clone(), + ts_metadata: self.ts_metadata.deep_clone(bump), }) } } diff --git a/src/ast/ts.rs b/src/ast/ts.rs index 2d769002e203..7e2dabdda8b7 100644 --- a/src/ast/ts.rs +++ b/src/ast/ts.rs @@ -150,7 +150,7 @@ impl Data { // Data-only; the parser-state predicates that depend on `P` stay in // `bun_js_parser::typescript`. -#[derive(Clone, Default)] +#[derive(Clone, Copy, Default)] pub enum Metadata { #[default] MNone, @@ -171,14 +171,24 @@ pub enum Metadata { MSymbol, MPromise, MIdentifier(Ref), - // A heap `Vec` is used here because `Metadata` is lifetime-free. - // Decorator metadata is rare and the lists are tiny. - MDot(Vec), + /// "a.b.c": one ref per part, arena-owned. + MDot(crate::nodes::StoreSlice), } impl Metadata { pub const DEFAULT: Self = Metadata::MNone; + /// Copy the `MDot` refs into `bump` so the clone outlives the source arena. + pub(crate) fn deep_clone(self, bump: &bun_alloc::Arena) -> Self { + match self { + Metadata::MDot(refs) => { + let copied: &mut [Ref] = bump.alloc_slice_copy(refs.slice()); + Metadata::MDot(crate::nodes::StoreSlice::new_mut(copied)) + } + other => other, + } + } + // the logic in finish_union, merge_union, finish_intersection and merge_intersection is // translated from: // https://github.com/microsoft/TypeScript/blob/e0a324b0503be479f2b33fd2e17c6e86c94d1297/src/compiler/transformers/typeSerializer.ts#L402 diff --git a/src/js_parser/lower/lower_decorators.rs b/src/js_parser/lower/lower_decorators.rs index b622c31c9e0d..dd81c53da4bb 100644 --- a/src/js_parser/lower/lower_decorators.rs +++ b/src/js_parser/lower/lower_decorators.rs @@ -78,13 +78,7 @@ fn prop_copy(p: &Property) -> Property { ts_decorators: bun_alloc::AstAlloc::vec(), key: p.key, value: p.value, - // SAFETY: this duplicates ownership of any heap allocation inside - // `Metadata` (`MDot` owns a global-heap `Vec`), but the source - // `Property` is an arena-resident AST node whose `Drop` never runs - // (AST stores are bulk-freed without dropping — see the - // `bun_alloc::ast_alloc` module docs), so at most one of the two - // copies ever reaches drop glue; no double free. - ts_metadata: unsafe { core::ptr::read(&raw const p.ts_metadata) }, + ts_metadata: p.ts_metadata, } } @@ -103,8 +97,7 @@ fn prop_full_copy(p: &Property) -> Property { ts_decorators, key: p.key, value: p.value, - // SAFETY: see `prop_copy`. - ts_metadata: unsafe { core::ptr::read(&raw const p.ts_metadata) }, + ts_metadata: p.ts_metadata, } } diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index 7aba6f7d8d38..683c7b544e32 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -7643,7 +7643,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O .alloc_slice_fill_default::(constructor_args.len()); for (i, ca) in constructor_args.iter().enumerate() { param_array[i] = self - .serialize_metadata(ca.ts_metadata.clone()) + .serialize_metadata(ca.ts_metadata) .expect("unreachable"); } let items = ExprNodeList::from_arena_slice(param_array); @@ -7735,7 +7735,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O { // design:type let v = self - .serialize_metadata(prop.ts_metadata.clone()) + .serialize_metadata(prop.ts_metadata) .expect("unreachable"); push_metadata!(b"design:type", v); } @@ -7754,7 +7754,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O .alloc_slice_fill_default::(method_args.len()); for (entry, method_arg) in args_array.iter_mut().zip(method_args) { *entry = self - .serialize_metadata(method_arg.ts_metadata.clone()) + .serialize_metadata(method_arg.ts_metadata) .expect("unreachable"); } let items = ExprNodeList::from_arena_slice(args_array); @@ -7769,7 +7769,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } { let v = self - .serialize_metadata(func.func.return_ts_metadata.clone()) + .serialize_metadata(func.func.return_ts_metadata) .expect("unreachable"); push_metadata!(b"design:returntype", v); } @@ -7786,7 +7786,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O .expect("infallible: variant checked"); { let v = self - .serialize_metadata(func.func.return_ts_metadata.clone()) + .serialize_metadata(func.func.return_ts_metadata) .expect("unreachable"); push_metadata!(b"design:type", v); } @@ -7821,7 +7821,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O .alloc_slice_fill_default::(method_args.len()); for (entry, method_arg) in args_array.iter_mut().zip(method_args) { *entry = self - .serialize_metadata(method_arg.ts_metadata.clone()) + .serialize_metadata(method_arg.ts_metadata) .expect("unreachable"); } let items = ExprNodeList::from_arena_slice(args_array); @@ -7836,7 +7836,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } if !method_args.is_empty() { let v = self - .serialize_metadata(method_args[0].ts_metadata.clone()) + .serialize_metadata(method_args[0].ts_metadata) .expect("unreachable"); push_metadata!(b"design:type", v); } @@ -7883,6 +7883,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } M::MPromise => ident!(b"Promise"), M::MIdentifier(ref_) => { + if let Some(enum_metadata) = self.ts_enum_metadata(ref_) { + return self.serialize_metadata(enum_metadata); + } self.record_usage(ref_); let e = if self.is_import_item.contains_key(&ref_) { self.new_expr( @@ -7899,6 +7902,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } M::MDot(refs) => { debug_assert!(refs.len() >= 2); + if let Some(enum_metadata) = self.ts_namespace_enum_metadata(&refs) { + return self.serialize_metadata(enum_metadata); + } // (refs.deinit(p.arena) — arena-backed; nothing to free in Rust) macro_rules! ref_name { @@ -8034,6 +8040,104 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O }) } + /// tsc serializes an enum type as `Number`, `String`, or `Object`, never as the enum object. + fn ts_enum_metadata(&mut self, ref_: Ref) -> Option { + let ref_ = self.resolve_metadata_ref(ref_)?; + if self.symbols[ref_.inner_index() as usize].kind != js_ast::symbol::Kind::TsEnum { + return None; + } + let js_ast::ts::Data::Namespace(members) = self.ref_to_ts_namespace_member.get(&ref_)? + else { + return None; + }; + Self::enum_members_metadata(members) + } + + /// `ts_enum_metadata` for `Ns.Inner.Enum` and for a member type `Enum.Member`. + fn ts_namespace_enum_metadata(&mut self, refs: &[Ref]) -> Option { + let root = self.resolve_metadata_ref(refs[0])?; + let js_ast::ts::Data::Namespace(mut members) = + *self.ref_to_ts_namespace_member.get(&root)? + else { + return None; + }; + for (i, &part) in refs[1..].iter().enumerate() { + let is_last = i + 2 == refs.len(); + let name = self.load_name_from_ref(part); + let map: &js_ast::TSNamespaceMemberMap = &members; + match map.get(name)?.data { + js_ast::ts::Data::Namespace(next) => members = next, + js_ast::ts::Data::EnumNumber(_) | js_ast::ts::Data::EnumProperty if is_last => { + return Some(bun_ast::ts::Metadata::MNumber); + } + js_ast::ts::Data::EnumString(_) if is_last => { + return Some(bun_ast::ts::Metadata::MString); + } + _ => return None, + } + } + // An empty map is an empty enum or an empty namespace. Only the enum is `Number`. + let map: &js_ast::TSNamespaceMemberMap = &members; + if map.count() == 0 { + let is_enum = self.ts_namespace_scopes.iter().any(|scope| { + scope.is_enum_scope + && core::ptr::eq::( + scope.exported_members.get(), + map, + ) + }); + return is_enum.then_some(bun_ast::ts::Metadata::MNumber); + } + Self::enum_members_metadata(map) + } + + /// Re-resolve the name from the class scope like `visit_expr`, then follow symbol links. + fn resolve_metadata_ref(&mut self, ref_: Ref) -> Option { + if ref_.tag() != js_ast::base::RefTag::Symbol { + return None; + } + let name = self.load_name_from_ref(ref_); + let found = self + .find_symbol_with_record_usage::(bun_ast::Loc::EMPTY, name) + .ok()? + .r#ref; + let mut ref_ = if found.is_empty() { ref_ } else { found }; + let mut symbol = &self.symbols[ref_.inner_index() as usize]; + while symbol.has_link() { + ref_ = symbol.link.get(); + symbol = &self.symbols[ref_.inner_index() as usize]; + } + Some(ref_) + } + + /// Computed members count as numbers (tsc allows no other), merged namespace entries do not. + fn enum_members_metadata( + members: &js_ast::TSNamespaceMemberMap, + ) -> Option { + let mut has_number = false; + let mut has_string = false; + let mut has_other = false; + for member in members.values() { + match member.data { + js_ast::ts::Data::EnumNumber(_) | js_ast::ts::Data::EnumProperty => { + has_number = true + } + js_ast::ts::Data::EnumString(_) => has_string = true, + js_ast::ts::Data::Property | js_ast::ts::Data::Namespace(_) => has_other = true, + } + } + if has_other && !has_number && !has_string { + return None; + } + Some(if has_string && has_number { + bun_ast::ts::Metadata::MObject + } else if has_string { + bun_ast::ts::Metadata::MString + } else { + bun_ast::ts::Metadata::MNumber + }) + } + #[cold] #[inline(never)] pub(crate) fn wrap_identifier_namespace(&mut self, loc: bun_ast::Loc, r#ref: Ref) -> Expr { diff --git a/src/js_parser/parse/parse_skip_typescript.rs b/src/js_parser/parse/parse_skip_typescript.rs index 300deba4d4c0..93d2c25a89df 100644 --- a/src/js_parser/parse/parse_skip_typescript.rs +++ b/src/js_parser/parse/parse_skip_typescript.rs @@ -251,6 +251,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O return Err(crate::Error::StackOverflow); } + // Refs of a qualified name ("a.b.c"); `ManuallyDrop` keeps the buffer `MDot` points at. + let mut dot_path: Option>> = None; + loop { match self.lexer.token { T::TNumericLiteral => { @@ -797,10 +800,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O self.lexer.next()?; if GET_METADATA { - let mut left = (**result + let mut left = **result .as_mut() - .expect("infallible: GET_METADATA implies Some")) - .clone(); + .expect("infallible: GET_METADATA implies Some"); if let Some(final_) = Metadata::finish_union(&mut left, |r| self.load_name_from_ref(r)) { @@ -842,10 +844,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O self.lexer.next()?; if GET_METADATA { - let mut left = (**result + let mut left = **result .as_mut() - .expect("infallible: GET_METADATA implies Some")) - .clone(); + .expect("infallible: GET_METADATA implies Some"); if let Some(final_) = Metadata::finish_intersection(&mut left, |r| self.load_name_from_ref(r)) { @@ -904,23 +905,32 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O let r = result .as_deref_mut() .expect("infallible: GET_METADATA implies Some"); - match r { + // `Metadata` is `Copy`; `head` borrows the arena, not `r`. + let (head, extends_path): (Option<&[Ref]>, bool) = match *r { Metadata::MIdentifier(id_ref) => { - let id_ref = *id_ref; - let mut dot: Vec = Vec::with_capacity(2); - dot.push(id_ref); - let find_result = self.find_symbol(bun_ast::Loc::EMPTY, ident)?; - dot.push(find_result.r#ref); - *r = Metadata::MDot(dot); + (Some(self.arena.alloc_slice_copy(&[id_ref])), false) } - Metadata::MDot(dot) => { - if self.lexer.is_identifier_or_keyword() { - let find_result = - self.find_symbol(bun_ast::Loc::EMPTY, ident)?; - dot.push(find_result.r#ref); - } + Metadata::MDot(dot) if self.lexer.is_identifier_or_keyword() => { + (Some(dot.slice()), true) } - _ => {} + _ => (None, false), + }; + if let Some(head) = head { + let find_result = self.find_symbol(bun_ast::Loc::EMPTY, ident)?; + // A path that came from a nested type (`(a.b).c`) starts a new vec. + let path = match &mut dot_path { + Some(path) if extends_path => path, + slot => { + let mut path = bun_alloc::ArenaVec::::with_capacity_in( + (head.len() + 1).max(4), + self.arena, + ); + path.extend_from_slice(head); + slot.insert(core::mem::ManuallyDrop::new(path)) + } + }; + path.push(find_result.r#ref); + *r = Metadata::MDot(bun_ast::StoreSlice::new_mut(path.as_mut_slice())); } } diff --git a/src/js_parser/parse/parse_typescript.rs b/src/js_parser/parse/parse_typescript.rs index eb4d49f92c69..e67688b4f7a9 100644 --- a/src/js_parser/parse/parse_typescript.rs +++ b/src/js_parser/parse/parse_typescript.rs @@ -661,13 +661,21 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O let value_name = value.name; let value_loc = value.loc; + // Decorator metadata can read this map before the visit pass fills it in. + let data = match value.value.map(|v| v.data) { + Some(js_ast::ExprData::ENumber(num)) => { + TSNamespaceMemberData::EnumNumber(num.value()) + } + Some(js_ast::ExprData::EString(str_)) => TSNamespaceMemberData::EnumString(str_), + _ => TSNamespaceMemberData::EnumProperty, + }; values.push(value); exported_members.put( value_name.slice(), TSNamespaceMember { loc: value_loc, - data: TSNamespaceMemberData::EnumProperty, + data, }, )?; diff --git a/test/bundler/transpiler/decorator-metadata.test.ts b/test/bundler/transpiler/decorator-metadata.test.ts index 41d1b95fc8a1..58436d37baa7 100644 --- a/test/bundler/transpiler/decorator-metadata.test.ts +++ b/test/bundler/transpiler/decorator-metadata.test.ts @@ -1,5 +1,30 @@ import "reflect-metadata"; +namespace MetadataNs { + export enum Inner { + X, + } + export namespace Deep { + export enum Inner { + S = "s", + } + } + export enum Empty {} + export namespace EmptyNs { + init(); + } +} +function init() {} + +enum WithHelpers { + A, +} +namespace WithHelpers { + export function from(value: number) { + return value; + } +} + describe("decorator metadata", () => { test("type serialization", () => { function d1() {} @@ -543,4 +568,160 @@ describe("decorator metadata", () => { expect(Reflect.getMetadata("design:type", A.prototype, "method4")).toBe(Function); expect(Reflect.getMetadata("design:returntype", A.prototype, "method4")).toBeUndefined(); }); + + // tsc serializes an enum type as Number, String, or Object, never as the enum object. + test("enum types", () => { + function d2(target: any, key: string) {} + + class Declared { + @d2 + later: Later; + @d2 + laterString: LaterString; + @d2 + laterAlias: LaterAlias; + @d2 + laterNested: LaterNs.Str; + @d2 + laterNestedMember: LaterNs.Str.A; + } + + enum Later { + A, + B, + } + enum LaterString { + A = "a", + } + enum LaterAlias { + A = "a", + B = A, + } + enum Numeric { + A, + B = 5, + C = A + B, + } + enum Str { + A = "a", + B = "b", + } + enum Mixed { + A = 1, + B = "b", + } + const enum Const { + X, + Y, + } + enum Computed { + A = (1 + 1) * 2, + B = Math.max(1, 2), + } + enum Merged { + A, + } + enum Merged { + B = "b", + } + enum Empty {} + + class A { + @d2 + numeric: Numeric; + @d2 + str: Str; + @d2 + mixed: Mixed; + @d2 + constEnum: Const; + @d2 + computed: Computed; + @d2 + merged: Merged; + @d2 + withHelpers: WithHelpers; + @d2 + empty: Empty; + @d2 + nsEmpty: MetadataNs.Empty; + @d2 + nsEmptyNs: MetadataNs.EmptyNs; + @d2 + nullable: Numeric | null; + @d2 + optional?: Str; + @d2 + array: Numeric[]; + @d2 + nsInner: MetadataNs.Inner; + @d2 + nsDeep: MetadataNs.Deep.Inner; + @d2 + ns: typeof MetadataNs; + @d2 + member: Numeric.B; + @d2 + strMember: Str.A; + @d2 + nsMember: MetadataNs.Deep.Inner.S; + + @d2 + method(a: Numeric, b: Str = Str.A, c: Mixed, ...rest: Numeric[]): Numeric { + return Numeric.A; + } + } + + const type = (target: any, key: string) => Reflect.getMetadata("design:type", target, key); + + expect(type(A.prototype, "numeric")).toBe(Number); + expect(type(A.prototype, "str")).toBe(String); + expect(type(A.prototype, "mixed")).toBe(Object); + expect(type(A.prototype, "constEnum")).toBe(Number); + expect(type(A.prototype, "computed")).toBe(Number); + expect(type(A.prototype, "merged")).toBe(Object); + expect(type(A.prototype, "withHelpers")).toBe(Number); + expect(type(A.prototype, "empty")).toBe(Number); + expect(type(A.prototype, "nsEmpty")).toBe(Number); + // an empty namespace is not a type in tsc; bun keeps the namespace object like before + expect(type(A.prototype, "nsEmptyNs")).toBe(MetadataNs.EmptyNs); + expect(type(A.prototype, "nullable")).toBe(Number); + expect(type(A.prototype, "optional")).toBe(String); + expect(type(A.prototype, "array")).toBe(Array); + expect(type(A.prototype, "nsInner")).toBe(Number); + expect(type(A.prototype, "nsDeep")).toBe(String); + expect(type(A.prototype, "ns")).toBe(Object); + expect(type(A.prototype, "member")).toBe(Number); + expect(type(A.prototype, "strMember")).toBe(String); + expect(type(A.prototype, "nsMember")).toBe(String); + + expect(Reflect.getMetadata("design:paramtypes", A.prototype, "method")).toEqual([Number, String, Object, Array]); + expect(Reflect.getMetadata("design:returntype", A.prototype, "method")).toBe(Number); + + expect(type(Declared.prototype, "later")).toBe(Number); + expect(type(Declared.prototype, "laterString")).toBe(String); + expect(type(Declared.prototype, "laterAlias")).toBe(String); + expect(type(Declared.prototype, "laterNested")).toBe(String); + expect(type(Declared.prototype, "laterNestedMember")).toBe(String); + }); + + // Unions are folded while the type is parsed, before the enum can be classified. + test.todo("enum in a union with its own primitive", () => { + function d2(target: any, key: string) {} + enum Numeric { + A, + } + class A { + @d2 + value: Numeric | number; + } + expect(Reflect.getMetadata("design:type", A.prototype, "value")).toBe(Number); + }); }); + +// Declared after the classes above so the enum is not visited yet when they are lowered. +namespace LaterNs { + export enum Str { + A = "a", + } +} diff --git a/test/integration/typegraphql/src/typegraphql.test.ts b/test/integration/typegraphql/src/typegraphql.test.ts index db09909dd126..eddde6855adf 100644 --- a/test/integration/typegraphql/src/typegraphql.test.ts +++ b/test/integration/typegraphql/src/typegraphql.test.ts @@ -56,5 +56,6 @@ test("correct reflect.metadata types for getters", () => { // is manually specified. expect(Reflect.getMetadata("design:type", User.prototype, "enum1")).toBe(Enum1); expect(Reflect.getMetadata("design:type", User.prototype, "enum2")).toBe(Enum2); - expect(Reflect.getMetadata("design:type", User.prototype, "enum3")).toBe(Enum3); + // an enum declared in the same file is classified like typescript does + expect(Reflect.getMetadata("design:type", User.prototype, "enum3")).toBe(String); });