From ea76490ca5341e3f413ece4cc766c9c1c836cd3e Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Wed, 4 Nov 2026 04:53:14 +0000 Subject: [PATCH] fix(transformer/class-properties): preserve private method values in assignments (#28788) Compound assignments and updates to private methods call the method body while reading its value. For example, `this.#method ||= rhs()` calls `#method`, evaluates `rhs()` when the method returns `undefined`, and throws instead of returning the method function. Distinguish method reads from accessor getter calls when lowering assignments and updates. Method reads now return the function through the existing brand check. Regression coverage includes static and instance methods, all compound operators, prefix/postfix updates, failed brand checks, and receivers obtained from function calls. Validation: - 39 Rust unit/integration tests and 53 Node binding tests passed. - Clippy and Rust formatting checks passed. - Transform conformance adds one passing output fixture and one passing executable fixture; existing failures are unchanged. - Transformer coverage has no new failures against Test262, Babel, TypeScript, and local cases. - Cross-target probes passed at ES2021, ES2020, and ES2015. AI assistance was used to implement and test this change. --- .../es2022/class_properties/class_details.rs | 1 - .../es2022/class_properties/private_field.rs | 26 +++- .../snapshots/oxc.snap.md | 2 +- .../snapshots/oxc_exec.snap.md | 2 +- .../private-method-assignment-read/exec.js | 144 ++++++++++++++++++ .../private-method-assignment-read/input.js | 22 +++ .../private-method-assignment-read/output.js | 26 ++++ 7 files changed, 213 insertions(+), 10 deletions(-) create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/exec.js create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/input.js create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/output.js diff --git a/crates/oxc_transformer/src/es2022/class_properties/class_details.rs b/crates/oxc_transformer/src/es2022/class_properties/class_details.rs index af3094b169..af59c4ed7f 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/class_details.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/class_details.rs @@ -264,7 +264,6 @@ pub(super) struct ResolvedGetSetPrivateProp<'a, 'b> { pub is_method: bool, /// `true` if is a private accessor property or [`PrivateProp::method_kind`] is /// `Some(MethodDefinitionKind::Get)` or `Some(MethodDefinitionKind::Set)` - #[expect(unused)] pub is_accessor: bool, /// `true` if class which defines this property is a class declaration pub is_declaration: bool, diff --git a/crates/oxc_transformer/src/es2022/class_properties/private_field.rs b/crates/oxc_transformer/src/es2022/class_properties/private_field.rs index d52f04cb12..d59c65f36b 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/private_field.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/private_field.rs @@ -494,7 +494,7 @@ impl<'a> ClassProperties<'a> { is_static, is_declaration, is_method, - .. + is_accessor, } = self.classes_stack.find_get_set_private_prop(&field_expr.field); if self.private_fields_as_properties { @@ -565,6 +565,7 @@ impl<'a> ClassProperties<'a> { get_binding.as_ref(), set_binding.as_ref(), class_binding.as_ref(), + is_accessor, ctx, ); } else { @@ -770,6 +771,7 @@ impl<'a> ClassProperties<'a> { get_binding: Option<&BoundIdentifier<'a>>, set_binding: Option<&BoundIdentifier<'a>>, class_binding: Option<&BoundIdentifier<'a>>, + is_accessor: bool, ctx: &mut TraverseCtx<'a>, ) { let is_value_used = Self::is_value_used(ctx); @@ -808,6 +810,7 @@ impl<'a> ClassProperties<'a> { &field.name, class_binding, get_binding, + is_accessor, object2, SPAN, ctx, @@ -836,6 +839,7 @@ impl<'a> ClassProperties<'a> { &field.name, class_binding, get_binding, + is_accessor, object1, SPAN, ctx, @@ -976,7 +980,7 @@ impl<'a> ClassProperties<'a> { is_static, is_method, is_declaration, - .. + is_accessor, } = self.classes_stack.find_get_set_private_prop(&field_expr.field); let temp_var_name_base = get_var_name_from_node(field_expr); @@ -1139,6 +1143,7 @@ impl<'a> ClassProperties<'a> { &private_name, class_binding.as_ref(), get_binding.as_ref(), + is_accessor, object2, SPAN, ctx, @@ -2172,27 +2177,34 @@ impl<'a> ClassProperties<'a> { } /// * Getter: `_prop.call(_assertClassBrand(Class, object))` + /// * Method: `_assertClassBrand(Class, object, _prop)` /// * Prop: `_privateFieldGet(_prop, object)` - /// * Prop binding is `None`: `_readOnlyError("#method")` + /// * Prop binding is `None`: `_writeOnlyError("#method")` fn create_private_getter( &self, private_name: &str, class_binding: Option<&BoundIdentifier<'a>>, prop_binding: Option<&BoundIdentifier<'a>>, + is_accessor: bool, object: Expression<'a>, span: Span, ctx: &mut TraverseCtx<'a>, ) -> Expression<'a> { let Some(prop_binding) = prop_binding else { - // `_readOnlyError("#method")` + // `_writeOnlyError("#method")` return self.create_sequence_with_write_only_error(private_name, object, span, ctx); }; let prop_ident = prop_binding.create_read_expression(ctx); if let Some(class_binding) = class_binding { let class_ident = class_binding.create_read_expression(ctx); - let object = self.create_assert_class_brand_without_value(class_ident, object, ctx); - // `_prop.call(_assertClassBrand(Class, object))` - create_call_call(prop_ident, object, span, ctx) + if is_accessor { + let object = self.create_assert_class_brand_without_value(class_ident, object, ctx); + // `_prop.call(_assertClassBrand(Class, object))` + create_call_call(prop_ident, object, span, ctx) + } else { + // `_assertClassBrand(Class, object, _prop)` + self.create_assert_class_brand(class_ident, object, prop_ident, span, ctx) + } } else { // `_privateFieldGet(_prop, object)` self.create_private_field_get(prop_ident, object, ctx) diff --git a/tasks/transform_conformance/snapshots/oxc.snap.md b/tasks/transform_conformance/snapshots/oxc.snap.md index d0807cb60b..37bc12188e 100644 --- a/tasks/transform_conformance/snapshots/oxc.snap.md +++ b/tasks/transform_conformance/snapshots/oxc.snap.md @@ -1,6 +1,6 @@ commit: b835f0c6 -Passed: 308/437 +Passed: 309/438 # All Passed: * babel-plugin-transform-class-static-block diff --git a/tasks/transform_conformance/snapshots/oxc_exec.snap.md b/tasks/transform_conformance/snapshots/oxc_exec.snap.md index 9e0ff50654..2d05c7a6c6 100644 --- a/tasks/transform_conformance/snapshots/oxc_exec.snap.md +++ b/tasks/transform_conformance/snapshots/oxc_exec.snap.md @@ -2,7 +2,7 @@ commit: b835f0c6 node: v26.12.1 -Passed: 43 of 45 (95.56%) +Passed: 44 of 46 (95.65%) Failures: diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/exec.js b/tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/exec.js new file mode 100644 index 0000000000..5542707932 --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/exec.js @@ -0,0 +1,144 @@ +let methodCalls = 0; + +class Methods { + #method() { + methodCalls++; + } + + static #staticMethod() { + methodCalls++; + } + + method() { + return this.#method; + } + + static method() { + return this.#staticMethod; + } + + run(operator, rhs, object = this) { + switch (operator) { + case "||=": return (object.#method ||= rhs()); + case "??=": return (object.#method ??= rhs()); + case "&&=": return (object.#method &&= rhs()); + case "+=": return (object.#method += rhs()); + case "-=": return (object.#method -= rhs()); + case "*=": return (object.#method *= rhs()); + case "/=": return (object.#method /= rhs()); + case "%=": return (object.#method %= rhs()); + case "**=": return (object.#method **= rhs()); + case "<<=": return (object.#method <<= rhs()); + case ">>=": return (object.#method >>= rhs()); + case ">>>=": return (object.#method >>>= rhs()); + case "&=": return (object.#method &= rhs()); + case "^=": return (object.#method ^= rhs()); + case "|=": return (object.#method |= rhs()); + case "pre++": return ++object.#method; + case "post++": return object.#method++; + case "pre--": return --object.#method; + case "post--": return object.#method--; + } + } + + static run(operator, rhs, object = this) { + switch (operator) { + case "||=": return (object.#staticMethod ||= rhs()); + case "??=": return (object.#staticMethod ??= rhs()); + case "&&=": return (object.#staticMethod &&= rhs()); + case "+=": return (object.#staticMethod += rhs()); + case "-=": return (object.#staticMethod -= rhs()); + case "*=": return (object.#staticMethod *= rhs()); + case "/=": return (object.#staticMethod /= rhs()); + case "%=": return (object.#staticMethod %= rhs()); + case "**=": return (object.#staticMethod **= rhs()); + case "<<=": return (object.#staticMethod <<= rhs()); + case ">>=": return (object.#staticMethod >>= rhs()); + case ">>>=": return (object.#staticMethod >>>= rhs()); + case "&=": return (object.#staticMethod &= rhs()); + case "^=": return (object.#staticMethod ^= rhs()); + case "|=": return (object.#staticMethod |= rhs()); + case "pre++": return ++object.#staticMethod; + case "post++": return object.#staticMethod++; + case "pre--": return --object.#staticMethod; + case "post--": return object.#staticMethod--; + } + } + + fromCall(operator, getObject, rhs) { + switch (operator) { + case "||=": return (getObject().#method ||= rhs()); + case "&&=": return (getObject().#method &&= rhs()); + case "+=": return (getObject().#method += rhs()); + case "post++": return getObject().#method++; + } + } + + static fromCall(operator, getObject, rhs) { + switch (operator) { + case "||=": return (getObject().#staticMethod ||= rhs()); + case "&&=": return (getObject().#staticMethod &&= rhs()); + case "+=": return (getObject().#staticMethod += rhs()); + case "post++": return getObject().#staticMethod++; + } + } +} + +const binaryOperators = ["+=", "-=", "*=", "/=", "%=", "**=", "<<=", ">>=", ">>>=", "&=", "^=", "|="]; +const updateOperators = ["pre++", "post++", "pre--", "post--"]; + +for (const receiver of [new Methods(), Methods]) { + let rhsCalls = 0; + const rhs = () => { + rhsCalls++; + return 2; + }; + const method = receiver.method(); + + for (const operator of ["||=", "??="]) { + expect(receiver.run(operator, rhs)).toBe(method); + expect(rhsCalls).toBe(0); + expect(methodCalls).toBe(0); + } + + for (const operator of ["&&=", ...binaryOperators]) { + rhsCalls = 0; + expect(() => receiver.run(operator, rhs)).toThrow(TypeError); + expect(rhsCalls).toBe(1); + expect(methodCalls).toBe(0); + } + + for (const operator of updateOperators) { + rhsCalls = 0; + expect(() => receiver.run(operator, rhs)).toThrow(TypeError); + expect(rhsCalls).toBe(0); + expect(methodCalls).toBe(0); + } + + // A failed brand check must precede RHS evaluation for every compound operator. + for (const operator of ["||=", "??=", "&&=", ...binaryOperators, ...updateOperators]) { + rhsCalls = 0; + expect(() => receiver.run(operator, rhs, {})).toThrow(TypeError); + expect(rhsCalls).toBe(0); + expect(methodCalls).toBe(0); + } + + let objectCalls = 0; + const getObject = () => { + objectCalls++; + return receiver; + }; + expect(receiver.fromCall("||=", getObject, rhs)).toBe(method); + expect(objectCalls).toBe(1); + expect(rhsCalls).toBe(0); + expect(methodCalls).toBe(0); + + for (const operator of ["&&=", "+=", "post++"]) { + objectCalls = 0; + rhsCalls = 0; + expect(() => receiver.fromCall(operator, getObject, rhs)).toThrow(TypeError); + expect(objectCalls).toBe(1); + expect(rhsCalls).toBe(operator === "post++" ? 0 : 1); + expect(methodCalls).toBe(0); + } +} diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/input.js b/tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/input.js new file mode 100644 index 0000000000..3fade0d74e --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/input.js @@ -0,0 +1,22 @@ +class Methods { + #method() {} + static #staticMethod() {} + + run(object, rhs) { + object.#method ||= rhs(); + object.#method ??= rhs(); + object.#method &&= rhs(); + object.#method += rhs(); + ++object.#method; + object.#method--; + } + + static run(object, rhs) { + object.#staticMethod ||= rhs(); + object.#staticMethod ??= rhs(); + object.#staticMethod &&= rhs(); + object.#staticMethod += rhs(); + ++object.#staticMethod; + object.#staticMethod--; + } +} diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/output.js b/tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/output.js new file mode 100644 index 0000000000..74b46a0636 --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-private-methods/test/fixtures/oxc/private-method-assignment-read/output.js @@ -0,0 +1,26 @@ +var _Methods_brand = /* @__PURE__ */ new WeakSet(); +class Methods { + constructor() { + babelHelpers.classPrivateMethodInitSpec(this, _Methods_brand); + } + run(object, rhs) { + var _object$method, _object$method2, _object$method3; + babelHelpers.assertClassBrand(_Methods_brand, object, _method) || (object, rhs(), babelHelpers.readOnlyError("#method")); + babelHelpers.assertClassBrand(_Methods_brand, object, _method) ?? (object, rhs(), babelHelpers.readOnlyError("#method")); + babelHelpers.assertClassBrand(_Methods_brand, object, _method) && (object, rhs(), babelHelpers.readOnlyError("#method")); + object, babelHelpers.assertClassBrand(_Methods_brand, object, _method) + rhs(), babelHelpers.readOnlyError("#method"); + object, _object$method = babelHelpers.assertClassBrand(_Methods_brand, object, _method), ++_object$method, babelHelpers.readOnlyError("#method"); + object, _object$method2 = babelHelpers.assertClassBrand(_Methods_brand, object, _method), _object$method3 = _object$method2--, _object$method2, babelHelpers.readOnlyError("#method"), _object$method3; + } + static run(object, rhs) { + var _object$staticMethod, _object$staticMethod2, _object$staticMethod3; + babelHelpers.assertClassBrand(Methods, object, _staticMethod) || (object, rhs(), babelHelpers.readOnlyError("#staticMethod")); + babelHelpers.assertClassBrand(Methods, object, _staticMethod) ?? (object, rhs(), babelHelpers.readOnlyError("#staticMethod")); + babelHelpers.assertClassBrand(Methods, object, _staticMethod) && (object, rhs(), babelHelpers.readOnlyError("#staticMethod")); + object, babelHelpers.assertClassBrand(Methods, object, _staticMethod) + rhs(), babelHelpers.readOnlyError("#staticMethod"); + object, _object$staticMethod = babelHelpers.assertClassBrand(Methods, object, _staticMethod), ++_object$staticMethod, babelHelpers.readOnlyError("#staticMethod"); + object, _object$staticMethod2 = babelHelpers.assertClassBrand(Methods, object, _staticMethod), _object$staticMethod3 = _object$staticMethod2--, _object$staticMethod2, babelHelpers.readOnlyError("#staticMethod"), _object$staticMethod3; + } +} +function _method() {} +function _staticMethod() {}