Skip to content

Commit

Permalink
feat(transformer/class-properties): transform static private method i…
Browse files Browse the repository at this point in the history
…nvoking
  • Loading branch information
Dunqing committed Dec 26, 2024
1 parent 52b6e42 commit 1338938
Show file tree
Hide file tree
Showing 4 changed files with 230 additions and 270 deletions.
39 changes: 27 additions & 12 deletions crates/oxc_transformer/src/es2022/class_properties/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// Check if class has any properties or statick blocks, and locate constructor (if class has one)
let mut instance_prop_count = 0;
let mut has_instance_private_method = false;
let mut has_static_prop = false;
let mut has_private_method = false;
let mut has_static_block = false;
// TODO: Store `FxIndexMap`s in a pool and re-use them
let mut private_props = FxIndexMap::default();
Expand Down Expand Up @@ -118,7 +118,12 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
constructor = Some(method);
}
} else if let PropertyKey::PrivateIdentifier(ident) = &method.key {
has_private_method = true;
if method.r#static {
has_static_prop = true;
} else {
has_instance_private_method = true;
}

let name = match method.kind {
MethodDefinitionKind::Method => ident.name.as_str(),
MethodDefinitionKind::Get => &format!("get_{}", ident.name),
Expand Down Expand Up @@ -154,7 +159,10 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
}

