feat(minifier): merge adjucent if stmt with the same content (#26445)

Merge adjacent terminated `if` statements with same content

```js
if (a) { b; return d } if (c) { b; return d }
if (a || c) return b, d;
```

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Cameron Clark <cameron.clark@hey.com>

crates/oxc_minifier/src/peephole/minimize_statements.rs

@@ -228,13 +228,6 @@ impl<'a> PeepholeOptimizations {
         Expression::new_sequence_expression(span, exprs, ctx)
     }
 
-    fn jump_stmts_look_the_same(left: &Statement<'a>, right: &Statement<'a>) -> bool {
-        if left.is_jump_statement() && right.is_jump_statement() {
-            return left.content_eq(right);
-        }
-        false
-    }
-
     /// For variable declarations:
     /// * merge with the previous variable declarator if their kinds are the same
     /// * remove the variable declarator if it is unused
@@ -564,82 +557,77 @@ impl<'a> PeepholeOptimizations {
         // `a; if (b) c;` => `if (a, b) c;`
         Self::merge_last_expression_into_sequence(&mut if_stmt.test, result, ctx);
 
-        // Absorb a previous expression statement
         if ctx.options().sequences {
-            if if_stmt.consequent.is_jump_statement() {
-                // Absorb a previous if statement
-                if let Some(Statement::IfStatement(prev_if_stmt)) = result.last_mut()
-                    && prev_if_stmt.alternate.is_none()
-                    && Self::jump_stmts_look_the_same(&prev_if_stmt.consequent, &if_stmt.consequent)
-                {
-                    // "if (a) break c; if (b) break c;" => "if (a || b) break c;"
-                    // "if (a) continue c; if (b) continue c;" => "if (a || b) continue c;"
-                    // "if (a) return c; if (b) return c;" => "if (a || b) return c;"
-                    // "if (a) throw c; if (b) throw c;" => "if (a || b) throw c;"
-                    let previous = result.pop().unwrap();
-                    let Statement::IfStatement(previous) = previous else { unreachable!() };
-                    let previous = previous.unbox();
-                    let span = if_stmt.test.span();
+            if let Some(Statement::IfStatement(prev_if_stmt)) = result.last_mut()
+                && prev_if_stmt.alternate.is_none()
+                && if_stmt.consequent.is_terminated()
+                && prev_if_stmt.consequent.content_eq(&if_stmt.consequent)
+            {
+                // Merge previous stmt if its terminated and both have same content
+                // `if (a) JUMP; if (b) JUMP;` => `if (a || b) JUMP;`
+                // `if (a) { b; JUMP }; if (b) { b; JUMP };` => `if (a || b) { b; JUMP };`
+                let previous = result.pop().unwrap();
+                let Statement::IfStatement(previous) = previous else { unreachable!() };
+                let previous = previous.unbox();
+                let span = if_stmt.test.span();
+                ctx.replace_expression_with(&mut if_stmt.test, |test, ctx| {
+                    Self::join_with_left_associative_op(
+                        span,
+                        LogicalOperator::Or,
+                        previous.test,
+                        test,
+                        ctx,
+                    )
+                });
+                ctx.drop_statement(&previous.consequent);
+            }
+
+            if if_stmt.consequent.is_jump_statement()
+                && Self::can_remove_termination_statement(&if_stmt.consequent, ctx)
+            {
+                // Don't do this transformation if the branch condition could
+                // potentially access symbols declared later on on this scope below.
+                // If so, inverting the branch condition and nesting statements after
+                // this in a block would break that access which is a behavior change.
+                //
+                //   // This transformation is incorrect
+                //   if (a()) return; function a() {}
+                //   if (!a()) { function a() {} }
+                //
+                //   // This transformation is incorrect
+                //   if (a(() => b)) return; let b;
+                //   if (a(() => b)) { let b; }
+                //
+                let can_move_branch_condition_outside_scope =
+                    !if_stmt.alternate.as_ref().is_some_and(Self::statement_cares_about_scope)
+                        && !stmts.as_slice().iter().any(Self::statement_cares_about_scope);
+
+                if can_move_branch_condition_outside_scope {
+                    let drained_stmts = stmts.by_ref();
+                    let mut body = if let Some(alternate) = if_stmt.alternate.take() {
+                        ArenaVec::from_iter_in(iter::once(alternate).chain(drained_stmts), ctx)
+                    } else {
+                        ArenaVec::from_iter_in(drained_stmts, ctx)
+                    };
+
+                    Self::minimize_statements(&mut body, ctx);
+                    let span =
+                        if body.is_empty() { if_stmt.consequent.span() } else { body[0].span() };
                     ctx.replace_expression_with(&mut if_stmt.test, |test, ctx| {
-                        Self::join_with_left_associative_op(
-                            span,
-                            LogicalOperator::Or,
-                            previous.test,
-                            test,
-                            ctx,
-                        )
+                        Self::minimize_not(test.span(), test, ctx, true)
                     });
-                    ctx.drop_statement(&previous.consequent);
-                }
-
-                if Self::can_remove_termination_statement(&if_stmt.consequent, ctx) {
-                    // Don't do this transformation if the branch condition could
-                    // potentially access symbols declared later on on this scope below.
-                    // If so, inverting the branch condition and nesting statements after
-                    // this in a block would break that access which is a behavior change.
-                    //
-                    //   // This transformation is incorrect
-                    //   if (a()) return; function a() {}
-                    //   if (!a()) { function a() {} }
-                    //
-                    //   // This transformation is incorrect
-                    //   if (a(() => b)) return; let b;
-                    //   if (a(() => b)) { let b; }
-                    //
-                    let can_move_branch_condition_outside_scope =
-                        !if_stmt.alternate.as_ref().is_some_and(Self::statement_cares_about_scope)
-                            && !stmts.as_slice().iter().any(Self::statement_cares_about_scope);
-
-                    if can_move_branch_condition_outside_scope {
-                        let drained_stmts = stmts.by_ref();
-                        let mut body = if let Some(alternate) = if_stmt.alternate.take() {
-                            ArenaVec::from_iter_in(iter::once(alternate).chain(drained_stmts), ctx)
-                        } else {
-                            ArenaVec::from_iter_in(drained_stmts, ctx)
-                        };
-
-                        Self::minimize_statements(&mut body, ctx);
-                        let span = if body.is_empty() {
-                            if_stmt.consequent.span()
-                        } else {
-                            body[0].span()
-                        };
-                        ctx.replace_expression_with(&mut if_stmt.test, |test, ctx| {
-                            Self::minimize_not(test.span(), test, ctx, true)
-                        });
-                        let consequent = if body.len() == 1 {
-                            body.remove(0)
-                        } else {
-                            let scope_id = ctx.create_child_scope_of_current(ScopeFlags::empty());
-                            Statement::new_block_statement_with_scope_id(span, body, scope_id, ctx)
-                        };
-                        ctx.replace_statement(&mut if_stmt.consequent, consequent);
-                        let mut if_stmt = Statement::IfStatement(if_stmt);
-                        Self::try_minimize_if(&mut if_stmt, ctx);
-                        ctx.notice_change();
-                        Self::minimize_statement(if_stmt, stmts, result, ctx);
-                        return;
-                    }
+                    let consequent = if body.len() == 1 {
+                        body.remove(0)
+                    } else {
+                        let scope_id = ctx.create_child_scope_of_current(ScopeFlags::empty());
+                        Statement::new_block_statement_with_scope_id(span, body, scope_id, ctx)
+                    };
+                    ctx.replace_statement(&mut if_stmt.consequent, consequent);
+                    let mut if_stmt = Statement::IfStatement(if_stmt);
+                    Self::try_minimize_if(&mut if_stmt, ctx);
+                    ctx.notice_change();
+                    Self::minimize_statement(if_stmt, stmts, result, ctx);
+                    return;
                 }
             }
 

crates/oxc_minifier/tests/peephole/minimize_statements.rs

@@ -200,6 +200,29 @@ fn test_merge_adjacent_ifs_with_shorthand_object_property() {
     );
 }
 
+#[test]
+fn test_merge_adjacent_ifs_with_same_content() {
+    test("if (a) { b; return d } if (c) { b; return d }", "if (a || c) return b, d;");
+    test(
+        "if (a) if (x) return b; else throw c; if (d) if (x) return b; else throw c;",
+        "if (a || d) { if (x) return b; throw c; }",
+    );
+    test(
+        "if (a) { if (x) return b; else return c; } if (d) { if (x) return b; else return c; }",
+        "if (a || d) return x ? b : c;",
+    );
+    test(
+        "if (a) { return c; var z; } if (d) { return c; var z; }",
+        "if (a || d) { return c; var z; }",
+    );
+    test("if (a) { f; return b } if (c) { f; b }", "if (a) return f, b; c && (f, b);");
+    test_same("if (a) { for(;;)f; return b } if (c) { for(;;)f; throw b }");
+    test(
+        "if (a) { b; return d } if (c) { b; return d } if (e) { b; return d }",
+        "if (a || c || e) return b, d;",
+    );
+}
+
 // `{ __proto__: __proto__ }` sets `[[Prototype]]` via the Annex B.3.1 proto
 // setter, while `{ __proto__ }` is a plain shorthand that creates a regular
 // own data property. Normalising the former into the latter would change

tasks/minsize/minsize.snap

@@ -19,9 +19,9 @@ Original   | minified   | minified   | gzip       | gzip       | Iterations | Fi
 
 2.14 MB    | 707.63 kB  | 724.14 kB  | 160.08 kB  | 181.07 kB  |          2 | victory.js
 
-3.20 MB    | 958.86 kB  | 1.01 MB    | 317.06 kB  | 331.56 kB  |          2 | echarts.js
+3.20 MB    | 958.82 kB  | 1.01 MB    | 317.06 kB  | 331.56 kB  |          2 | echarts.js
 
-6.69 MB    | 2.12 MB    | 2.31 MB    | 453.42 kB  | 488.28 kB  |          4 | antd.js
+6.69 MB    | 2.12 MB    | 2.31 MB    | 453.41 kB  | 488.28 kB  |          4 | antd.js
 
 10.95 MB   | 3.31 MB    | 3.49 MB    | 849.97 kB  | 915.50 kB  |          4 | typescript.js
 

tasks/track_memory_allocations/allocs_minifier.yaml

@@ -49,9 +49,9 @@ antd.js:
   sys deallocs: 860
   sys alloc bytes: 29973448 # 29.97 MB
   sys peak growth: 19934725 # 19.93 MB
-  arena allocs: 181463
+  arena allocs: 181457
   arena reallocs: 34406
-  arena size: 43039480 # 43.04 MB
+  arena size: 43039192 # 43.04 MB
 
 binder.ts:
   file size: 193077 # 193.08 kB