Skip to content

Commit

Permalink
fix(transformer/class-properties): correctly resolve private fields p…
Browse files Browse the repository at this point in the history
…ointing to private accessors
  • Loading branch information
overlookmotel committed Dec 20, 2024
1 parent 88d36ca commit b2ec515
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 18 deletions.
27 changes: 19 additions & 8 deletions crates/oxc_transformer/src/es2022/class_properties/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
);
}

Expand Down Expand Up @@ -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?
}
}
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}

Expand Down Expand Up @@ -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,
};
}
Expand All @@ -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,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
};

Expand Down Expand Up @@ -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;
};

Expand Down Expand Up @@ -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;
};

Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit b2ec515

Please sign in to comment.