Skip to content

Commit

Permalink
refactor(transformer/class-properties): rename class_binding
Browse files Browse the repository at this point in the history
  • Loading branch information
overlookmotel committed Dec 3, 2024
1 parent 3dc46a8 commit f7d1be8
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 59 deletions.
8 changes: 4 additions & 4 deletions crates/oxc_transformer/src/es2022/class_properties/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,13 +342,13 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
if private_props.is_empty() {
self.private_props_stack.push(None);
} else {
let class_name_binding = match &self.class_name {
let class_binding = match &self.class_name {
ClassName::Binding(binding) => Some(binding.clone()),
ClassName::Name(_) => None,
};
self.private_props_stack.push(Some(PrivateProps {
props: private_props,
class_name_binding,
class_binding,
is_declaration: self.is_declaration,
}));
}
Expand Down Expand Up @@ -460,11 +460,11 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
self.insert_private_static_init_assignment(ident, value, ctx);
} else {
// Convert to assignment or `_defineProperty` call, depending on `loose` option
let ClassName::Binding(class_name_binding) = &self.class_name else {
let ClassName::Binding(class_binding) = &self.class_name else {
// Binding is initialized in 1st pass in `transform_class` when a static prop is found
unreachable!();
};
let assignee = class_name_binding.create_read_expression(ctx);
let assignee = class_binding.create_read_expression(ctx);
let init_expr = self.create_init_assignment(prop, value, assignee, true, ctx);
self.insert_expr_after_class(init_expr, ctx);
}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_transformer/src/es2022/class_properties/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,8 @@ struct PrivateProps<'a> {
/// Private properties for class. Indexed by property name.
// TODO(improve-on-babel): Order that temp vars are created in is not important. Use `FxHashMap` instead.
props: FxIndexMap<Atom<'a>, PrivateProp<'a>>,
/// Binding for class name
class_name_binding: Option<BoundIdentifier<'a>>,
/// Binding for class, if class has name
class_binding: Option<BoundIdentifier<'a>>,
/// `true` for class declaration, `false` for class expression
is_declaration: bool,
}
Expand Down
78 changes: 33 additions & 45 deletions crates/oxc_transformer/src/es2022/class_properties/private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
) -> Expression<'a> {
let prop_details = self.lookup_private_property(&field_expr.field);
// TODO: Should never be `None` - only because implementation is incomplete.
let Some((prop, class_name_binding, is_declaration)) = prop_details else {
let Some((prop, class_binding, is_declaration)) = prop_details else {
return Expression::PrivateFieldExpression(field_expr);
};
let prop_ident = prop.binding.create_read_expression(ctx);
Expand All @@ -54,21 +54,20 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

if prop.is_static {
// TODO: Ensure there are tests for nested classes with references to private static props
// of outer class inside inner class, to make sure we're getting the right `class_name_binding`.
let class_name_binding = class_name_binding.as_ref().unwrap();
// of outer class inside inner class, to make sure we're getting the right `class_binding`.
let class_binding = class_binding.as_ref().unwrap();

// If `object` is reference to class name, there's no need for the class brand assertion
if let Some(reference_id) =
Self::shortcut_static_class(is_declaration, class_name_binding, &object, ctx)
Self::shortcut_static_class(is_declaration, class_binding, &object, ctx)
{
// `_prop._`
ctx.symbols_mut()
.delete_resolved_reference(class_name_binding.symbol_id, reference_id);
ctx.symbols_mut().delete_resolved_reference(class_binding.symbol_id, reference_id);
Self::create_underscore_member_expression(prop_ident, span, ctx)
} else {
// `_assertClassBrand(Class, object, _prop)._`
self.create_assert_class_brand_underscore(
class_name_binding.create_read_expression(ctx),
class_binding.create_read_expression(ctx),
object,
prop_ident,
span,
Expand All @@ -90,18 +89,18 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
/// If can use shorter version, returns `ReferenceId` of the `IdentifierReference`.
//
// TODO(improve-on-babel): No reason not to use the short version for class expressions too.
// TODO: Take `SymbolId` instead of `class_name_binding: &BoundIdentifier<'a>`?
// TODO: Take `SymbolId` instead of `class_binding: &BoundIdentifier<'a>`?
fn shortcut_static_class(
is_declaration: bool,
class_name_binding: &BoundIdentifier<'a>,
class_binding: &BoundIdentifier<'a>,
object: &Expression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<ReferenceId> {
if is_declaration {
if let Expression::Identifier(ident) = object {
let reference_id = ident.reference_id();
if let Some(symbol_id) = ctx.symbols().get_reference(reference_id).symbol_id() {
if symbol_id == class_name_binding.symbol_id {
if symbol_id == class_binding.symbol_id {
return Some(reference_id);
}
}
Expand Down Expand Up @@ -184,7 +183,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
ctx: &mut TraverseCtx<'a>,
) -> Option<(Expression<'a>, Expression<'a>)> {
// TODO: Should never be `None` - only because implementation is incomplete.
let (prop, class_name_binding, is_declaration) =
let (prop, class_binding, is_declaration) =
self.lookup_private_property(&field_expr.field)?;
let prop_ident = prop.binding.create_read_expression(ctx);

Expand All @@ -194,15 +193,13 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let replacement = if prop.is_static {
// `object.#prop(arg)` -> `_assertClassBrand(Class, object, _prop)._.call(object, arg)`
// or shortcut `_prop._.call(object, arg)`
let class_name_binding = class_name_binding.as_ref().unwrap();
let class_ident = class_name_binding.create_read_expression(ctx);
let class_binding = class_binding.as_ref().unwrap();
let class_ident = class_binding.create_read_expression(ctx);

// If `object` is reference to class name, there's no need for the class brand assertion
// TODO: Combine this check with `duplicate_object`. Both check if `object` is an identifier,
// and look up the `SymbolId`
if Self::shortcut_static_class(is_declaration, class_name_binding, &object, ctx)
.is_some()
{
if Self::shortcut_static_class(is_declaration, class_binding, &object, ctx).is_some() {
// `_prop._`
let callee =
Self::create_underscore_member_expression(prop_ident, field_expr.span, ctx);
Expand All @@ -213,7 +210,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// `_assertClassBrand(Class, object, _prop)._`
// TODO: Ensure there are tests for nested classes with references to private static props
// of outer class inside inner class, to make sure we're getting the right `class_name_binding`.
// of outer class inside inner class, to make sure we're getting the right `class_binding`.
let assert_obj = self.create_assert_class_brand_underscore(
class_ident,
object1,
Expand Down Expand Up @@ -266,7 +263,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

let prop_details = self.lookup_private_property(&field_expr.field);
// TODO: Should never be `None` - only because implementation is incomplete.
let Some((prop, class_name_binding, is_declaration)) = prop_details else { return };
let Some((prop, class_binding, is_declaration)) = prop_details else { return };

// Note: `transform_static_assignment_expression` and `transform_instance_assignment_expression`
// are marked `#[inline]`, so hopefully compiler will see these clones of `BoundIdentifier`s
Expand All @@ -276,11 +273,11 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let prop_binding = prop.binding.clone();

if prop.is_static {
let class_name_binding = class_name_binding.as_ref().unwrap().clone();
let class_binding = class_binding.as_ref().unwrap().clone();
self.transform_static_assignment_expression(
expr,
prop_binding,
class_name_binding,
class_binding,
is_declaration,
ctx,
);
Expand Down Expand Up @@ -310,7 +307,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
&mut self,
expr: &mut Expression<'a>,
prop_binding: BoundIdentifier<'a>,
class_name_binding: BoundIdentifier<'a>,
class_binding: BoundIdentifier<'a>,
is_declaration: bool,
ctx: &mut TraverseCtx<'a>,
) {
Expand All @@ -323,12 +320,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// Check if object (`object` in `object.#prop`) is a reference to class name
// TODO: Combine this check with `duplicate_object`. Both check if `object` is an identifier,
// and look up the `SymbolId`.
let object_reference_id = Self::shortcut_static_class(
is_declaration,
&class_name_binding,
&field_expr.object,
ctx,
);
let object_reference_id =
Self::shortcut_static_class(is_declaration, &class_binding, &field_expr.object, ctx);

// If `object` is reference to class name, there's no need for the class brand assertion.
// `Class.#prop = value` -> `_prop._ = value`
Expand All @@ -346,7 +339,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
);

// Delete reference for `object` as `object.#prop` has been removed
ctx.symbols_mut().delete_resolved_reference(class_name_binding.symbol_id, reference_id);
ctx.symbols_mut().delete_resolved_reference(class_binding.symbol_id, reference_id);

if operator == AssignmentOperator::Assign {
// `Class.#prop = value` -> `_prop._ = value`
Expand Down Expand Up @@ -389,23 +382,23 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
};
let object = field_expr.object;

let class_ident = class_name_binding.create_read_expression(ctx);
let class_ident = class_binding.create_read_expression(ctx);
let value = ctx.ast.move_expression(&mut assign_expr.right);

if operator == AssignmentOperator::Assign {
// Replace right side of assignment with `_assertClassBrand(Class, object, _prop)`
// TODO: Ensure there are tests for nested classes with references to private static props
// of outer class inside inner class, to make sure we're getting the right `class_name_binding`.
// of outer class inside inner class, to make sure we're getting the right `class_binding`.
assign_expr.right = self.create_assert_class_brand(class_ident, object, value, ctx);
} else {
let class_ident = class_name_binding.create_read_expression(ctx);
let class_ident = class_binding.create_read_expression(ctx);
let value = ctx.ast.move_expression(&mut assign_expr.right);

// Make 2 copies of `object`
let (object1, object2) = self.duplicate_object(object, ctx);

let prop_ident = prop_binding.create_read_expression(ctx);
let class_ident2 = class_name_binding.create_read_expression(ctx);
let class_ident2 = class_binding.create_read_expression(ctx);

if let Some(operator) = operator.to_binary_operator() {
// `object.#prop += value`
Expand Down Expand Up @@ -615,7 +608,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

let prop_details = self.lookup_private_property(&field_expr.field);
// TODO: Should never be `None` - only because implementation is incomplete.
let Some((prop, class_name_binding, is_declaration)) = prop_details else { return };
let Some((prop, class_binding, is_declaration)) = prop_details else { return };
let prop_ident = prop.binding.create_read_expression(ctx);
let prop_ident2 = prop.binding.create_read_expression(ctx);

Expand Down Expand Up @@ -643,19 +636,18 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// (_object$prop = _assertClassBrand(Class, object, _prop)._, ++_object$prop)
// )
// ```
let class_name_binding = class_name_binding.as_ref().unwrap().clone();
let class_binding = class_binding.as_ref().unwrap().clone();

// Check if object (`object` in `object.#prop`) is a reference to class name
// TODO: Combine this check with `duplicate_object`. Both check if `object` is an identifier,
// and look up the `SymbolId`.
let object_reference_id =
Self::shortcut_static_class(is_declaration, &class_name_binding, &object, ctx);
Self::shortcut_static_class(is_declaration, &class_binding, &object, ctx);

// `_assertClassBrand(Class, object, _prop)._` or `_prop._`
let (get_expr, object) = if let Some(reference_id) = object_reference_id {
// Delete reference for `object` as `object.#prop` is being removed
ctx.symbols_mut()
.delete_resolved_reference(class_name_binding.symbol_id, reference_id);
ctx.symbols_mut().delete_resolved_reference(class_binding.symbol_id, reference_id);

// `_prop._`
let get_expr = Self::create_underscore_member_expression(prop_ident, SPAN, ctx);
Expand All @@ -666,7 +658,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// `_assertClassBrand(Class, object, _prop)._`
let get_call = self.create_assert_class_brand_underscore(
class_name_binding.create_read_expression(ctx),
class_binding.create_read_expression(ctx),
object2,
prop_ident,
SPAN,
Expand Down Expand Up @@ -694,7 +686,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// `_assertClassBrand(Class, object, <value>)`
if object_reference_id.is_none() {
let class_ident = class_name_binding.create_read_expression(ctx);
let class_ident = class_binding.create_read_expression(ctx);
value = self.create_assert_class_brand(class_ident, object, value, ctx);
}

Expand Down Expand Up @@ -728,7 +720,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// `_assertClassBrand(Class, object, <value>)`
if object_reference_id.is_none() {
let class_ident = class_name_binding.create_read_expression(ctx);
let class_ident = class_binding.create_read_expression(ctx);
value = self.create_assert_class_brand(class_ident, object, value, ctx);
}

Expand Down Expand Up @@ -1043,11 +1035,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// TODO: Check there are tests for bindings in enclosing classes.
for private_props in self.private_props_stack.as_slice().iter().rev() {
if let Some(prop) = private_props.props.get(&ident.name) {
return Some((
prop,
&private_props.class_name_binding,
private_props.is_declaration,
));
return Some((prop, &private_props.class_binding, private_props.is_declaration));
}
}
// TODO: This should be unreachable. Only returning `None` because implementation is incomplete.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
) {
// TODO: Insert temp var if class binding is mutated.

let ClassName::Binding(class_name_binding) = &self.class_name else {
let ClassName::Binding(class_binding) = &self.class_name else {
// Binding is initialized in 1st pass in `transform_class` when a static prop is found
unreachable!();
};
// Unfortunately have to clone, because also pass `&mut self` to `StaticInitializerVisitor::new`
let class_name_binding = class_name_binding.clone();
let class_binding = class_binding.clone();

let mut replacer = StaticInitializerVisitor::new(class_name_binding, self, ctx);
let mut replacer = StaticInitializerVisitor::new(class_binding, self, ctx);
replacer.visit_expression(value);
}
}
Expand All @@ -52,8 +52,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
//
// TODO: Also re-parent child scopes.
struct StaticInitializerVisitor<'a, 'ctx, 'v> {
/// Binding for class name.
class_name_binding: BoundIdentifier<'a>,
/// Binding for class (temp var).
class_binding: BoundIdentifier<'a>,
/// `true` if class has private properties.
class_has_private_props: bool,
/// Incremented when entering a different `this` context, decremented when exiting it.
Expand All @@ -67,12 +67,12 @@ struct StaticInitializerVisitor<'a, 'ctx, 'v> {

impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> {
fn new(
class_name_binding: BoundIdentifier<'a>,
class_binding: BoundIdentifier<'a>,
class_properties: &'v mut ClassProperties<'a, 'ctx>,
ctx: &'v mut TraverseCtx<'a>,
) -> Self {
Self {
class_name_binding,
class_binding,
class_has_private_props: class_properties.private_props_stack.last().is_some(),
this_depth: 0,
class_properties,
Expand Down Expand Up @@ -219,7 +219,7 @@ impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> {
/// Replace `this` with reference to class name binding.
fn replace_this_with_class_name(&mut self, expr: &mut Expression<'a>, span: Span) {
if self.this_depth == 0 {
*expr = self.class_name_binding.create_spanned_read_expression(span, self.ctx);
*expr = self.class_binding.create_spanned_read_expression(span, self.ctx);
}
}

Expand Down

0 comments on commit f7d1be8

Please sign in to comment.