mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
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.
This commit is contained in:
1 parent
e6c4feffcc
commit
ea76490ca5
7 files changed
+213
-10
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
commit: b835f0c6
|
||||
|
||||
Passed: 308/437
|
||||
Passed: 309/438
|
||||
|
||||
# All Passed:
|
||||
* babel-plugin-transform-class-static-block
|
||||
|
||||
@@ -2,7 +2,7 @@ commit: b835f0c6
|
||||
|
||||
node: v26.12.1
|
||||
|
||||
Passed: 43 of 45 (95.56%)
|
||||
Passed: 44 of 46 (95.65%)
|
||||
|
||||
Failures:
|
||||
|
||||
|
||||
+144
@@ -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);
|
||||
}
|
||||
}
|
||||
+22
@@ -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--;
|
||||
}
|
||||
}
|
||||
+26
@@ -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() {}
|
||||
Reference in new issue
Block a user