From 888396845ca78263e251dcc60ff1fe26adf7237b Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Thu, 5 Dec 2024 17:46:22 +0000 Subject: [PATCH] refactor(transformer/class-properties): use `duplicate_object` in `transform_expression_to_wrap_nullish_check` (#7664) The `duplicate_object` is great, it has handled unbound identifiers and `this` expressions, so we can remove a lot of code. But it is still a little bit annoying because we need two `object` here, although we can use `clone_in` it needs a special logic for `Expression::Identifier`. --- .../src/es2022/class_properties/private.rs | 53 +++++-------------- 1 file changed, 14 insertions(+), 39 deletions(-) diff --git a/crates/oxc_transformer/src/es2022/class_properties/private.rs b/crates/oxc_transformer/src/es2022/class_properties/private.rs index 49204c97f4a4e..14031be717fa4 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/private.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/private.rs @@ -3,7 +3,7 @@ use std::mem; -use oxc_allocator::Box as ArenaBox; +use oxc_allocator::{Box as ArenaBox, CloneIn}; use oxc_ast::{ast::*, NONE}; use oxc_span::SPAN; use oxc_syntax::{ @@ -1102,34 +1102,26 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { /// /// Returns: /// * Bound Identifier: `A` -> `A === null || A === void 0` + /// * `this`: `this` -> `this === null || this === void 0` /// * Unbound Identifier or anything else: `A.B` -> `(_A$B = A.B) === null || _A$B === void 0` /// - /// NOTE: This method will mutate the passed-in `object` to a temp variable identifier. + /// NOTE: This method will mutate the passed-in `object` to a second copy of + /// [`Self::duplicate_object`]'s return. fn transform_expression_to_wrap_nullish_check( &mut self, object: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>, ) -> Expression<'a> { - // `A` -> `A === null || A === void 0` - if let Expression::Identifier(ident) = object { - if let Some(binding) = self.get_existing_binding_for_identifier(ident, ctx) { - let left1 = binding.create_read_expression(ctx); - let left2 = binding.create_read_expression(ctx); - return self.wrap_nullish_check(left1, left2, ctx); - } - } - - // `A.B` -> `(_A$B = A.B) === null || _A$B === void 0` - // TODO: should add an API `generate_uid_in_current_hoist_scope_based_on_node` to instead this - let temp_var_binding = self.ctx.var_declarations.create_uid_var_based_on_node(object, ctx); - - let object = mem::replace(object, temp_var_binding.create_read_expression(ctx)); - let assignment = create_assignment( - &temp_var_binding, - Self::ensure_optional_expression_wrapped_by_chain_expression(object, ctx), - ctx, - ); - let reference = temp_var_binding.create_read_expression(ctx); + let owned_object = ctx.ast.move_expression(object.get_inner_expression_mut()); + let owned_object = + Self::ensure_optional_expression_wrapped_by_chain_expression(owned_object, ctx); + let (assignment, reference) = self.duplicate_object(owned_object, ctx); + // We cannot use `clone_in` to clone identifier reference because it will lose reference id + *object = if let Expression::Identifier(ident) = &reference { + MaybeBoundIdentifier::from_identifier_reference(ident, ctx).create_read_expression(ctx) + } else { + reference.clone_in(ctx.ast.allocator) + }; self.wrap_nullish_check(assignment, reference, ctx) } @@ -1159,23 +1151,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { } } - /// Get an MaybeBoundIdentifier from an bound identifier reference. - /// - /// If no temp variable required, returns `MaybeBoundIdentifier` for existing variable/global. - /// If temp variable is required, returns `None`. - fn get_existing_binding_for_identifier( - &self, - ident: &IdentifierReference<'a>, - ctx: &TraverseCtx<'a>, - ) -> Option> { - let binding = MaybeBoundIdentifier::from_identifier_reference(ident, ctx); - if self.ctx.assumptions.pure_getters || binding.to_bound_identifier().is_some() { - Some(binding) - } else { - None - } - } - /// Ensure that the expression is wrapped by a chain expression. /// /// If the given expression contains optional expression, it will be wrapped by