// Exit if nothing to transform
if instance_prop_count == 0 && !has_static_prop && !has_static_block && !has_private_method
if instance_prop_count == 0
&& !has_static_prop
&& !has_static_block
&& !has_instance_private_method
{
self.classes_stack.push(ClassDetails {
is_declaration,
Expand Down Expand Up @@ -193,7 +201,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
None
};

let class_brand_binding = has_private_method.then(|| {
let class_brand_binding = has_instance_private_method.then(|| {
// `_Class_brand`
let name = class_name_binding.as_ref().map_or_else(|| "Class", |binding| &binding.name);
let name = &format!("_{name}_brand");
Expand All @@ -219,7 +227,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
});

// Exit if no instance properties (public or private)
if instance_prop_count == 0 && !has_private_method {
if instance_prop_count == 0 && !has_instance_private_method {
return;
}

Expand Down Expand Up @@ -271,10 +279,10 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// Those assignments will be moved to before class in exit phase of the transform.
// -> `_foo = foo(); class C {}`
let mut instance_inits =
Vec::with_capacity(instance_prop_count + usize::from(has_private_method));
Vec::with_capacity(instance_prop_count + usize::from(has_instance_private_method));

// `_classPrivateMethodInitSpec(this, _C_brand);`
if has_private_method {
if has_instance_private_method {
instance_inits.push(self.create_class_private_method_init_spec(ctx));
}

Expand Down Expand Up @@ -590,11 +598,10 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let mut weakmap_symbol_id = None;
let mut has_method = false;
exprs.extend(private_props.values().filter_map(|prop| {
if (prop.is_method && has_method) || prop.is_accessor {
return None;
}

if prop.is_method {
if prop.is_method || prop.is_accessor {
if prop.is_static || has_method {
return None;
}
has_method = true;
// `_C_brand = new WeakSet()`
let binding = class_details.bindings.brand();
Expand Down Expand Up @@ -642,6 +649,14 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// `_Class = class {}`
let class_expr = ctx.ast.move_expression(expr);
let assignment = create_assignment(binding, class_expr, ctx);

if exprs.is_empty() && self.insert_after_exprs.is_empty() {
// No need to wrap in sequence if no static property
// and static blocks
*expr = assignment;
return;
}

exprs.push(assignment);
// Add static property assignments + static blocks
exprs.extend(self.insert_after_exprs.drain(..));
Expand Down
111 changes: 58 additions & 53 deletions crates/oxc_transformer/src/es2022/class_properties/private_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
} = self.classes_stack.find_private_prop(&field_expr.field);

let span = field_expr.span;

if is_method || is_accessor {
let prop_ident = prop_binding.create_read_expression(ctx);
return if is_assignment {
// TODO: Handle assignment to private method or accessor
None
} else {
Some(self.create_assert_class_brand_for_private_method(prop_ident, span, ctx))
};
};

let object = ctx.ast.move_expression(&mut field_expr.object);

if self.private_fields_as_properties {
Expand All @@ -88,6 +77,11 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let prop_ident = prop_binding.create_read_expression(ctx);

let replacement = if is_static {
if is_assignment && (is_method || is_accessor) {
// TODO: Handle assignment to static private method or static accessor
return None;
}

// 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_bindings`.

Expand All @@ -100,23 +94,37 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
) {
// `_prop._`
ctx.symbols_mut().delete_resolved_reference(class_symbol_id, object_reference_id);
Self::create_underscore_member_expression(prop_ident, span, ctx)
if is_method || is_accessor {
prop_ident
} else {
Self::create_underscore_member_expression(prop_ident, span, ctx)
}
} else {
// `_assertClassBrand(Class, object, _prop)._`
let class_binding = class_bindings.get_or_init_static_binding(ctx);
let class_ident = class_binding.create_read_expression(ctx);

self.create_assert_class_brand_underscore(
class_ident,
object,
prop_ident,
span,
ctx,
)
if is_method || is_accessor {
self.create_assert_class_brand(class_ident, object, prop_ident, span, ctx)
} else {
self.create_assert_class_brand_underscore(
class_ident,
object,
prop_ident,
span,
ctx,
)
}
}
} else if is_assignment {
if is_method || is_accessor {
// TODO: Handle assignment to private method or accessor
return None;
}
// `_toSetter(_classPrivateFieldSet2, [_prop, object])._`
self.create_to_setter(prop_ident, object, span, ctx)
} else if is_method || is_accessor {
let brand_ident = class_bindings.brand().create_read_expression(ctx);
self.create_assert_class_brand(brand_ident, object, prop_ident, span, ctx)
} else {
// `_classPrivateFieldGet2(_prop, object)`
self.create_private_field_get(prop_ident, object, span, ctx)
Expand Down Expand Up @@ -278,13 +286,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let span = field_expr.span;
let prop_ident = prop_binding.create_read_expression(ctx);

if is_method || is_accessor {
return (
self.create_assert_class_brand_for_private_method(prop_ident, span, ctx),
ctx.ast.expression_this(SPAN),
);
};

// `(object.#method)()`
// ^^^^^^^^^^^^^^^^ is a parenthesized expression
let object = ctx.ast.move_expression(field_expr.object.get_inner_expression_mut());
Expand All @@ -308,8 +309,13 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
)
.is_some()
{
// `_prop._`
let callee = Self::create_underscore_member_expression(prop_ident, span, ctx);
let callee = if is_method || is_accessor {
// `_prop`
prop_ident
} else {
// `_prop._`
Self::create_underscore_member_expression(prop_ident, span, ctx)
};
(callee, object)
} else {
let class_binding = class_bindings.get_or_init_static_binding(ctx);
Expand All @@ -318,24 +324,36 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// Make 2 copies of `object`
let (object1, object2) = self.duplicate_object(object, ctx);

// `_assertClassBrand(Class, object, _prop)._`
let assert_obj = self.create_assert_class_brand_underscore(
class_ident,
object1,
prop_ident,
span,
ctx,
);
let assert_obj = if is_method || is_accessor {
// `_assertClassBrand(Class, object, _prop)._`
self.create_assert_class_brand(class_ident, object1, prop_ident, span, ctx)
} else {
// `_assertClassBrand(Class, object, _prop)._`
self.create_assert_class_brand_underscore(
class_ident,
object1,
prop_ident,
span,
ctx,
)
};

(assert_obj, object2)
}
} else {
// `object.#prop(arg)` -> `_classPrivateFieldGet2(_prop, object).call(object, arg)`
// Make 2 copies of `object`
let (object1, object2) = self.duplicate_object(object, ctx);

// `_classPrivateFieldGet2(_prop, object)`
let get_call = self.create_private_field_get(prop_ident, object1, span, ctx);
(get_call, object2)
let callee = if is_method || is_accessor {
// `(_Class_brand, this)`
let brand_ident = self.current_class().bindings.brand().create_read_expression(ctx);
self.create_assert_class_brand(brand_ident, object1, prop_ident, span, ctx)
} else {
// `_classPrivateFieldGet2(_prop, object)`
self.create_private_field_get(prop_ident, object1, span, ctx)
};
(callee, object2)
};

replacement
Expand Down Expand Up @@ -1795,19 +1813,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
)
}

/// `_assertClassBrand(_Class_brand, object, _prop)`
#[inline]
fn create_assert_class_brand_for_private_method(
&self,
value_or_prop_ident: Expression<'a>,
span: Span,
ctx: &mut TraverseCtx<'a>,
) -> Expression<'a> {
let class_ident = self.current_class().bindings.brand().create_read_expression(ctx);
let object = ctx.ast.expression_this(SPAN);
self.create_assert_class_brand(class_ident, object, value_or_prop_ident, span, ctx)
}

/// `_assertClassBrand(Class, object, _prop)._`
fn create_assert_class_brand_underscore(
&self,
Expand Down
31 changes: 2 additions & 29 deletions tasks/transform_conformance/snapshots/babel.snap.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
commit: 54a8389f

Passed: 632/1095
Passed: 641/1095

# All Passed:
* babel-plugin-transform-logical-assignment-operators
Expand Down Expand Up @@ -462,7 +462,7 @@ x Output mismatch
x Output mismatch


# babel-plugin-transform-private-methods (17/148)
# babel-plugin-transform-private-methods (26/148)
* accessors/arguments/input.js
x Output mismatch

Expand Down Expand Up @@ -675,36 +675,9 @@ x Output mismatch
`----


* private-static-method/basic/input.js
x Output mismatch

* private-static-method/class-check/input.js
x Output mismatch

* private-static-method/class-expression/input.js
x Output mismatch

* private-static-method/exfiltrated/input.js
x Output mismatch

* private-static-method/generator/input.js
x Output mismatch

* private-static-method/preserve-comments/input.js
x Output mismatch

* private-static-method/read-only/input.js
x Output mismatch

* private-static-method/super/input.js
x Output mismatch

* private-static-method/tagged-template/input.js
x Output mismatch

* private-static-method/this/input.js
x Output mismatch

* private-static-method-loose/async/input.js

x TS(1108): A 'return' statement can only be used within a function body.
Expand Down
Loading

0 comments on commit 1338938

Please sign in to comment.