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

refactor(transformer/class-properties): rename class_binding #7533

Merged
merged 1 commit into from
Dec 3, 2024
Merged

refactor(transformer/class-properties): rename class_binding #7533

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
8 changes: 4 additions & 4 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 @@ -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
View file
Open in desktop
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
View file
Open in desktop
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
16 changes: 8 additions & 8 deletions crates/oxc_transformer/src/es2022/class_properties/static_prop.rs
View file
Open in desktop
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
Loading