Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(transformer/class-properties): correctly resolve private fields pointing to private accessors #8047

Merged
merged 1 commit into from
Dec 23, 2024
Merged

fix(transformer/class-properties): correctly resolve private fields pointing to private accessors #8047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions crates/oxc_transformer/src/es2022/class_properties/class.rs
View file
Open in desktop
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
13 changes: 11 additions & 2 deletions crates/oxc_transformer/src/es2022/class_properties/class_details.rs
View file
Open in desktop
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
20 changes: 12 additions & 8 deletions crates/oxc_transformer/src/es2022/class_properties/private_field.rs
View file
Open in desktop
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
Loading