From e67cd056a86826538d88bd54da81568cd2cecad1 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Mon, 23 Dec 2024 03:44:10 +0000 Subject: [PATCH] fix(transformer/class-properties): correctly resolve private fields pointing to private accessors (#8047) We don't transform private accessors yet, but include them in private property resolution. This fixes some panics in TS conformance tests which use private accessors. --- .../src/es2022/class_properties/class.rs | 27 +++++++++++++------ .../es2022/class_properties/class_details.rs | 13 +++++++-- .../es2022/class_properties/private_field.rs | 20 ++++++++------ 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/crates/oxc_transformer/src/es2022/class_properties/class.rs b/crates/oxc_transformer/src/es2022/class_properties/class.rs index 5f1d8c764..d6ac7c6ea 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/class.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/class.rs @@ -92,7 +92,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { let binding = ctx.generate_uid_in_current_hoist_scope(&ident.name); private_props.insert( ident.name.clone(), - PrivateProp::new(binding, prop.r#static, false), + PrivateProp::new(binding, prop.r#static, false, false), ); } @@ -120,12 +120,23 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { let dummy_binding = BoundIdentifier::new(Atom::empty(), SymbolId::new(0)); private_props.insert( ident.name.clone(), - PrivateProp::new(dummy_binding, method.r#static, true), + PrivateProp::new(dummy_binding, method.r#static, true, false), ); } } - ClassElement::AccessorProperty(_) | ClassElement::TSIndexSignature(_) => { - // TODO: Need to handle these? + ClassElement::AccessorProperty(prop) => { + // TODO: Not sure what we should do here. + // Only added this to prevent panics in TS conformance tests. + if let PropertyKey::PrivateIdentifier(ident) = &prop.key { + let dummy_binding = BoundIdentifier::new(Atom::empty(), SymbolId::new(0)); + private_props.insert( + ident.name.clone(), + PrivateProp::new(dummy_binding, prop.r#static, true, true), + ); + } + } + ClassElement::TSIndexSignature(_) => { + // TODO: Need to handle this? } } } @@ -396,7 +407,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { self.ctx.statement_injector.insert_many_before( &stmt_address, private_props.iter().filter_map(|(name, prop)| { - if prop.is_method { + if prop.is_method || prop.is_accessor { return None; } @@ -411,7 +422,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { self.ctx.statement_injector.insert_many_before( &stmt_address, private_props.values().filter_map(|prop| { - if prop.is_static || prop.is_method { + if prop.is_static || prop.is_method || prop.is_accessor { return None; } @@ -528,7 +539,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { // TODO(improve-on-babel): Simplify this. if self.private_fields_as_properties { exprs.extend(private_props.iter().filter_map(|(name, prop)| { - if prop.is_method { + if prop.is_method || prop.is_accessor { return None; } @@ -542,7 +553,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { } else { let mut weakmap_symbol_id = None; exprs.extend(private_props.values().filter_map(|prop| { - if prop.is_method { + if prop.is_method || prop.is_accessor { return None; } diff --git a/crates/oxc_transformer/src/es2022/class_properties/class_details.rs b/crates/oxc_transformer/src/es2022/class_properties/class_details.rs index 3362f2b63..0289923c9 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/class_details.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/class_details.rs @@ -41,11 +41,17 @@ pub(super) struct PrivateProp<'a> { pub binding: BoundIdentifier<'a>, pub is_static: bool, pub is_method: bool, + pub is_accessor: bool, } impl<'a> PrivateProp<'a> { - pub fn new(binding: BoundIdentifier<'a>, is_static: bool, is_method: bool) -> Self { - Self { binding, is_static, is_method } + pub fn new( + binding: BoundIdentifier<'a>, + is_static: bool, + is_method: bool, + is_accessor: bool, + ) -> Self { + Self { binding, is_static, is_method, is_accessor } } } @@ -111,6 +117,7 @@ impl<'a> ClassesStack<'a> { class_bindings: &mut class.bindings, is_static: prop.is_static, is_method: prop.is_method, + is_accessor: prop.is_accessor, is_declaration: class.is_declaration, }; } @@ -133,6 +140,8 @@ pub(super) struct ResolvedPrivateProp<'a, 'b> { pub is_static: bool, /// `true` if is a private method pub is_method: bool, + /// `true` if is a private accessor property + pub is_accessor: bool, /// `true` if class which defines this property is a class declaration pub is_declaration: bool, } diff --git a/crates/oxc_transformer/src/es2022/class_properties/private_field.rs b/crates/oxc_transformer/src/es2022/class_properties/private_field.rs index 15b9c10db..a4140ccd3 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/private_field.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/private_field.rs @@ -53,10 +53,11 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { class_bindings, is_static, is_method, + is_accessor, is_declaration, } = self.classes_stack.find_private_prop(&field_expr.field); - if is_method { + if is_method || is_accessor { // TODO: Should not consume existing `PrivateFieldExpression` and then create a new one // which is identical to the original return Expression::PrivateFieldExpression(field_expr); @@ -184,10 +185,10 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { if self.private_fields_as_properties { // `object.#prop(arg)` -> `_classPrivateFieldLooseBase(object, _prop)[_prop](arg)` - let ResolvedPrivateProp { prop_binding, is_method, .. } = + let ResolvedPrivateProp { prop_binding, is_method, is_accessor, .. } = self.classes_stack.find_private_prop(&field_expr.field); - if is_method { + if is_method || is_accessor { return; } @@ -262,10 +263,11 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { class_bindings, is_static, is_method, + is_accessor, is_declaration, } = self.classes_stack.find_private_prop(&field_expr.field); - if is_method { + if is_method || is_accessor { return None; }; @@ -367,10 +369,11 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { class_bindings, is_static, is_method, + is_accessor, is_declaration, } = self.classes_stack.find_private_prop(&field_expr.field); - if is_method { + if is_method || is_accessor { return; }; @@ -752,10 +755,11 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { class_bindings, is_static, is_method, + is_accessor, is_declaration, } = self.classes_stack.find_private_prop(&field_expr.field); - if is_method { + if is_method || is_accessor { return; }; @@ -1547,10 +1551,10 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { // // ^^^^^^^^^^^^^ // ``` // But this is not needed, so we omit it. - let ResolvedPrivateProp { prop_binding, is_method, .. } = + let ResolvedPrivateProp { prop_binding, is_method, is_accessor, .. } = self.classes_stack.find_private_prop(&field_expr.field); - if is_method { + if is_method || is_accessor { return None; }