refactor(transformer/class-properties): PrivatePropsStack type (#7589)

Pure refactor. Move all logic and types related to storing details of what private properties classes have into a separate file `private_props.rs`. Introduce a type `PrivatePropsStack` to represent the stack of classes.

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

@@ -14,11 +14,12 @@ use crate::common::helper_loader::Helper;
 
 use super::super::ClassStaticBlock;
 use super::{
+    private_props::{PrivateProp, PrivateProps},
     utils::{
         create_assignment, create_underscore_ident_name, create_variable_declaration,
         exprs_into_stmts,
     },
-    ClassName, ClassProperties, FxIndexMap, PrivateProp, PrivateProps,
+    ClassName, ClassProperties, FxIndexMap,
 };
 
 impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

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

@@ -129,6 +129,7 @@
 //! * `class.rs`:          Transform of class body.
 //! * `constructor.rs`:    Insertion of property initializers into class constructor.
 //! * `private.rs`:        Transform of private property usages (`this.#prop`).
+//! * `private_props.rs`:  Structures storing details of private properties.
 //! * `static_prop.rs`:    Transform of static property initializers.
 //! * `utils.rs`:          Utility functions.
 //!
@@ -146,7 +147,7 @@ use serde::Deserialize;
 
 use oxc_allocator::{Address, GetAddress};
 use oxc_ast::ast::*;
-use oxc_data_structures::stack::{NonEmptyStack, SparseStack};
+use oxc_data_structures::stack::NonEmptyStack;
 use oxc_traverse::{BoundIdentifier, Traverse, TraverseCtx};
 
 use crate::TransformCtx;
@@ -154,8 +155,10 @@ use crate::TransformCtx;
 mod class;
 mod constructor;
 mod private;
+mod private_props;
 mod static_prop;
 mod utils;
+use private_props::PrivatePropsStack;
 
 type FxIndexMap<K, V> = IndexMap<K, V, FxBuildHasher>;
 
@@ -192,7 +195,7 @@ pub struct ClassProperties<'a, 'ctx> {
     // then stack will get out of sync.
     // TODO: Should push to the stack only when entering class body, because `#x` in class `extends`
     // clause resolves to `#x` in *outer* class, not the current class.
-    private_props_stack: SparseStack<PrivateProps<'a>>,
+    private_props_stack: PrivatePropsStack<'a>,
     /// Addresses of class expressions being processed, to prevent same class being visited twice.
     /// Have to use a stack because the revisit doesn't necessarily happen straight after the first visit.
     /// e.g. `c = class C { [class D {}] = 1; }` -> `c = (_D = class D {}, class C { ... })`
@@ -223,23 +226,6 @@ enum ClassName<'a> {
     Name(&'a str),
 }
 
-/// Details of private properties for a class.
-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, if class has name
-    class_binding: Option<BoundIdentifier<'a>>,
-    /// `true` for class declaration, `false` for class expression
-    is_declaration: bool,
-}
-
-/// Details of a private property.
-struct PrivateProp<'a> {
-    binding: BoundIdentifier<'a>,
-    is_static: bool,
-}
-
 impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
     pub fn new(
         options: ClassPropertiesOptions,
@@ -254,7 +240,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             set_public_class_fields,
             transform_static_blocks,
             ctx,
-            private_props_stack: SparseStack::new(),
+            private_props_stack: PrivatePropsStack::default(),
             class_expression_addresses_stack: NonEmptyStack::new(Address::DUMMY),
             // Temporary values - overwritten when entering class
             is_declaration: false,

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

@@ -15,24 +15,11 @@ use oxc_traverse::{ast_operations::get_var_name_from_node, BoundIdentifier, Trav
 use crate::common::helper_loader::Helper;
 
 use super::{
+    private_props::ResolvedPrivateProp,
     utils::{create_assignment, create_underscore_ident_name},
     ClassProperties,
 };
 
-/// Details of a private property resolved for a private field.
-///
-/// This is the return value of `lookup_private_property`.
-struct ResolvedPrivateProp<'a, 'b> {
-    /// Binding for temp var representing the property
-    prop_binding: &'b BoundIdentifier<'a>,
-    /// Binding for class (if it has one)
-    class_binding: Option<&'b BoundIdentifier<'a>>,
-    /// `true` if is a static property
-    is_static: bool,
-    /// `true` if class which defines this property is a class declaration
-    is_declaration: bool,
-}
-
 impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
     /// Transform private field expression.
     ///
@@ -56,7 +43,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         field_expr: ArenaBox<'a, PrivateFieldExpression<'a>>,
         ctx: &mut TraverseCtx<'a>,
     ) -> Expression<'a> {
-        let prop_details = self.lookup_private_property(&field_expr.field);
+        let prop_details = self.private_props_stack.find(&field_expr.field);
         // TODO: Should never be `None` - only because implementation is incomplete.
         let Some(prop_details) = prop_details else {
             return Expression::PrivateFieldExpression(field_expr);
@@ -199,7 +186,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
     ) -> Option<(Expression<'a>, Expression<'a>)> {
         // TODO: Should never be `None` - only because implementation is incomplete.
         let ResolvedPrivateProp { prop_binding, class_binding, is_static, is_declaration } =
-            self.lookup_private_property(&field_expr.field)?;
+            self.private_props_stack.find(&field_expr.field)?;
         let prop_ident = prop_binding.create_read_expression(ctx);
 
         let object = ctx.ast.move_expression(&mut field_expr.object);
@@ -278,7 +265,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             unreachable!()
         };
 
-        let prop_details = self.lookup_private_property(&field_expr.field);
+        let prop_details = self.private_props_stack.find(&field_expr.field);
         // TODO: Should never be `None` - only because implementation is incomplete.
         let Some(prop_details) = prop_details else { return };
         let ResolvedPrivateProp { prop_binding, class_binding, is_static, is_declaration } =
@@ -629,7 +616,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             _ => unreachable!(),
         };
 
