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

feat(transformer/class-properties): transform static private method invoking #8117

Draft
wants to merge 1 commit into
base: 12-25-feat_transformer_class-properties_insert_statements_after_statement_of_class_expression
Choose a base branch
from
+230 −270
Draft

feat(transformer/class-properties): transform static private method invoking #8117

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
39 changes: 27 additions & 12 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 @@ -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
View file
Open in desktop
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
View file
Open in desktop
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
Loading