Skip to content

Commit

Permalink
fix(es/transforms/module): Use correct this (#1561)
Browse files Browse the repository at this point in the history 
swc_ecma_transforms_module:
 - Preserve semantics of `this` in imported functions. (#1556)
  • Loading branch information
@Austaras
Austaras authored Apr 13, 2021
1 parent 92bbde3 commit df2a926
Show file tree
Hide file tree
Showing 16 changed files with 202 additions and 70 deletions.
124 changes: 96 additions & 28 deletions ecmascript/transforms/base/src/fixer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,18 +454,27 @@ impl VisitMut for Fixer<'_> {

impl Fixer<'_> {
fn wrap_with_paren_if_required(&mut self, e: &mut Expr) {
let mut has_padding_value = false;
match e {
// Flatten seq expr
Expr::Seq(SeqExpr { span, exprs }) => {
let len = exprs
.iter()
.map(|expr| match **expr {
Expr::Paren(ParenExpr { ref expr, .. }) => {
if let Expr::Seq(SeqExpr { exprs, .. }) = expr.as_ref() {
exprs.len()
} else {
1
}
}
Expr::Seq(SeqExpr { ref exprs, .. }) => exprs.len(),
_ => 1,
})
.sum();

let exprs_len = exprs.len();
// don't has child seq
let expr = if len == exprs_len {
let mut exprs = exprs
.into_iter()
Expand All @@ -475,14 +484,15 @@ impl Fixer<'_> {
if is_last {
Some(e.take())
} else {
ignore_return_value(e.take())
ignore_return_value(e.take(), &mut has_padding_value)
}
})
.collect::<Vec<_>>();
if exprs.len() == 1 {
*e = *exprs.pop().unwrap();
return;
}
let exprs = ignore_padding_value(exprs);
Expr::Seq(SeqExpr { span: *span, exprs })
} else {
let mut buf = Vec::with_capacity(len);
Expand All @@ -493,15 +503,20 @@ impl Fixer<'_> {
Expr::Seq(SeqExpr { ref mut exprs, .. }) => {
let exprs = exprs.take();
if !is_last {
buf.extend(exprs.into_iter().filter_map(ignore_return_value));
buf.extend(exprs.into_iter().filter_map(|expr| {
ignore_return_value(expr, &mut has_padding_value)
}));
} else {
let exprs_len = exprs.len();
for (i, expr) in exprs.into_iter().enumerate() {
let is_last = i + 1 == exprs_len;
if is_last {
buf.push(expr);
} else {
buf.extend(ignore_return_value(expr));
buf.extend(ignore_return_value(
expr,
&mut has_padding_value,
));
}
}
}
Expand All @@ -510,7 +525,10 @@ impl Fixer<'_> {
if is_last {
buf.push(expr.take());
} else {
buf.extend(ignore_return_value(expr.take()));
buf.extend(ignore_return_value(
expr.take(),
&mut has_padding_value,
));
}
}
}
Expand All @@ -520,16 +538,14 @@ impl Fixer<'_> {
*e = *buf.pop().unwrap();
return;
}
buf.shrink_to_fit();

Expr::Seq(SeqExpr {
span: *span,
exprs: buf,
})
let exprs = ignore_padding_value(buf);

Expr::Seq(SeqExpr { span: *span, exprs })
};

match self.ctx {
Context::ForcedExpr { .. } | Context::Callee { .. } => {
Context::ForcedExpr { .. } => {
*e = Expr::Paren(ParenExpr {
span: *span,
expr: Box::new(expr),
Expand Down Expand Up @@ -569,6 +585,16 @@ impl Fixer<'_> {
}
}

Expr::Call(CallExpr {
callee: ExprOrSuper::Expr(ref mut callee),
..
}) if callee.is_seq() => {
*callee = Box::new(Expr::Paren(ParenExpr {
span: callee.span(),
expr: callee.take(),
}))
}

Expr::Call(CallExpr {
callee: ExprOrSuper::Expr(ref mut callee),
..
Expand Down Expand Up @@ -676,9 +702,16 @@ impl Fixer<'_> {
}
}

fn ignore_return_value(expr: Box<Expr>) -> Option<Box<Expr>> {
fn ignore_return_value(expr: Box<Expr>, has_padding_value: &mut bool) -> Option<Box<Expr>> {
match *expr {
Expr::Ident(..) | Expr::Fn(..) | Expr::Lit(..) => None,
Expr::Fn(..) | Expr::Arrow(..) | Expr::Lit(..) => {
if *has_padding_value {
None
} else {
*has_padding_value = true;
Some(expr)
}
}
Expr::Seq(SeqExpr { span, exprs }) => {
let len = exprs.len();
let mut exprs: Vec<_> = exprs
Expand All @@ -688,7 +721,7 @@ fn ignore_return_value(expr: Box<Expr>) -> Option<Box<Expr>> {
if i + 1 == len {
Some(expr)
} else {
ignore_return_value(expr)
ignore_return_value(expr, has_padding_value)
}
})
.collect();
Expand All @@ -702,11 +735,30 @@ fn ignore_return_value(expr: Box<Expr>) -> Option<Box<Expr>> {
op: op!("void"),
arg,
..
}) => ignore_return_value(arg),
}) => ignore_return_value(arg, has_padding_value),
_ => Some(expr),
}
}

// at least 3 element in seq, which means we can safely
// remove that padding, if not at last position
fn ignore_padding_value(exprs: Vec<Box<Expr>>) -> Vec<Box<Expr>> {
let len = exprs.len();

if len > 2 {
exprs
.into_iter()
.enumerate()
.filter_map(|(i, e)| match e.as_ref() {
Expr::Fn(..) | Expr::Arrow(..) | Expr::Lit(..) if i + 1 != len => None,
_ => Some(e),
})
.collect()
} else {
exprs
}
}

#[cfg(test)]
mod tests {
use super::fixer;
Expand Down Expand Up @@ -821,36 +873,44 @@ const _ref = {}, { c =( _tmp = {}, d = _extends({}, _tmp), _tmp) } = _ref;"
((a, b), (c())) + ((d, e), (f()));
",
"var a, b, c, d, e, f;
c() + f()"
(a, b, c()) + (d, e, f())"
);

test_fixer!(fixer_02, "(b, c), d;", "d;");
test_fixer!(fixer_02, "(b, c), d;", "b, c, d;");

test_fixer!(fixer_03, "((a, b), (c && d)) && e;", "c && d && e;");
test_fixer!(fixer_03, "((a, b), (c && d)) && e;", "(a, b, c && d) && e;");

test_fixer!(fixer_04, "for ((a, b), c;;) ;", "for(c;;);");
test_fixer!(fixer_04, "for ((a, b), c;;) ;", "for(a, b, c;;);");

test_fixer!(
fixer_05,
"var a, b, c = (1), d, e, f = (2);
((a, b), c) + ((d, e), f);",
"var a, b, c = 1, d, e, f = 2;
c + f;"
(a, b, c) + (d, e, f);"
);

test_fixer!(
fixer_06,
"var a, b, c, d;
a = ((b, c), d);",
"var a, b, c, d;
a = d;"
a = (b, c, d);"
);

test_fixer!(fixer_07, "a => ((b, c) => ((a, b), c));", "(a)=>(b, c)=>c;");
test_fixer!(
fixer_07,
"a => ((b, c) => ((a, b), c));",
"(a)=>(b, c)=>(a, b, c);"
);

test_fixer!(fixer_08, "typeof (((1), a), (2));", "typeof 2");
test_fixer!(fixer_08, "typeof (((1), a), (2));", "typeof (a, 2)");

test_fixer!(fixer_09, "(((a, b), c), d) ? e : f;", "d ? e : f;");
test_fixer!(
fixer_09,
"(((a, b), c), d) ? e : f;",
"(a, b, c, d) ? e : f;"
);

test_fixer!(
fixer_10,
Expand All @@ -861,16 +921,18 @@ function a() {
",
"
function a() {
return void 3;
return a, void 3;
}
"
);

test_fixer!(fixer_11, "c && ((((2), (3)), d), b);", "c && b");
test_fixer!(fixer_11, "c && ((((2), (3)), d), b);", "c && (d, b)");

test_fixer!(fixer_12, "(((a, b), c), d) + e;", "(a, b, c, d) + e;");

test_fixer!(fixer_12, "(((a, b), c), d) + e;", "d + e;");
test_fixer!(fixer_13, "delete (((1), a), (2));", "delete (a, 2)");

test_fixer!(fixer_13, "delete (((1), a), (2));", "delete 2");
test_fixer!(fixer_14, "(1, 2, a)", "1, a");

identical!(issue_231, "'' + (truthy && '?') + truthy;");

Expand Down Expand Up @@ -950,7 +1012,7 @@ var store = global[SHARED] || (global[SHARED] = {});

identical!(bin_seq_expr_1, "(foo(), op) || (seq(), foo)");

test_fixer!(bin_seq_expr_2, "(foo, op) || (seq, foo)", "op || foo");
identical!(bin_seq_expr_2, "(foo, op) || (seq, foo)");

identical!(cond_object_1, "let foo = {} ? 1 : 2;");

Expand Down Expand Up @@ -1088,6 +1150,12 @@ var store = global[SHARED] || (global[SHARED] = {});
identical!(issue_1493, "('a' ?? 'b') || ''");
identical!(call_seq, "let x = ({}, () => 2)();");

test_fixer!(
call_seq_with_padding,
"let x = ({}, (1, 2), () => 2)();",
"let x = ({}, () => 2)();"
);

identical!(
param_seq,
"function t(x = ({}, 2)) {
Expand Down
4 changes: 2 additions & 2 deletions ecmascript/transforms/compat/tests/es2015_destructuring.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ test!(
[{ a = 1 }] = foo"#,
r#"let a;
var ref, ref1, ref2;
ref = foo, ref1 = ref[0], ref2 = ref1.a, a = ref2 === void 0 ? 1 : ref2, ref;"#
ref = foo, ref1 = ref[0], ref2 = ref1.a, a = ref2 === void 0 ? 1 : ref2, ref1, ref;"#
);

test!(
Expand Down Expand Up @@ -1118,7 +1118,7 @@ var x = z[x],
var _o;
var ref;
_o = o, z = _objectWithoutProperties(_o, ['x', 'y']), ref = _o, x = ref.x, y = ref.y, _o;
_o = o, z = _objectWithoutProperties(_o, ['x', 'y']), ref = _o, x = ref.x, y = ref.y, ref, _o;
"#
Expand Down
2 changes: 1 addition & 1 deletion ecmascript/transforms/compat/tests/es2020_class_properties.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3942,7 +3942,7 @@ class SuperClass extends Obj {
constructor() {
var _temp;
class B extends (_temp = super(), _defineProperty(this, 'field', 1), Obj) {
class B extends (_temp = super(), _defineProperty(this, 'field', 1), _temp, Obj) {
constructor() {
super();
expect(this.field).toBeUndefined();
Expand Down
4 changes: 2 additions & 2 deletions ecmascript/transforms/compat/tests/es2020_optional_chaining.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ orders === null || orders === void 0 ? void 0 : orders[0].price;
orders === null || orders === void 0 ? void 0 : (ref5 = orders[0]) === null || ref5 === void 0 ? void 0 : ref5.price;
orders[client === null || client === void 0 ? void 0 : client.key].price;
(ref6 = orders[client.key]) === null || ref6 === void 0 ? void 0 : ref6.price;
(a === null || a === void 0 ? void 0 : a.b).c;
((ref7 = (a === null || a === void 0 ? void 0 : a.b).c) === null || ref7 === void 0 ? void 0 : ref7.d).e;
(0, a === null || a === void 0 ? void 0 : a.b).c;
(0, (ref7 = (0, a === null || a === void 0 ? void 0 : a.b).c) === null || ref7 === void 0 ? void 0 : ref7.d).e;
"#
);

Expand Down
50 changes: 49 additions & 1 deletion ecmascript/transforms/module/src/util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::{
use swc_atoms::{js_word, JsWord};
use swc_common::{Mark, Span, SyntaxContext, DUMMY_SP};
use swc_ecma_ast::*;
use swc_ecma_transforms_base::ext::MapWithMut;
use swc_ecma_utils::member_expr;
use swc_ecma_utils::private_ident;
use swc_ecma_utils::quote_ident;
Expand Down Expand Up @@ -489,7 +490,54 @@ impl Scope {
_ => false,
} =>
{
return folder.make_dynamic_import(span, args)
folder.make_dynamic_import(span, args)
}

Expr::Call(CallExpr {
span,
callee,
args,
type_args,
}) => {
let callee = if let ExprOrSuper::Expr(expr) = callee {
let callee = if let Expr::Ident(ident) = *expr {
match Self::fold_ident(folder, top_level, ident) {
Ok(mut expr) => {
if let Expr::Member(member) = &mut expr {
if let ExprOrSuper::Expr(expr) = &mut member.obj {
if let Expr::Ident(ident) = expr.as_mut() {
member.obj = ExprOrSuper::Expr(Box::new(Expr::Paren(
ParenExpr {
expr: Box::new(Expr::Seq(SeqExpr {
span,
exprs: vec![
Box::new(0_f64.into()),
Box::new(ident.take().into()),
],
})),
span,
},
)))
}
}
};
expr
}
Err(ident) => Expr::Ident(ident),
}
} else {
*expr.fold_with(folder)
};
ExprOrSuper::Expr(Box::new(callee))
} else {
callee.fold_with(folder)
};
Expr::Call(CallExpr {
span,
callee,
args: args.fold_with(folder),
type_args,
})
}

Expr::Member(e) => {
Expand Down
4 changes: 2 additions & 2 deletions ecmascript/transforms/module/tests/amd.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ obj[bar('bas')] = '123'",
"define(['bar'], function(_bar) {
'use strict';
_bar = _interopRequireDefault(_bar);
obj[_bar.default('bas')] = '123';
obj[(0, _bar).default('bas')] = '123';
});"
);

Expand Down Expand Up @@ -835,7 +835,7 @@ define(["exports", "./evens"], function (_exports, _evens) {
_exports.isOdd = void 0;
function nextOdd(n) {
return _evens.isEven(n) ? n + 1 : n + 2;
return (0, _evens).isEven(n) ? n + 1 : n + 2;
}
var isOdd = function (isEven) {
Expand Down
Loading

0 comments on commit df2a926

Please sign in to comment.