feat(transformer/class-properties): transform private methods

crates/oxc_transformer/src/common/helper_loader.rs

@@ -152,6 +152,7 @@ pub enum Helper {
     ToPropertyKey,
     DefineProperty,
     ClassPrivateFieldInitSpec,
+    ClassPrivateMethodInitSpec,
     ClassPrivateFieldGet2,
     ClassPrivateFieldSet2,
     AssertClassBrand,
@@ -177,6 +178,7 @@ impl Helper {
             Self::ToPropertyKey => "toPropertyKey",
             Self::DefineProperty => "defineProperty",
             Self::ClassPrivateFieldInitSpec => "classPrivateFieldInitSpec",
+            Self::ClassPrivateMethodInitSpec => "classPrivateMethodInitSpec",
             Self::ClassPrivateFieldGet2 => "classPrivateFieldGet2",
             Self::ClassPrivateFieldSet2 => "classPrivateFieldSet2",
             Self::AssertClassBrand => "assertClassBrand",

crates/oxc_transformer/src/es2022/class_properties/class.rs

@@ -77,6 +77,7 @@ 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_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();
@@ -117,10 +118,21 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
                             constructor = Some(method);
                         }
                     } else if let PropertyKey::PrivateIdentifier(ident) = &method.key {
-                        let dummy_binding = BoundIdentifier::new(Atom::empty(), SymbolId::new(0));
+                        has_private_method = true;
+                        let name = match method.kind {
+                            MethodDefinitionKind::Method => ident.name.as_str(),
+                            MethodDefinitionKind::Get => &format!("get_{}", ident.name),
+                            MethodDefinitionKind::Set => &format!("set_{}", ident.name),
+                            MethodDefinitionKind::Constructor => unreachable!(),
+                        };
+                        let binding = ctx.generate_uid(
+                            name,
+                            ctx.current_block_scope_id(),
+                            SymbolFlags::FunctionScopedVariable,
+                        );
                         private_props.insert(
                             ident.name.clone(),
-                            PrivateProp::new(dummy_binding, method.r#static, true, false),
+                            PrivateProp::new(binding, method.r#static, true, false),
                         );
                     }
                 }
@@ -142,7 +154,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         }
 
         // Exit if nothing to transform
