From c0576faf80384981da355f4ae58c14a9a33be8af Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Sat, 14 Dec 2024 04:18:48 +0000 Subject: [PATCH] fix(transformer/class-properties): use UID for `args` in created class constructor (#7866) When creating class constructor for a class which has super class, use UID `_args` for temp var (rather than `args`). This avoids shadowing a var called `args` used in an instance property initializer. This diverges from Babel. Babel uses `args` unless it finds a var called `args` in an instance property initializer. But searching the AST of initializers can be fairly expensive, so it's better to skip it. The overrides for test fixtures included in this PR are just to account for that difference. --- .../es2022/class_properties/constructor.rs | 13 ++++--------- .../instance-field/output.js | 6 ++++++ .../derived/output.js | 6 ++++++ .../super-call/output.js | 11 +++++++++++ .../fixtures/private-loose/derived/output.js | 19 +++++++++++++++++++ .../output.js | 10 +++++----- .../test/fixtures/private/derived/output.js | 13 +++++++++++++ .../fixtures/private/super-call/output.js | 12 ++++++++++++ .../fixtures/public-loose/derived/output.js | 6 ++++++ .../public-loose/super-call/output.js | 11 +++++++++++ .../test/fixtures/public/derived/output.js | 6 ++++++ .../test/fixtures/public/super-call/output.js | 11 +++++++++++ .../test/fixtures/regression/6154/output.js | 13 +++++++++++++ .../snapshots/babel.snap.md | 7 ++----- 14 files changed, 125 insertions(+), 19 deletions(-) create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-constantSuper/instance-field/output.js create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-setPublicClassFields/derived/output.js create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-setPublicClassFields/super-call/output.js create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private-loose/derived/output.js create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private/derived/output.js create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private/super-call/output.js create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/derived/output.js create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/super-call/output.js create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public/derived/output.js create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public/super-call/output.js create mode 100644 tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/regression/6154/output.js diff --git a/crates/oxc_transformer/src/es2022/class_properties/constructor.rs b/crates/oxc_transformer/src/es2022/class_properties/constructor.rs index ebd796f4390bf..ae497c34edfa4 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/constructor.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/constructor.rs @@ -228,17 +228,12 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { let has_super_class = class.super_class.is_some(); let mut stmts = ctx.ast.vec_with_capacity(inits.len() + usize::from(has_super_class)); - // Add `super(...args);` statement and `...args` param if class has a super class. - // `constructor(...args) { super(...args); /* prop initialization */ }` - // TODO: One of initializers could access a var called `args` from outer scope. - // Use a UID `_args` instead of `args` here. + // Add `super(..._args);` statement and `..._args` param if class has a super class. + // `constructor(..._args) { super(..._args); /* prop initialization */ }` let mut params_rest = None; if has_super_class { - let args_binding = ctx.generate_binding( - Atom::from("args"), - constructor_scope_id, - SymbolFlags::FunctionScopedVariable, - ); + let args_binding = + ctx.generate_uid("args", constructor_scope_id, SymbolFlags::FunctionScopedVariable); params_rest = Some( ctx.ast.alloc_binding_rest_element(SPAN, args_binding.create_binding_pattern(ctx)), ); diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-constantSuper/instance-field/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-constantSuper/instance-field/output.js new file mode 100644 index 0000000000000..c8b1e695bebf9 --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-constantSuper/instance-field/output.js @@ -0,0 +1,6 @@ +class A extends B { + constructor(..._args) { + super(..._args); + babelHelpers.defineProperty(this, "foo", super.bar); + } +} diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-setPublicClassFields/derived/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-setPublicClassFields/derived/output.js new file mode 100644 index 0000000000000..35544eaf81583 --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-setPublicClassFields/derived/output.js @@ -0,0 +1,6 @@ +class Foo extends Bar { + constructor(..._args) { + super(..._args); + this.bar = "foo"; + } +} diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-setPublicClassFields/super-call/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-setPublicClassFields/super-call/output.js new file mode 100644 index 0000000000000..90e8cab409b12 --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/assumption-setPublicClassFields/super-call/output.js @@ -0,0 +1,11 @@ +class A { + foo() { + return "bar"; + } +} +class B extends A { + constructor(..._args) { + super(..._args); + this.foo = super.foo(); + } +} diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private-loose/derived/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private-loose/derived/output.js new file mode 100644 index 0000000000000..e8a064c1a740d --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private-loose/derived/output.js @@ -0,0 +1,19 @@ +var _prop = babelHelpers.classPrivateFieldLooseKey("prop"); +class Foo { + constructor() { + Object.defineProperty(this, _prop, { + writable: true, + value: "foo" + }); + } +} +var _prop2 = babelHelpers.classPrivateFieldLooseKey("prop"); +class Bar extends Foo { + constructor(..._args) { + super(..._args); + Object.defineProperty(this, _prop2, { + writable: true, + value: "bar" + }); + } +} diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private-loose/nested-class-extends-computed-redeclared/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private-loose/nested-class-extends-computed-redeclared/output.js index a01bfb9c1b51d..75f00d33b8dcd 100644 --- a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private-loose/nested-class-extends-computed-redeclared/output.js +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private-loose/nested-class-extends-computed-redeclared/output.js @@ -1,4 +1,4 @@ -var _foo = /*#__PURE__*/babelHelpers.classPrivateFieldLooseKey("foo"); +var _foo = babelHelpers.classPrivateFieldLooseKey("foo"); class Foo { constructor() { Object.defineProperty(this, _foo, { @@ -9,8 +9,8 @@ class Foo { test() { var _foo3; let _this$foo; - var _foo2 = /*#__PURE__*/babelHelpers.classPrivateFieldLooseKey("foo"); - class Nested extends (_foo3 = /*#__PURE__*/babelHelpers.classPrivateFieldLooseKey("foo"), _this$foo = babelHelpers.classPrivateFieldLooseBase(this, _foo3)[_foo3], class { + var _foo2 = babelHelpers.classPrivateFieldLooseKey("foo"); + class Nested extends (_foo3 = babelHelpers.classPrivateFieldLooseKey("foo"), _this$foo = babelHelpers.classPrivateFieldLooseBase(this, _foo3)[_foo3], class { constructor() { Object.defineProperty(this, _foo3, { writable: true, @@ -19,8 +19,8 @@ class Foo { this[_this$foo] = 2; } }) { - constructor(...args) { - super(...args); + constructor(..._args) { + super(..._args); Object.defineProperty(this, _foo2, { writable: true, value: 3 diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private/derived/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private/derived/output.js new file mode 100644 index 0000000000000..bdbb7d9f63bb7 --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private/derived/output.js @@ -0,0 +1,13 @@ +var _prop = new WeakMap(); +class Foo { + constructor() { + babelHelpers.classPrivateFieldInitSpec(this, _prop, "foo"); + } +} +var _prop2 = new WeakMap(); +class Bar extends Foo { + constructor(..._args) { + super(..._args); + babelHelpers.classPrivateFieldInitSpec(this, _prop2, "bar"); + } +} diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private/super-call/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private/super-call/output.js new file mode 100644 index 0000000000000..ec9d7c1925493 --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private/super-call/output.js @@ -0,0 +1,12 @@ +class A { + foo() { + return "bar"; + } +} +var _foo = new WeakMap(); +class B extends A { + constructor(..._args) { + super(..._args); + babelHelpers.classPrivateFieldInitSpec(this, _foo, super.foo()); + } +} diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/derived/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/derived/output.js new file mode 100644 index 0000000000000..35544eaf81583 --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/derived/output.js @@ -0,0 +1,6 @@ +class Foo extends Bar { + constructor(..._args) { + super(..._args); + this.bar = "foo"; + } +} diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/super-call/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/super-call/output.js new file mode 100644 index 0000000000000..90e8cab409b12 --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/super-call/output.js @@ -0,0 +1,11 @@ +class A { + foo() { + return "bar"; + } +} +class B extends A { + constructor(..._args) { + super(..._args); + this.foo = super.foo(); + } +} diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public/derived/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public/derived/output.js new file mode 100644 index 0000000000000..16a42bfa25d7a --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public/derived/output.js @@ -0,0 +1,6 @@ +class Foo extends Bar { + constructor(..._args) { + super(..._args); + babelHelpers.defineProperty(this, "bar", "foo"); + } +} diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public/super-call/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public/super-call/output.js new file mode 100644 index 0000000000000..be072ebce3e7e --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public/super-call/output.js @@ -0,0 +1,11 @@ +class A { + foo() { + return "bar"; + } +} +class B extends A { + constructor(..._args) { + super(..._args); + babelHelpers.defineProperty(this, "foo", super.foo()); + } +} diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/regression/6154/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/regression/6154/output.js new file mode 100644 index 0000000000000..0abe0c10653ba --- /dev/null +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/regression/6154/output.js @@ -0,0 +1,13 @@ +class Test { + constructor() { + var _Other; + class Other extends Test { + constructor(..._args) { + super(..._args); + babelHelpers.defineProperty(this, "a", () => super.test); + } + } + _Other = Other; + babelHelpers.defineProperty(Other, "a", () => babelHelpers.superPropGet(_Other, "test", _Other)); + } +} diff --git a/tasks/transform_conformance/snapshots/babel.snap.md b/tasks/transform_conformance/snapshots/babel.snap.md index 3ea56699bb48d..6fe4b22662e97 100644 --- a/tasks/transform_conformance/snapshots/babel.snap.md +++ b/tasks/transform_conformance/snapshots/babel.snap.md @@ -1,6 +1,6 @@ commit: 54a8389f -Passed: 581/927 +Passed: 582/927 # All Passed: * babel-plugin-transform-class-static-block @@ -276,7 +276,7 @@ x Output mismatch x Output mismatch -# babel-plugin-transform-class-properties (193/264) +# babel-plugin-transform-class-properties (194/264) * assumption-constantSuper/complex-super-class/input.js x Output mismatch @@ -499,9 +499,6 @@ x Output mismatch * regression/6153/input.js x Output mismatch -* regression/7951/input.mjs -x Output mismatch - # babel-plugin-transform-nullish-coalescing-operator (5/12) * assumption-noDocumentAll/transform/input.js