From e35990390b271fffca7e4e7a89971d82df0dc2e6 Mon Sep 17 00:00:00 2001 From: Austaras Date: Wed, 10 Aug 2022 16:26:33 +0800 Subject: [PATCH] fix(es/fixer): Remove needless parens for nested `??` (#5432) --- crates/swc_ecma_ast/src/operators.rs | 4 ++++ crates/swc_ecma_transforms_base/src/fixer.rs | 23 ++++++++++--------- .../src/simplify/branch/mod.rs | 2 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/crates/swc_ecma_ast/src/operators.rs b/crates/swc_ecma_ast/src/operators.rs index ea26b88c569..8d17a1ec587 100644 --- a/crates/swc_ecma_ast/src/operators.rs +++ b/crates/swc_ecma_ast/src/operators.rs @@ -104,6 +104,10 @@ impl BinaryOp { BinaryOp::NullishCoalescing => 1, } } + + pub fn may_short_circuit(&self) -> bool { + matches!(self, op!("??") | op!("||") | op!("&&")) + } } #[derive(StringEnum, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Hash, EqIgnoreSpan)] diff --git a/crates/swc_ecma_transforms_base/src/fixer.rs b/crates/swc_ecma_transforms_base/src/fixer.rs index 2cf7f9ff4fe..c76dcc20c09 100644 --- a/crates/swc_ecma_transforms_base/src/fixer.rs +++ b/crates/swc_ecma_transforms_base/src/fixer.rs @@ -255,14 +255,10 @@ impl VisitMut for Fixer<'_> { } Expr::Bin(BinExpr { op: op_of_rhs, .. }) => { if *op_of_rhs == expr.op { - match expr.op { - // `a && (b && c)` == `a && b && c` - // `a || (b || c)` == `a || b || c` - // `a ** (b ** c)` == `a ** b ** c` - op!("&&") | op!("||") | op!("**") => {} - _ => { - self.wrap(&mut expr.right); - } + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence#precedence_and_associativity + // `**` is the only right associative operator in js + if !(expr.op.may_short_circuit() || expr.op == op!("**")) { + self.wrap(&mut expr.right); } } else if op_of_rhs.precedence() <= expr.op.precedence() || (*op_of_rhs == op!("&&") && expr.op == op!("??")) @@ -274,7 +270,7 @@ impl VisitMut for Fixer<'_> { }; match &mut *expr.left { - Expr::Bin(BinExpr { op: op!("??"), .. }) => { + Expr::Bin(BinExpr { op: op!("??"), .. }) if expr.op != op!("??") => { self.wrap(&mut expr.left); } @@ -321,8 +317,11 @@ impl VisitMut for Fixer<'_> { } if let op!("??") = expr.op { - if let Expr::Bin(..) = &mut *expr.left { - self.wrap(&mut expr.left); + match &*expr.left { + Expr::Bin(BinExpr { op, .. }) if *op != op!("??") => { + self.wrap(&mut expr.left); + } + _ => (), } } } @@ -1641,5 +1640,7 @@ var store = global[SHARED] || (global[SHARED] = {}); " ); + identical!(issue_5417, "console.log(a ?? b ?? c)"); + identical!(bin_and_unary, "console.log(a++ && b--)"); } diff --git a/crates/swc_ecma_transforms_optimization/src/simplify/branch/mod.rs b/crates/swc_ecma_transforms_optimization/src/simplify/branch/mod.rs index fd9538fa952..d21a567cfd0 100644 --- a/crates/swc_ecma_transforms_optimization/src/simplify/branch/mod.rs +++ b/crates/swc_ecma_transforms_optimization/src/simplify/branch/mod.rs @@ -1430,7 +1430,7 @@ fn ignore_result(e: Expr, drop_str_lit: bool, ctx: &ExprCtx) -> Option { left, op, right, - }) if op != op!("&&") && op != op!("||") && op != op!("??") => { + }) if !op.may_short_circuit() => { let left = ignore_result(*left, true, ctx); let right = ignore_result(*right, true, ctx);