-        if instance_prop_count == 0 && !has_static_prop && !has_static_block {
+        if instance_prop_count == 0 && !has_static_prop && !has_static_block && !has_private_method
+        {
             self.classes_stack.push(ClassDetails {
                 is_declaration,
                 is_transform_required: false,
@@ -180,10 +193,18 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             None
         };
 
+        let class_brand_binding = has_private_method.then(|| {
+            // `_Class_brand`
+            let name = class_name_binding.as_ref().map_or_else(|| "Class", |binding| &binding.name);
+            let name = &format!("_{name}_brand");
+            let scope_id = ctx.current_block_scope_id();
+            ctx.generate_uid(name, scope_id, SymbolFlags::FunctionScopedVariable)
+        });
         let static_private_fields_use_temp = !is_declaration;
         let class_bindings = ClassBindings::new(
             class_name_binding,
             class_temp_binding,
+            class_brand_binding,
             outer_hoist_scope_id,
             static_private_fields_use_temp,
             need_temp_var,
@@ -198,7 +219,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         });
 
         // Exit if no instance properties (public or private)
-        if instance_prop_count == 0 {
+        if instance_prop_count == 0 && !has_private_method {
             return;
         }
 
@@ -249,7 +270,14 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         // `class C { [foo()] = 123; }` -> `class C { [_foo = foo()]; }`
         // 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);
+        let mut instance_inits =
+            Vec::with_capacity(instance_prop_count + usize::from(has_private_method));
+
+        // `_classPrivateMethodInitSpec(this, _C_brand);`
+        if has_private_method {
+            instance_inits.push(self.create_class_private_method_init_spec(ctx));
+        }
+
         let mut constructor = None;
         for element in body.body.iter_mut() {
             #[expect(clippy::match_same_arms)]
@@ -419,16 +447,23 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             } else {
                 // TODO: Only call `insert_many_before` if some private *instance* props
                 let mut weakmap_symbol_id = None;
+                let has_method = false;
                 self.ctx.statement_injector.insert_many_before(
                     &stmt_address,
                     private_props.values().filter_map(|prop| {
-                        if prop.is_static || prop.is_method || prop.is_accessor {
+                        if prop.is_static || has_method || prop.is_accessor {
                             return None;
                         }
-
-                        // `var _prop = new WeakMap();`
-                        let value = create_new_weakmap(&mut weakmap_symbol_id, ctx);
-                        Some(create_variable_declaration(&prop.binding, value, ctx))
+                        if prop.is_method {
+                            // `var _C_brand = new WeakSet();`
+                            let binding = class_details.bindings.brand();
+                            let value = create_new_weakset(ctx);
+                            Some(create_variable_declaration(binding, value, ctx))
+                        } else {
+                            // `var _prop = new WeakMap();`
+                            let value = create_new_weakmap(&mut weakmap_symbol_id, ctx);
+                            Some(create_variable_declaration(&prop.binding, value, ctx))
+                        }
                     }),
                 );
             }
@@ -623,8 +658,9 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
     ///
     /// * Transform static properties and insert after class.
     /// * Transform static blocks and insert after class.
+    /// * Transform private methods and insert after class.
     /// * Extract computed key assignments and insert them before class.
-    /// * Remove all properties and static blocks from class body.
+    /// * Remove all properties, private methods and static blocks from class body.
     fn transform_class_elements(&mut self, class: &mut Class<'a>, ctx: &mut TraverseCtx<'a>) {
         class.body.body.retain_mut(|element| {
             match element {
@@ -644,6 +680,9 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
                 }
                 ClassElement::MethodDefinition(method) => {
                     self.substitute_temp_var_for_method_computed_key(method, ctx);
+                    if self.convert_private_method(method, ctx) {
+                        return false;
+                    }
                 }
                 ClassElement::AccessorProperty(_) | ClassElement::TSIndexSignature(_) => {
                     // TODO: Need to handle these?
@@ -743,3 +782,10 @@ fn create_new_weakmap<'a>(
     let ident = ctx.create_ident_expr(SPAN, Atom::from("WeakMap"), symbol_id, ReferenceFlags::Read);
     ctx.ast.expression_new(SPAN, ident, ctx.ast.vec(), NONE)
 }
+
+/// Create `new WeakSet()` expression.
+fn create_new_weakset<'a>(ctx: &mut TraverseCtx<'a>) -> Expression<'a> {
+    let symbol_id = ctx.scopes().find_binding(ctx.current_scope_id(), "WeakSet");
+    let ident = ctx.create_ident_expr(SPAN, Atom::from("WeakSet"), symbol_id, ReferenceFlags::Read);
+    ctx.ast.expression_new(SPAN, ident, ctx.ast.vec(), NONE)
+}

crates/oxc_transformer/src/es2022/class_properties/class_bindings.rs

@@ -44,6 +44,8 @@ pub(super) struct ClassBindings<'a> {
     /// Temp var for class.
     /// e.g. `_Class` in `_Class = class {}, _Class.x = 1, _Class`
     pub temp: Option<BoundIdentifier<'a>>,
+    /// Temp var for WeakSet.
+    pub brand: Option<BoundIdentifier<'a>>,
     /// `ScopeId` of hoist scope outside class (which temp `var` binding would be created in)
     pub outer_hoist_scope_id: ScopeId,
     /// `true` if should use temp binding for references to class in transpiled static private fields,
@@ -58,13 +60,15 @@ impl<'a> ClassBindings<'a> {
     pub fn new(
         name_binding: Option<BoundIdentifier<'a>>,
         temp_binding: Option<BoundIdentifier<'a>>,
+        brand_binding: Option<BoundIdentifier<'a>>,
         outer_scope_id: ScopeId,
         static_private_fields_use_temp: bool,
         temp_var_is_created: bool,
     ) -> Self {
         Self {
             name: name_binding,
             temp: temp_binding,
+            brand: brand_binding,
             outer_hoist_scope_id: outer_scope_id,
             static_private_fields_use_temp,
             temp_var_is_created,
@@ -75,14 +79,25 @@ impl<'a> ClassBindings<'a> {
     ///
     /// Used when class needs no transform, and for dummy entry at top of `ClassesStack`.
     pub fn dummy() -> Self {
-        Self::new(None, None, ScopeId::new(0), false, false)
+        Self::new(None, None, None, ScopeId::new(0), false, false)
     }
 
     /// Get `SymbolId` of name binding.
     pub fn name_symbol_id(&self) -> Option<SymbolId> {
         self.name.as_ref().map(|binding| binding.symbol_id)
     }
 
+    /// Get [`BoundIdentifier`] for class brand.
+    ///
+    /// Only use this method when you are sure that [Self::brand] is not `None`,
+    /// this will happen when there is a private method in the class.
+    ///
+    /// # Panics
+    /// Panics if [Self::brand] is `None`.
+    pub fn brand(&self) -> &BoundIdentifier<'a> {
+        self.brand.as_ref().unwrap()
+    }
+
     /// Get binding to use for referring to class in transpiled static private fields.
     ///
     /// e.g. `Class` in `_assertClassBrand(Class, object, _prop)._` (class name)

crates/oxc_transformer/src/es2022/class_properties/mod.rs

@@ -183,6 +183,7 @@
 //! * `static_block_and_prop_init.rs`: Transform of static property initializers and static blocks.
 //! * `computed_key.rs`:               Transform of property/method computed keys.
 //! * `private_field.rs`:              Transform of private fields (`this.#prop`).
+//! * `private_method.rs`:              Transform of private methods (`this.#method()`).
 //! * `super.rs`:                      Transform `super` expressions.
 //! * `class_details.rs`:              Structures containing details of classes and private properties.
 //! * `class_bindings.rs`:             Structure containing bindings for class name and temp var.
@@ -214,6 +215,7 @@ mod computed_key;
 mod constructor;
 mod instance_prop_init;
 mod private_field;
+mod private_method;
 mod prop_decl;
 mod static_block_and_prop_init;
 mod supers;

crates/oxc_transformer/src/es2022/class_properties/private_method.rs

@@ -0,0 +1,73 @@
+//! ES2022: Class Properties
+//! Transform of private method uses e.g. `this.#method()`.
+
+use oxc_ast::ast::{Argument, Expression, FunctionType, MethodDefinition, PropertyKey, Statement};
+use oxc_semantic::ScopeFlags;
+use oxc_span::SPAN;
+use oxc_traverse::TraverseCtx;
+
+use crate::Helper;
+
+use super::ClassProperties;
+
+impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
+    /// Convert method definition where the key is a private identifier and
+    /// insert it after the class.
+    ///
+    /// ```js
+    /// class C {
+    ///    #method() {}
+    ///    set #prop(value) {}
+    ///    get #prop() {return 0}
+    /// }
+    /// ```
+    ///
+    /// ->
+    ///
+    /// ```js
+    /// class C {}
+    /// function _method() {}
+    /// function _set_prop(value) {}
+    /// function _get_prop() {return 0}
+    /// ```
+    ///
+    /// Returns `true` if the method was converted.
+    pub(super) fn convert_private_method(
+        &mut self,
+        method: &mut MethodDefinition<'a>,
+        ctx: &mut TraverseCtx<'a>,
+    ) -> bool {
+        let PropertyKey::PrivateIdentifier(ident) = &method.key else {
+            return false;
+        };
+
+        let mut function = ctx.ast.move_function(&mut method.value);
+
+        let temp_binding = self.classes_stack.find_private_prop(ident).prop_binding;
+        function.id = Some(temp_binding.create_binding_identifier(ctx));
+        function.r#type = FunctionType::FunctionDeclaration;
+
+        let scope_id = function.scope_id();
+        let new_parent_id = ctx.current_block_scope_id();
+        ctx.scopes_mut().change_parent_id(scope_id, Some(new_parent_id));
+        *ctx.scopes_mut().get_flags_mut(scope_id) -= ScopeFlags::StrictMode;
+
+        let function = ctx.ast.alloc(function);
+        self.insert_after_stmts.push(Statement::FunctionDeclaration(function));
+
+        true
+    }
+
+    // `_classPrivateMethodInitSpec(this, brand)`
+    pub(super) fn create_class_private_method_init_spec(
+        &self,
+        ctx: &mut TraverseCtx<'a>,
+    ) -> Expression<'a> {
+        let brand = self.classes_stack.last().bindings.brand.as_ref().unwrap();
+        let arguments = ctx.ast.vec_from_array([
+            Argument::from(ctx.ast.expression_this(SPAN)),
+            Argument::from(brand.create_read_expression(ctx)),
+        ]);
+        self.ctx.helper_call_expr(Helper::ClassPrivateMethodInitSpec, SPAN, arguments, ctx)
+    }
+}

tasks/transform_conformance/snapshots/babel.snap.md

@@ -1,6 +1,6 @@
 commit: 54a8389f
 
-Passed: 622/1095
+Passed: 623/1095
 
 # All Passed:
 * babel-plugin-transform-logical-assignment-operators
@@ -462,7 +462,7 @@ x Output mismatch
 x Output mismatch
 
 
-# babel-plugin-transform-private-methods (7/148)
+# babel-plugin-transform-private-methods (8/148)
 * accessors/arguments/input.js
 x Output mismatch
 
@@ -592,9 +592,6 @@ x Output mismatch
 * private-method/generator/input.js
 x Output mismatch
 
-* private-method/preserve-comments/input.js
-x Output mismatch
-
 * private-method/read-only/input.js
 x Output mismatch