From aa18612e62eee6f0fd1977c10aa2102ce113e6e4 Mon Sep 17 00:00:00 2001 From: Austaras Date: Mon, 31 Oct 2022 10:29:55 +0800 Subject: [PATCH] fix(es/compat): Fix `new.target` in a nested scope (#6296) --- .../tsc-references/newTarget.es5.1.normal.js | 20 +- .../src/es2015/new_target.rs | 153 +++------------ .../tests/es2015_new_target.rs | 185 ++++++++++++------ .../general/class-properties-loose/output.js | 8 +- .../general/class-properties/output.js | 8 +- .../general/extended-class/output.js | 1 + 6 files changed, 175 insertions(+), 200 deletions(-) diff --git a/crates/swc/tests/tsc-references/newTarget.es5.1.normal.js b/crates/swc/tests/tsc-references/newTarget.es5.1.normal.js index 24941f19612f..51afde121097 100644 --- a/crates/swc/tests/tsc-references/newTarget.es5.1.normal.js +++ b/crates/swc/tests/tsc-references/newTarget.es5.1.normal.js @@ -5,14 +5,14 @@ import _instanceof from "@swc/helpers/src/_instanceof.mjs"; import _create_super from "@swc/helpers/src/_create_super.mjs"; var A = function A() { "use strict"; - var _newtarget = _instanceof(this, A) ? this.constructor : void 0; + var _this = this; _class_call_check(this, A); this.d = function _target() { return _instanceof(this, _target) ? this.constructor : void 0; }; var a = _instanceof(this, A) ? this.constructor : void 0; var b = function() { - return _newtarget; + return _instanceof(_this, A) ? _this.constructor : void 0; }; }; A.c = function _target() { @@ -23,29 +23,29 @@ var B = /*#__PURE__*/ function _target(A) { _inherits(B, A); var _super = _create_super(B); function B() { - var _newtarget = _instanceof(this, B) ? this.constructor : void 0; + var _this = this; _class_call_check(this, B); - var _this = _super.call(this); + var _this1 = _super.call(this); var e = _instanceof(this, B) ? this.constructor : void 0; var f = function() { - return _newtarget; + return _instanceof(_this, B) ? _this.constructor : void 0; }; - return _this; + return _this1; } return B; }(A); function f1() { - var _newtarget = _instanceof(this, f1) ? this.constructor : void 0; + var _this = this; var g = _instanceof(this, f1) ? this.constructor : void 0; var h = function() { - return _newtarget; + return _instanceof(_this, f1) ? _this.constructor : void 0; }; } var f2 = function _target() { - var _newtarget = _instanceof(this, _target) ? this.constructor : void 0; + var _this = this; var i = _instanceof(this, _target) ? this.constructor : void 0; var j = function() { - return _newtarget; + return _instanceof(_this, _target) ? _this.constructor : void 0; }; }; var O = { diff --git a/crates/swc_ecma_transforms_compat/src/es2015/new_target.rs b/crates/swc_ecma_transforms_compat/src/es2015/new_target.rs index 603054ca905b..b044f20beef7 100644 --- a/crates/swc_ecma_transforms_compat/src/es2015/new_target.rs +++ b/crates/swc_ecma_transforms_compat/src/es2015/new_target.rs @@ -1,9 +1,9 @@ -use std::borrow::Cow; +use std::{borrow::Cow, mem}; use swc_common::{pass::CompilerPass, DUMMY_SP}; use swc_ecma_ast::*; use swc_ecma_transforms_base::perf::{should_work, Check}; -use swc_ecma_utils::{prepend_stmt, private_ident, quote_ident, undefined, ExprFactory}; +use swc_ecma_utils::{private_ident, quote_ident, undefined, ExprFactory}; use swc_ecma_visit::{ as_folder, noop_visit_mut_type, noop_visit_type, Fold, Visit, VisitMut, VisitMutWith, }; @@ -11,30 +11,28 @@ use swc_trace_macro::swc_trace; #[tracing::instrument(level = "info", skip_all)] pub fn new_target() -> impl Fold + VisitMut + CompilerPass { - as_folder(NewTarget::default()) + as_folder(NewTarget { + ctx: Ctx::Constructor, + }) } -#[derive(Default)] - struct NewTarget { - cur: Option, - - in_constructor: bool, - in_method: bool, - in_arrow_expr: bool, + ctx: Ctx, +} - var: Option, +enum Ctx { + Constructor, + Method, + Function(Ident), } impl NewTarget { fn visit_mut_method>(&mut self, c: &mut T) { - let old = self.in_method; - - self.in_method = true; + let old = mem::replace(&mut self.ctx, Ctx::Method); c.visit_mut_with(self); - self.in_method = old; + self.ctx = old; } } @@ -42,50 +40,6 @@ impl NewTarget { impl VisitMut for NewTarget { noop_visit_mut_type!(); - fn visit_mut_arrow_expr(&mut self, e: &mut ArrowExpr) { - // Ensure that `e` contains new.target - if !should_work::(&*e) { - return; - } - - let old = self.in_arrow_expr; - if self.var.is_none() { - let mut v = Expr::MetaProp(MetaPropExpr { - span: DUMMY_SP, - kind: MetaPropKind::NewTarget, - }); - v.visit_mut_with(self); - self.var.get_or_insert_with(|| VarDeclarator { - span: DUMMY_SP, - name: private_ident!("_newtarget").into(), - init: Some(Box::new(v)), - definite: Default::default(), - }); - } - self.in_arrow_expr = true; - e.visit_mut_children_with(self); - - self.in_arrow_expr = old; - } - - fn visit_mut_class_decl(&mut self, class: &mut ClassDecl) { - let old = self.cur.take(); - self.cur = Some(class.ident.clone()); - - class.visit_mut_children_with(self); - - self.cur = old; - } - - fn visit_mut_class_expr(&mut self, class: &mut ClassExpr) { - let old = self.cur.take(); - self.cur = class.ident.clone(); - - class.visit_mut_children_with(self); - - self.cur = old; - } - fn visit_mut_class_method(&mut self, c: &mut ClassMethod) { c.key.visit_mut_with(self); @@ -93,15 +47,11 @@ impl VisitMut for NewTarget { } fn visit_mut_constructor(&mut self, c: &mut Constructor) { - let old = self.in_constructor; - - self.in_constructor = true; - self.in_arrow_expr = false; - self.var = None; + let old = mem::replace(&mut self.ctx, Ctx::Constructor); c.visit_mut_children_with(self); - self.in_constructor = old; + self.ctx = old; } fn visit_mut_expr(&mut self, e: &mut Expr) { @@ -109,34 +59,27 @@ impl VisitMut for NewTarget { if let Expr::MetaProp(MetaPropExpr { kind: MetaPropKind::NewTarget, - .. + span, }) = e { - if self.in_arrow_expr { - *e = Expr::Ident(self.var.as_ref().unwrap().name.clone().ident().unwrap().id); - } else if self.in_method { - *e = *undefined(DUMMY_SP) - } else if let Some(cur) = self.cur.clone() { - let c = ThisExpr { span: DUMMY_SP }.make_member(quote_ident!("constructor")); - - if self.in_constructor { - *e = c; - } else { - // (this instanceof Foo ? this.constructor : void 0) + let this_ctor = |span| ThisExpr { span }.make_member(quote_ident!("constructor")); + match &self.ctx { + Ctx::Constructor => *e = this_ctor(*span), + Ctx::Method => *e = *undefined(DUMMY_SP), + Ctx::Function(i) => { *e = Expr::Cond(CondExpr { - span: DUMMY_SP, + span: *span, // this instanceof Foo test: Box::new(Expr::Bin(BinExpr { span: DUMMY_SP, op: op!("instanceof"), left: Box::new(Expr::This(ThisExpr { span: DUMMY_SP })), - right: Box::new(Expr::Ident(cur)), + right: Box::new(Expr::Ident(i.clone())), })), - // this.constructor - cons: Box::new(c), + cons: Box::new(this_ctor(DUMMY_SP)), // void 0 alt: undefined(DUMMY_SP), - }); + }) } } } @@ -148,12 +91,11 @@ impl VisitMut for NewTarget { return; } - let old = self.cur.take(); - self.cur = Some(f.ident.clone()); + let old = mem::replace(&mut self.ctx, Ctx::Function(f.ident.clone())); f.visit_mut_children_with(self); - self.cur = old; + self.ctx = old; } fn visit_mut_fn_expr(&mut self, f: &mut FnExpr) { @@ -167,12 +109,11 @@ impl VisitMut for NewTarget { .get_or_insert_with(|| private_ident!("_target")) .clone(); - let old = self.cur.take(); - self.cur = Some(i); + let old = mem::replace(&mut self.ctx, Ctx::Function(i)); f.visit_mut_children_with(self); - self.cur = old; + self.ctx = old; } fn visit_mut_method_prop(&mut self, m: &mut MethodProp) { @@ -189,42 +130,6 @@ impl VisitMut for NewTarget { m.key.visit_mut_with(self); self.visit_mut_method(&mut m.body) } - - fn visit_mut_module_items(&mut self, stmts: &mut Vec) { - stmts.visit_mut_children_with(self); - - if let Some(var) = self.var.take() { - prepend_stmt( - stmts, - VarDecl { - span: DUMMY_SP, - kind: VarDeclKind::Var, - declare: false, - decls: vec![var], - } - .into(), - ) - } - } - - fn visit_mut_stmts(&mut self, stmts: &mut Vec) { - stmts.visit_mut_children_with(self); - - if !self.in_arrow_expr { - if let Some(var) = self.var.take() { - prepend_stmt( - stmts, - VarDecl { - span: DUMMY_SP, - kind: VarDeclKind::Var, - declare: false, - decls: vec![var], - } - .into(), - ) - } - } - } } impl CompilerPass for NewTarget { diff --git a/crates/swc_ecma_transforms_compat/tests/es2015_new_target.rs b/crates/swc_ecma_transforms_compat/tests/es2015_new_target.rs index 7ee535e0682a..5a165b66d889 100644 --- a/crates/swc_ecma_transforms_compat/tests/es2015_new_target.rs +++ b/crates/swc_ecma_transforms_compat/tests/es2015_new_target.rs @@ -5,16 +5,76 @@ use swc_common::{chain, Mark}; use swc_ecma_parser::{EsConfig, Syntax}; use swc_ecma_transforms_base::pass::noop; use swc_ecma_transforms_compat::{ - es2015::{arrow, new_target::new_target}, + es2015::{arrow, classes, new_target::new_target}, es2022::class_properties, }; -use swc_ecma_transforms_testing::{exec_tr, parse_options, test, test_fixture}; +use swc_ecma_transforms_testing::{exec_tr, parse_options, test, test_fixture, Tester}; use swc_ecma_visit::Fold; +fn get_passes(t: &Tester, plugins: &[PluginConfig]) -> Box { + let mut pass: Box = Box::new(noop()); + + for plugin in plugins { + let (name, option) = match plugin { + PluginConfig::WithOption(name, config) => (name, config.clone()), + PluginConfig::Name(name) => (name, serde_json::Value::Null), + }; + + let loose = option + .as_object() + .and_then(|opt| opt.get("loose")) + .and_then(|loose| { + if let Some(true) = loose.as_bool() { + Some(()) + } else { + None + } + }) + .is_some(); + + match &**name { + "transform-new-target" => {} + + "proposal-class-properties" => { + pass = Box::new(chain!( + pass, + class_properties( + Some(t.comments.clone()), + class_properties::Config { + constant_super: loose, + set_public_fields: loose, + private_as_properties: loose, + no_document_all: loose + } + ) + )); + } + + "transform-arrow-functions" => { + pass = Box::new(chain!(pass, arrow(Mark::new()))); + } + + _ => { + panic!("unknown pass: {}", name) + } + } + } + + pass = Box::new(chain!(pass, new_target())); + + pass +} + #[testing::fixture("tests/new-target/**/exec.js")] fn exec(input: PathBuf) { + let options: TestOptions = parse_options(&input); let input = read_to_string(&input).unwrap(); - exec_tr("new-target", Default::default(), |_| new_target(), &input); + exec_tr( + "new-target", + Default::default(), + |t| get_passes(t, &options.plugins), + &input, + ); } #[derive(Debug, Clone, Deserialize)] @@ -40,59 +100,7 @@ fn fixture(input: PathBuf) { private_in_object: true, ..Default::default() }), - &|t| { - let mut pass: Box = Box::new(noop()); - - for plugin in &options.plugins { - let (name, option) = match plugin { - PluginConfig::WithOption(name, config) => (name, config.clone()), - PluginConfig::Name(name) => (name, serde_json::Value::Null), - }; - - let loose = option - .as_object() - .and_then(|opt| opt.get("loose")) - .and_then(|loose| { - if let Some(true) = loose.as_bool() { - Some(()) - } else { - None - } - }) - .is_some(); - - match &**name { - "transform-new-target" => {} - - "proposal-class-properties" => { - pass = Box::new(chain!( - pass, - class_properties( - Some(t.comments.clone()), - class_properties::Config { - constant_super: loose, - set_public_fields: loose, - private_as_properties: loose, - no_document_all: loose - } - ) - )); - } - - "transform-arrow-functions" => { - pass = Box::new(chain!(pass, arrow(Mark::new()))); - } - - _ => { - panic!("unknown pass: {}", name) - } - } - } - - pass = Box::new(chain!(pass, new_target())); - - pass - }, + &|t| get_passes(t, &options.plugins), &input, &output, Default::default(), @@ -107,8 +115,7 @@ test!( const a = () => new.target }"#, r#"function foo() { - var _newtarget = this instanceof foo ? this.constructor : void 0; - const a = () => _newtarget + const a = () => this instanceof foo ? this.constructor : void 0 }"# ); @@ -135,3 +142,65 @@ test!( } };"# ); + +test!( + ::swc_ecma_parser::Syntax::default(), + |_| new_target(), + edge_13, + r#" +class A { + foo() { + return () => new.target + } + + constructor() { + () => new.target + } +} +"#, + r#" +class A { + foo() { + return ()=>void 0; + } + constructor(){ + ()=>this.constructor; + } +} +"# +); + +test!( + ::swc_ecma_parser::Syntax::default(), + |t| chain!( + classes(Some(t.comments.clone()), Default::default()), + new_target() + ), + issue_6259, + r#" +(() => { + var SomeError = class extends Error { + constructor(issues) { + super(); + const actualProto = new.target.prototype; + } + } +})(); +"#, + r#" +(()=>{ + var SomeError = function _target(Error) { + "use strict"; + _inherits(SomeError, Error); + var _super = _createSuper(SomeError); + function SomeError(issues) { + _classCallCheck(this, SomeError); + var _this = _super.call(this); + const actualProto = (this instanceof SomeError ? this.constructor : void 0).prototype; + return _this; + } + return SomeError; + }(_wrapNativeSuper(Error)); +})(); +"# +); diff --git a/crates/swc_ecma_transforms_compat/tests/new-target/general/class-properties-loose/output.js b/crates/swc_ecma_transforms_compat/tests/new-target/general/class-properties-loose/output.js index 2ebc3a6094de..7bd1d4ff5a07 100644 --- a/crates/swc_ecma_transforms_compat/tests/new-target/general/class-properties-loose/output.js +++ b/crates/swc_ecma_transforms_compat/tests/new-target/general/class-properties-loose/output.js @@ -1,7 +1,7 @@ class Foo { constructor(){ this.test = function _target() { - this.constructor; + this instanceof _target ? this.constructor : void 0; }; this.test2 = function() { void 0; @@ -14,17 +14,17 @@ class Foo { } }, _class.p = void 0, _class.p1 = class { constructor(){ - new.target; + this.constructor; } } // should not replace , _class.p2 = new function _target() { - this.constructor; + this instanceof _target ? this.constructor : void 0; }() // should not replace , _class.p3 = function() { void 0; } // should replace , _class.p4 = function _target() { - this.constructor; + this instanceof _target ? this.constructor : void 0; } // should not replace , _class); } diff --git a/crates/swc_ecma_transforms_compat/tests/new-target/general/class-properties/output.js b/crates/swc_ecma_transforms_compat/tests/new-target/general/class-properties/output.js index 343d81f9f3b5..995ca9d0448b 100644 --- a/crates/swc_ecma_transforms_compat/tests/new-target/general/class-properties/output.js +++ b/crates/swc_ecma_transforms_compat/tests/new-target/general/class-properties/output.js @@ -1,7 +1,7 @@ class Foo { constructor(){ _defineProperty(this, "test", function _target() { - this.constructor; + this instanceof _target ? this.constructor : void 0; }); _defineProperty(this, "test2", function() { void 0; @@ -14,17 +14,17 @@ class Foo { } }, _defineProperty(_class, "p", void 0), _defineProperty(_class, "p1", class { constructor(){ - new.target; + this.constructor; } }) // should not replace , _defineProperty(_class, "p2", new function _target() { - this.constructor; + this instanceof _target ? this.constructor : void 0; }()) // should not replace , _defineProperty(_class, "p3", function() { void 0; }) // should replace , _defineProperty(_class, "p4", function _target() { - this.constructor; + this instanceof _target ? this.constructor : void 0; }) // should not replace , _class); } diff --git a/crates/swc_ecma_transforms_compat/tests/new-target/general/extended-class/output.js b/crates/swc_ecma_transforms_compat/tests/new-target/general/extended-class/output.js index 423422c676c6..469ec76aefee 100644 --- a/crates/swc_ecma_transforms_compat/tests/new-target/general/extended-class/output.js +++ b/crates/swc_ecma_transforms_compat/tests/new-target/general/extended-class/output.js @@ -5,6 +5,7 @@ class Foo { } class Bar extends Foo { constructor(){ + // This is probably bad... this.constructor; super(); }