From 2964a615461cbd165e4db6f9667f80407e6bcb95 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Wed, 11 Dec 2024 11:32:21 +0000 Subject: [PATCH] fix(transformer/class-properties): unwrap failed when private field expression doesn't contain optional expression in `ChainExpression` (#7798) The root cause is due to transform wrongly a PrivateFieldExpression that doesn't contain any optional expression, so call `to_member_expression_mut` causes unwrap to fail. I have fixed the incorrect transform and changed `to_member_expression_mut` to `as_member_expression_mut`. --- .../src/es2022/class_properties/private.rs | 31 ++++++++++--------- .../snapshots/oxc.snap.md | 4 +-- .../input.js | 6 ++++ .../output.js | 9 ++++++ 4 files changed, 34 insertions(+), 16 deletions(-) create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/private-optional-call-with-non-optional-callee/input.js create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/private-optional-call-with-non-optional-callee/output.js diff --git a/crates/oxc_transformer/src/es2022/class_properties/private.rs b/crates/oxc_transformer/src/es2022/class_properties/private.rs index fbf4e388d51a9..0dbfae74f62d0 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/private.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/private.rs @@ -959,8 +959,13 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { if matches!(element, ChainElement::PrivateFieldExpression(_)) { // The PrivateFieldExpression must be transformed, so we can convert it to a normal expression here. let mut chain_expr = Self::convert_chain_expression_to_expression(expr, ctx); - let result = - self.transform_private_field_expression_of_chain_expression(&mut chain_expr, ctx); + let result = self + .transform_private_field_expression_of_chain_expression(&mut chain_expr, ctx) + .unwrap_or_else(|| { + unreachable!( + "The ChainExpression must contains at least one optional expression, so it never be `None` here." + ) + }); Some((result, chain_expr)) } else if let Some(result) = self.transform_chain_expression_element(element, ctx) { let chain_expr = Self::convert_chain_expression_to_expression(expr, ctx); @@ -1003,7 +1008,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { ) -> Option> { match expr { Expression::PrivateFieldExpression(_) => { - Some(self.transform_private_field_expression_of_chain_expression(expr, ctx)) + self.transform_private_field_expression_of_chain_expression(expr, ctx) } match_member_expression!(Expression) => self .transform_member_expression_of_chain_expression( @@ -1086,17 +1091,20 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { Some(result) } + /// Transform private field expression of chain expression. + /// + /// Returns `None` if the `expr` doesn't contain any optional expression. fn transform_private_field_expression_of_chain_expression( &mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>, - ) -> Expression<'a> { + ) -> Option> { let Expression::PrivateFieldExpression(field_expr) = expr else { unreachable!() }; let is_optional = field_expr.optional; let object = &mut field_expr.object; - let left = if is_optional { + let result = if is_optional { Some(self.transform_expression_to_wrap_nullish_check(object, ctx)) } else { self.transform_first_optional_expression(object, ctx) @@ -1110,13 +1118,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { self.transform_private_field_expression(expr, ctx); } - left.unwrap_or_else(|| { - // `o.Foo.#x?.self` -> `(_babelHelpers$assertC = expr === null || _babelHelpers$assertC === void 0 ? - // void 0 : _babelHelpers$assertC?.self;` - // `expr` is `o.Foo.#x` that has transformed to `babelHelpers.assertClassBrand(Foo, o.Foo, _x)._)` by - // `self.transform_private_field_expression`. - self.transform_expression_to_wrap_nullish_check(expr, ctx) - }) + result } fn transform_member_expression_of_chain_expression( @@ -1145,6 +1147,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { ctx: &mut TraverseCtx<'a>, ) -> Option> { let is_optional = call_expr.optional; + let callee = call_expr.callee.get_inner_expression_mut(); if matches!(callee, Expression::PrivateFieldExpression(_)) { let result = self.transform_first_optional_expression(callee, ctx); @@ -1157,9 +1160,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { return result; } - let result = self.transform_chain_element_recursively(callee, ctx)?; if !is_optional { - return Some(result); + return self.transform_chain_element_recursively(callee, ctx); } // `o?.Foo.#self.getSelf?.()?.self.#m();` @@ -1168,6 +1170,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { // then use it as a first argument of `getSelf` call. // // TODO(improve-on-babel): Consider remove this logic, because it seems no runtime behavior change. + let result = self.transform_chain_element_recursively(callee, ctx)?; let object = callee.to_member_expression_mut().object_mut(); let (assignment, context) = self.duplicate_object(ctx.ast.move_expression(object), ctx); *object = assignment; diff --git a/tasks/transform_conformance/snapshots/oxc.snap.md b/tasks/transform_conformance/snapshots/oxc.snap.md index 20496075a061d..387c20f4e124e 100644 --- a/tasks/transform_conformance/snapshots/oxc.snap.md +++ b/tasks/transform_conformance/snapshots/oxc.snap.md @@ -1,6 +1,6 @@ commit: 54a8389f -Passed: 106/120 +Passed: 107/121 # All Passed: * babel-plugin-transform-class-static-block @@ -16,7 +16,7 @@ Passed: 106/120 * regexp -# babel-plugin-transform-class-properties (7/10) +# babel-plugin-transform-class-properties (8/11) * private-loose-tagged-template/input.js Scope children mismatch: after transform: ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4)] diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/private-optional-call-with-non-optional-callee/input.js b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/private-optional-call-with-non-optional-callee/input.js new file mode 100644 index 0000000000000..062d1fba50b2a --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/private-optional-call-with-non-optional-callee/input.js @@ -0,0 +1,6 @@ +class A { + #a = {}; + method() { + this.#a.get(message.id)?.(message); + } +} diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/private-optional-call-with-non-optional-callee/output.js b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/private-optional-call-with-non-optional-callee/output.js new file mode 100644 index 0000000000000..bbbd336e6d1b9 --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/private-optional-call-with-non-optional-callee/output.js @@ -0,0 +1,9 @@ +var _a = /*#__PURE__*/ new WeakMap(); +class A { + constructor() { + babelHelpers.classPrivateFieldInitSpec(this, _a, {}); + } + method() { + babelHelpers.classPrivateFieldGet2(_a, this).get(message.id)?.(message); + } +}