diff --git a/crates/oxc_minifier/src/peephole/fold_constants.rs b/crates/oxc_minifier/src/peephole/fold_constants.rs index 7b43b8e05e867..7e275ef31871b 100644 --- a/crates/oxc_minifier/src/peephole/fold_constants.rs +++ b/crates/oxc_minifier/src/peephole/fold_constants.rs @@ -6,7 +6,6 @@ use oxc_ecmascript::{ }; use oxc_span::GetSpan; use oxc_syntax::operator::{BinaryOperator, LogicalOperator}; -use oxc_traverse::Ancestor; use crate::ctx::Ctx; @@ -126,12 +125,22 @@ impl<'a> PeepholeOptimizations { if if lval { op.is_or() } else { op.is_and() } { return Some(ctx.ast.move_expression(&mut logical_expr.left)); } else if !left.may_have_side_effects(&ctx) { - let parent = ctx.ancestry.parent(); - // Bail `let o = { f() { assert.ok(this !== o); } }; (true && o.f)(); (true && o.f)``;` - if parent.is_tagged_template_expression() - || matches!(parent, Ancestor::CallExpressionCallee(_)) - { - return None; + let should_keep_indirect_access = + Self::should_keep_indirect_access(&logical_expr.right, ctx); + // (true && o.f) => (0, o.f) + if should_keep_indirect_access { + return Some(ctx.ast.expression_sequence( + logical_expr.span, + ctx.ast.vec_from_array([ + ctx.ast.expression_numeric_literal( + logical_expr.left.span(), + 0.0, + None, + NumberBase::Decimal, + ), + ctx.ast.move_expression(&mut logical_expr.right), + ]), + )); } // (FALSE || x) => x // (TRUE && x) => x @@ -191,6 +200,23 @@ impl<'a> PeepholeOptimizations { ]); ctx.ast.expression_sequence(logical_expr.span, expressions) } else { + let should_keep_indirect_access = + Self::should_keep_indirect_access(&logical_expr.right, ctx); + // (null ?? o.f) => (0, o.f) + if should_keep_indirect_access { + return Some(ctx.ast.expression_sequence( + logical_expr.span, + ctx.ast.vec_from_array([ + ctx.ast.expression_numeric_literal( + logical_expr.left.span(), + 0.0, + None, + NumberBase::Decimal, + ), + ctx.ast.move_expression(&mut logical_expr.right), + ]), + )); + } // nullish condition => this expression evaluates to the right side. ctx.ast.move_expression(&mut logical_expr.right) }) @@ -200,6 +226,23 @@ impl<'a> PeepholeOptimizations { | ValueType::String | ValueType::Boolean | ValueType::Object => { + let should_keep_indirect_access = + Self::should_keep_indirect_access(&logical_expr.left, ctx); + // (o.f ?? something) => (0, o.f) + if should_keep_indirect_access { + return Some(ctx.ast.expression_sequence( + logical_expr.span, + ctx.ast.vec_from_array([ + ctx.ast.expression_numeric_literal( + logical_expr.right.span(), + 0.0, + None, + NumberBase::Decimal, + ), + ctx.ast.move_expression(&mut logical_expr.left), + ]), + )); + } // non-nullish condition => this expression evaluates to the left side. Some(ctx.ast.move_expression(&mut logical_expr.left)) } @@ -1274,6 +1317,11 @@ mod test { // 1 || 0 == 1, but true =/= 1 fold("x = foo() && true || bar()", "x = foo() && !0 || bar()"); fold("foo() && true || bar()", "foo() && !0 || bar()"); + + test("var y; x = (true && y)()", "var y; x = y()"); + test("var y; x = (true && y.z)()", "var y; x = (0, y.z)()"); + test("var y; x = (false || y)()", "var y; x = y()"); + test("var y; x = (false || y.z)()", "var y; x = (0, y.z)()"); } #[test] @@ -1315,6 +1363,11 @@ mod test { fold("a() ?? (1 ?? b())", "a() ?? 1"); fold("(a() ?? 1) ?? b()", "a() ?? 1 ?? b()"); + + test_same("var y; x = (y ?? 1)()"); // can compress to "var y; x = y()" if y is not null or undefined + test_same("var y; x = (y.z ?? 1)()"); // "var y; x = (0, y.z)()" if y is not null or undefined + test("var y; x = (null ?? y)()", "var y; x = y()"); + test("var y; x = (null ?? y.z)()", "var y; x = (0, y.z)()"); } #[test] diff --git a/crates/oxc_minifier/src/peephole/remove_dead_code.rs b/crates/oxc_minifier/src/peephole/remove_dead_code.rs index e46e163dedad7..fb17d256d0ce9 100644 --- a/crates/oxc_minifier/src/peephole/remove_dead_code.rs +++ b/crates/oxc_minifier/src/peephole/remove_dead_code.rs @@ -379,14 +379,6 @@ impl<'a, 'b> PeepholeOptimizations { expr: &mut ConditionalExpression<'a>, ctx: Ctx<'a, 'b>, ) -> Option> { - // Bail `let o = { f() { assert.ok(this !== o); } }; (true ? o.f : false)(); (true ? o.f : false)``;` - let parent = ctx.ancestry.parent(); - if parent.is_tagged_template_expression() - || matches!(parent, Ancestor::CallExpressionCallee(_)) - { - return None; - } - expr.test.evaluate_value_to_boolean(&ctx).map(|v| { if expr.test.may_have_side_effects(&ctx) { // "(a, true) ? b : c" => "a, b" @@ -404,7 +396,31 @@ impl<'a, 'b> PeepholeOptimizations { ]); ctx.ast.expression_sequence(expr.span, exprs) } else { - ctx.ast.move_expression(if v { &mut expr.consequent } else { &mut expr.alternate }) + let result_expr = ctx.ast.move_expression(if v { + &mut expr.consequent + } else { + &mut expr.alternate + }); + + let should_keep_as_sequence_expr = + Self::should_keep_indirect_access(&result_expr, ctx); + // "(1 ? a.b : 0)()" => "(0, a.b)()" + if should_keep_as_sequence_expr { + ctx.ast.expression_sequence( + expr.span, + ctx.ast.vec_from_iter([ + ctx.ast.expression_numeric_literal( + expr.span, + 0.0, + None, + NumberBase::Decimal, + ), + result_expr, + ]), + ) + } else { + result_expr + } } }) } @@ -414,10 +430,10 @@ impl<'a, 'b> PeepholeOptimizations { sequence_expr: &mut SequenceExpression<'a>, ctx: Ctx<'a, 'b>, ) -> Option> { - let should_keep_as_sequence_expr = matches!( - ctx.parent(), - Ancestor::CallExpressionCallee(_) | Ancestor::TaggedTemplateExpressionTag(_) - ); + let should_keep_as_sequence_expr = sequence_expr + .expressions + .last() + .is_some_and(|last_expr| Self::should_keep_indirect_access(last_expr, ctx)); if should_keep_as_sequence_expr && sequence_expr.expressions.len() == 2 && sequence_expr.expressions.first().unwrap().is_number_0() @@ -455,6 +471,22 @@ impl<'a, 'b> PeepholeOptimizations { } None } + + /// Whether the indirect access should be kept. + /// For example, `(0, foo.bar)()` should not be transformed to `foo.bar()`. + /// Example case: `let o = { f() { assert.ok(this !== o); } }; (true && o.f)(); (true && o.f)``;` + /// + /// * `access_value` - The expression that may need to be kept as indirect reference (`foo.bar` in the example above) + pub fn should_keep_indirect_access(access_value: &Expression<'a>, ctx: Ctx<'a, 'b>) -> bool { + matches!( + ctx.parent(), + Ancestor::CallExpressionCallee(_) | Ancestor::TaggedTemplateExpressionTag(_) + ) && match access_value { + Expression::Identifier(id) => id.name == "eval" && ctx.is_global_reference(id), + match_member_expression!(Expression) => true, + _ => false, + } + } } impl<'a> LatePeepholeOptimizations { @@ -604,6 +636,11 @@ mod test { test("false ? foo() : bar()", "bar()"); test_same("foo() ? bar() : baz()"); test("foo && false ? foo() : bar()", "(foo, bar());"); + + test("var a; (true ? a : 0)()", "var a; a()"); + test("var a; (true ? a.b : 0)()", "var a; (0, a.b)()"); + test("var a; (false ? 0 : a)()", "var a; a()"); + test("var a; (false ? 0 : a.b)()", "var a; (0, a.b)()"); } #[test] @@ -635,10 +672,20 @@ mod test { #[test] fn remove_unused_expressions_in_sequence() { test("true, foo();", "foo();"); - test("(true, foo)();", "(0, foo)();"); - test("(true, true, foo)();", "(0, foo)();"); - test("var foo; (true, foo)();", "var foo; (0, foo)();"); - test("var foo; (true, true, foo)();", "var foo; (0, foo)();"); + test("(0, foo)();", "foo();"); + test("(0, foo)``;", "foo``;"); + test("(0, foo)?.();", "foo?.();"); + test_same("(0, eval)();"); // this can be compressed to `eval?.()` + test_same("(0, eval)``;"); // this can be compressed to `eval?.()` + test_same("(0, eval)?.();"); // this can be compressed to `eval?.()` + test("var eval; (0, eval)();", "var eval; eval();"); + test_same("(0, foo.bar)();"); + test_same("(0, foo.bar)``;"); + test_same("(0, foo.bar)?.();"); + test("(true, foo.bar)();", "(0, foo.bar)();"); + test("(true, true, foo.bar)();", "(0, foo.bar)();"); + test("var foo; (true, foo.bar)();", "var foo; (0, foo.bar)();"); + test("var foo; (true, true, foo.bar)();", "var foo; (0, foo.bar)();"); } #[test] diff --git a/crates/oxc_minifier/tests/peephole/esbuild.rs b/crates/oxc_minifier/tests/peephole/esbuild.rs index 12a6316c69c23..1b58da32a697c 100644 --- a/crates/oxc_minifier/tests/peephole/esbuild.rs +++ b/crates/oxc_minifier/tests/peephole/esbuild.rs @@ -1281,18 +1281,18 @@ fn test_flatten_values() { test("tag`a${x}b${'y'}c`", "tag`a${x}b${'y'}c`;"); test("tag`a${'x'}b${y}c`", "tag`a${'x'}b${y}c`;"); test("tag`a${'x'}b${'y'}c`", "tag`a${'x'}b${'y'}c`;"); - // test("(1, x)``", "x``;"); + test("(1, x)``", "x``;"); test("(1, x.y)``", "(0, x.y)``;"); test("(1, x[y])``", "(0, x[y])``;"); - // test("(true && x)``", "x``;"); - // test("(true && x.y)``", "(0, x.y)``;"); - // test("(true && x[y])``", "(0, x[y])``;"); - // test("(false || x)``", "x``;"); - // test("(false || x.y)``", "(0, x.y)``;"); - // test("(false || x[y])``", "(0, x[y])``;"); + test("(true && x)``", "x``;"); + test("(true && x.y)``", "(0, x.y)``;"); + test("(true && x[y])``", "(0, x[y])``;"); + test("(false || x)``", "x``;"); + test("(false || x.y)``", "(0, x.y)``;"); + test("(false || x[y])``", "(0, x[y])``;"); test("(null ?? x)``", "x``;"); - // test("(null ?? x.y)``", "(0, x.y)``;"); - // test("(null ?? x[y])``", "(0, x[y])``;"); + test("(null ?? x.y)``", "(0, x.y)``;"); + test("(null ?? x[y])``", "(0, x[y])``;"); // test("function f(a) { let c = a.b; return c`` }", "function f(a) { return (0, a.b)``;}"); // test( // "function f(a) { let c = a.b; return c`${x}` }", @@ -1525,26 +1525,26 @@ fn test_remove_dead_expr() { test("let x = (y, 2)", "let x = (y, 2);"); test("let x = (/* @__PURE__ */ foo(bar), 2)", "let x = (bar, 2);"); test("let x = (2, y)", "let x = y;"); - // test("let x = (2, y)()", "let x = y();"); - // test("let x = (true && y)()", "let x = y();"); - // test("let x = (false || y)()", "let x = y();"); + test("let x = (2, y)()", "let x = y();"); + test("let x = (true && y)()", "let x = y();"); + test("let x = (false || y)()", "let x = y();"); test("let x = (null ?? y)()", "let x = y();"); - // test("let x = (1 ? y : 2)()", "let x = y();"); - // test("let x = (0 ? 1 : y)()", "let x = y();"); + test("let x = (1 ? y : 2)()", "let x = y();"); + test("let x = (0 ? 1 : y)()", "let x = y();"); test("let x = (2, y.z)", "let x = y.z;"); test("let x = (2, y.z)()", "let x = (0, y.z)();"); - // test("let x = (true && y.z)()", "let x = (0, y.z)();"); - // test("let x = (false || y.z)()", "let x = (0, y.z)();"); - // test("let x = (null ?? y.z)()", "let x = (0, y.z)();"); - // test("let x = (1 ? y.z : 2)()", "let x = (0, y.z)();"); - // test("let x = (0 ? 1 : y.z)()", "let x = (0, y.z)();"); + test("let x = (true && y.z)()", "let x = (0, y.z)();"); + test("let x = (false || y.z)()", "let x = (0, y.z)();"); + test("let x = (null ?? y.z)()", "let x = (0, y.z)();"); + test("let x = (1 ? y.z : 2)()", "let x = (0, y.z)();"); + test("let x = (0 ? 1 : y.z)()", "let x = (0, y.z)();"); test("let x = (2, y[z])", "let x = y[z];"); - // test("let x = (2, y[z])()", "let x = (0, y[z])();"); - // test("let x = (true && y[z])()", "let x = (0, y[z])();"); - // test("let x = (false || y[z])()", "let x = (0, y[z])();"); - // test("let x = (null ?? y[z])()", "let x = (0, y[z])();"); - // test("let x = (1 ? y[z] : 2)()", "let x = (0, y[z])();"); - // test("let x = (0 ? 1 : y[z])()", "let x = (0, y[z])();"); + test("let x = (2, y[z])()", "let x = (0, y[z])();"); + test("let x = (true && y[z])()", "let x = (0, y[z])();"); + test("let x = (false || y[z])()", "let x = (0, y[z])();"); + test("let x = (null ?? y[z])()", "let x = (0, y[z])();"); + test("let x = (1 ? y[z] : 2)()", "let x = (0, y[z])();"); + test("let x = (0 ? 1 : y[z])()", "let x = (0, y[z])();"); test("delete (x)", "delete x;"); test("delete (x); var x", "delete x;var x;"); test("delete (x.y)", "delete x.y;"); diff --git a/crates/oxc_minifier/tests/peephole/mod.rs b/crates/oxc_minifier/tests/peephole/mod.rs index b6283a7f0fa1c..c9233848d3e17 100644 --- a/crates/oxc_minifier/tests/peephole/mod.rs +++ b/crates/oxc_minifier/tests/peephole/mod.rs @@ -76,10 +76,10 @@ fn fold() { fn tagged_template() { test("(1, o.f)()", "(0, o.f)()"); test("(1, o.f)``", "(0, o.f)``"); - test_same("(!0 && o.f)()"); - test_same("(!0 && o.f)``"); - test("(!0 ? o.f : !1)()", "(0 ? !1: o.f)()"); - test("(!0 ? o.f : !1)``", "(0 ? !1: o.f)``"); + test("(!0 && o.f)()", "(0, o.f)()"); + test("(!0 && o.f)``", "(0, o.f)``"); + test("(!0 ? o.f : !1)()", "(0, o.f)()"); + test("(!0 ? o.f : !1)``", "(0, o.f)``"); test("foo(true && o.f)", "foo(o.f)"); test("foo(true ? o.f : false)", "foo(o.f)"); @@ -88,9 +88,9 @@ fn tagged_template() { #[test] fn eval() { // Keep indirect-eval syntaxes - test_same("(!0 && eval)(x)"); - test_same("(1 ? eval : 2)(x)"); - test_same("(1 ? eval : 2)?.(x)"); + test("(!0 && eval)(x)", "(0, eval)(x)"); + test("(1 ? eval : 2)(x)", "(0, eval)(x)"); + test("(1 ? eval : 2)?.(x)", "(0, eval)?.(x)"); test("(1, eval)(x)", "(0, eval)(x)"); test("(1, eval)?.(x)", "(0, eval)?.(x)"); test("(3, eval)(x)", "(0, eval)(x)");