-        let prop_details = self.lookup_private_property(&field_expr.field);
+        let prop_details = self.private_props_stack.find(&field_expr.field);
         // TODO: Should never be `None` - only because implementation is incomplete.
         let Some(prop_details) = prop_details else { return };
         let ResolvedPrivateProp { prop_binding, class_binding, is_static, is_declaration } =
@@ -1052,27 +1039,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         Self::create_underscore_member_expression(func_call, span, ctx)
     }
 
-    /// Lookup details of private property referred to by `ident`.
-    fn lookup_private_property<'b>(
-        &'b self,
-        ident: &PrivateIdentifier<'a>,
-    ) -> Option<ResolvedPrivateProp<'a, 'b>> {
-        // Check for binding in closest class first, then enclosing classes
-        // 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(ResolvedPrivateProp {
-                    prop_binding: &prop.binding,
-                    class_binding: private_props.class_binding.as_ref(),
-                    is_static: prop.is_static,
-                    is_declaration: private_props.is_declaration,
-                });
-            }
-        }
-        // TODO: This should be unreachable. Only returning `None` because implementation is incomplete.
-        None
-    }
-
     /// Create `<object>._` assignment target.
     fn create_underscore_member_expr_target(
         object: Expression<'a>,

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

@@ -0,0 +1,95 @@
+use oxc_ast::ast::PrivateIdentifier;
+use oxc_data_structures::stack::SparseStack;
+use oxc_span::Atom;
+use oxc_traverse::BoundIdentifier;
+
+use super::FxIndexMap;
+
+/// Stack of private props defined by classes.
+///
+/// Pushed to when entering a class (`None` if class has no private props, `Some` if it does).
+/// Entries contain a mapping from private prop name to binding for temp var for that property,
+/// and details of the class that defines the private prop.
+///
+/// This is used as lookup when transforming e.g. `this.#x`.
+#[derive(Default)]
+pub(super) struct PrivatePropsStack<'a> {
+    stack: SparseStack<PrivateProps<'a>>,
+}
+
+impl<'a> PrivatePropsStack<'a> {
+    // Forward methods to underlying `SparseStack`
+
+    #[inline]
+    pub fn push(&mut self, props: Option<PrivateProps<'a>>) {
+        self.stack.push(props);
+    }
+
+    #[inline]
+    pub fn pop(&mut self) -> Option<PrivateProps<'a>> {
+        self.stack.pop()
+    }
+
+    #[inline]
+    pub fn last(&self) -> Option<&PrivateProps<'a>> {
+        self.stack.last()
+    }
+
+    #[inline]
+    #[expect(dead_code)]
+    pub fn last_mut(&mut self) -> Option<&mut PrivateProps<'a>> {
+        self.stack.last_mut()
+    }
+
+    /// Lookup details of private property referred to by `ident`.
+    pub fn find<'b>(
+        &'b self,
+        ident: &PrivateIdentifier<'a>,
+    ) -> Option<ResolvedPrivateProp<'a, 'b>> {
+        // Check for binding in closest class first, then enclosing classes
+        // TODO: Check there are tests for bindings in enclosing classes.
+        for private_props in self.stack.as_slice().iter().rev() {
+            if let Some(prop) = private_props.props.get(&ident.name) {
+                return Some(ResolvedPrivateProp {
+                    prop_binding: &prop.binding,
+                    class_binding: private_props.class_binding.as_ref(),
+                    is_static: prop.is_static,
+                    is_declaration: private_props.is_declaration,
+                });
+            }
+        }
+        // TODO: This should be unreachable. Only returning `None` because implementation is incomplete.
+        None
+    }
+}
+
+/// Details of private properties for a class.
+pub(super) 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.
+    pub props: FxIndexMap<Atom<'a>, PrivateProp<'a>>,
+    /// Binding for class, if class has name
+    pub class_binding: Option<BoundIdentifier<'a>>,
+    /// `true` for class declaration, `false` for class expression
+    pub is_declaration: bool,
+}
+
+/// Details of a private property.
+pub(super) struct PrivateProp<'a> {
+    pub binding: BoundIdentifier<'a>,
+    pub is_static: bool,
+}
+
+/// Details of a private property resolved for a private field.
+///
+/// This is the return value of [`PrivatePropsStack::find`].
+pub(super) struct ResolvedPrivateProp<'a, 'b> {
+    /// Binding for temp var representing the property
+    pub prop_binding: &'b BoundIdentifier<'a>,
+    /// Binding for class (if it has one)
+    pub class_binding: Option<&'b BoundIdentifier<'a>>,
+    /// `true` if is a static property
+    pub is_static: bool,
+    /// `true` if class which defines this property is a class declaration
+    pub is_declaration: bool,
+}