From 9c8403cacd94eee30ab766c86e5550ae07572a8f Mon Sep 17 00:00:00 2001
From: Cameron Clark <cameron.clark@hey.com>
Date: Mon, 6 Jan 2025 18:47:39 +0000
Subject: [PATCH] feat(minifier): minify sequence expressions

---
 .../side_effects/check_for_state_change.rs    |  2 +
 .../ast_passes/peephole_remove_dead_code.rs   | 69 ++++++++++++++++++-
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/crates/oxc_ecmascript/src/side_effects/check_for_state_change.rs b/crates/oxc_ecmascript/src/side_effects/check_for_state_change.rs
index a1959d435ab06..76dabb0f1bfae 100644
--- a/crates/oxc_ecmascript/src/side_effects/check_for_state_change.rs
+++ b/crates/oxc_ecmascript/src/side_effects/check_for_state_change.rs
@@ -27,6 +27,7 @@ impl<'a> CheckForStateChange<'a, '_> for Expression<'a> {
             | Self::MetaProperty(_)
             | Self::ThisExpression(_)
             | Self::ClassExpression(_)
+            | Self::ArrowFunctionExpression(_)
             | Self::FunctionExpression(_) => false,
             Self::TemplateLiteral(template) => template
                 .expressions
@@ -73,6 +74,7 @@ impl<'a> CheckForStateChange<'a, '_> for Expression<'a> {
                     .iter()
                     .any(|element| element.check_for_state_change(check_for_new_objects))
             }
+
             _ => true,
         }
     }
diff --git a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs
index 29225a2a55bee..c69235c761b5a 100644
--- a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs
+++ b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs
@@ -1,6 +1,9 @@
 use oxc_allocator::Vec;
 use oxc_ast::{ast::*, Visit};
-use oxc_ecmascript::constant_evaluation::{ConstantEvaluation, IsLiteralValue};
+use oxc_ecmascript::{
+    constant_evaluation::{ConstantEvaluation, IsLiteralValue},
+    side_effects::MayHaveSideEffects,
+};
 use oxc_span::SPAN;
 use oxc_traverse::{traverse_mut_with_ctx, Ancestor, ReusableTraverseCtx, Traverse, TraverseCtx};
 
@@ -52,6 +55,9 @@ impl<'a> Traverse<'a> for PeepholeRemoveDeadCode {
         let ctx = Ctx(ctx);
         if let Some(folded_expr) = match expr {
             Expression::ConditionalExpression(e) => Self::try_fold_conditional_expression(e, ctx),
+            Expression::SequenceExpression(sequence_expression) => {
+                Self::try_fold_sequence_expression(sequence_expression, ctx)
+            }
             _ => None,
         } {
             *expr = folded_expr;
@@ -423,6 +429,57 @@ impl<'a, 'b> PeepholeRemoveDeadCode {
             None => None,
         }
     }
+
+    fn try_fold_sequence_expression(
+        sequence_expr: &mut SequenceExpression<'a>,
+        ctx: Ctx<'a, 'b>,
+    ) -> Option<Expression<'a>> {
+        let should_include_ret_val =
+            matches!(ctx.parent(), Ancestor::ExpressionStatementExpression(_));
+        let should_keep_as_sequence_expr =
+            matches!(ctx.parent(), Ancestor::CallExpressionCallee(_));
+
+        if should_keep_as_sequence_expr && sequence_expr.expressions.len() == 2 {
+            return None;
+        }
+
+        let (should_fold, new_len) = sequence_expr.expressions.iter().enumerate().fold(
+            (false, 0),
+            |(mut should_fold, mut new_len), (i, expr)| {
+                if expr.may_have_side_effects() || i == sequence_expr.expressions.len() - 1 {
+                    new_len += 1;
+                } else {
+                    should_fold = true;
+                }
+                (should_fold, new_len)
+            },
+        );
+
+        if should_fold {
+            let mut new_exprs = ctx.ast.vec_with_capacity(new_len);
+            let len = sequence_expr.expressions.len();
+            for (i, expr) in sequence_expr.expressions.iter_mut().enumerate() {
+                if expr.may_have_side_effects() || (should_include_ret_val && i == len - 1) {
+                    new_exprs.push(ctx.ast.move_expression(expr));
+                }
+            }
+
+            if should_keep_as_sequence_expr && new_exprs.len() == 1 {
+                new_exprs.insert(
+                    0,
+                    ctx.ast.expression_numeric_literal(SPAN, 1.0, None, NumberBase::Decimal),
+                );
+            }
+
+            if new_exprs.len() == 1 {
+                return Some(new_exprs.pop().unwrap());
+            }
+
+            return Some(ctx.ast.expression_sequence(sequence_expr.span, new_exprs));
+        }
+
+        None
+    }
 }
 
 /// <https://github.com/google/closure-compiler/blob/v20240609/test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java>
@@ -594,4 +651,14 @@ mod test {
         fold_same("(() => {})()");
         fold_same("(function () {})()");
     }
+
+    #[test]
+    fn test_fold_sequence_expr() {
+        fold("('foo', 'bar', 'baz')", "");
+        fold("('foo', 'bar', baz())", "baz()");
+        fold("('foo', bar(), baz())", "bar(), baz()");
+        fold("(() => {}, bar(), baz())", "bar(), baz()");
+        fold("(function k() {}, k(), baz())", "k(), baz()");
+        fold_same("(0, o.f)();");
+    }
 }