From 32157787aa2db8721ed4606dc4aee91f02a4e92b Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 04:54:14 +0100 Subject: [PATCH 01/25] feat(minifier): minimize switch statements --- .../src/peephole/minimize_conditions.rs | 4 +- .../src/peephole/minimize_switch_statement.rs | 415 ++++++++++++++++++ crates/oxc_minifier/src/peephole/mod.rs | 4 + crates/oxc_minifier/tests/peephole/esbuild.rs | 6 +- .../tests/peephole/obscure_edge_cases.rs | 5 +- .../tests/peephole/statement_fusion.rs | 4 +- tasks/minsize/minsize.snap | 8 +- .../allocs_minifier.snap | 4 +- 8 files changed, 436 insertions(+), 14 deletions(-) create mode 100644 crates/oxc_minifier/src/peephole/minimize_switch_statement.rs diff --git a/crates/oxc_minifier/src/peephole/minimize_conditions.rs b/crates/oxc_minifier/src/peephole/minimize_conditions.rs index 26ff09e6658af..532c584d898ce 100644 --- a/crates/oxc_minifier/src/peephole/minimize_conditions.rs +++ b/crates/oxc_minifier/src/peephole/minimize_conditions.rs @@ -293,7 +293,7 @@ mod test { test_same("function f(){try{foo()}catch(e){bar(e)}finally{baz()}}"); // Try it out with switch statements - test_same("function f(){switch(x){case 1:break}}"); + test("function f(){switch(x){case 1:break}}", "function f(){x;}"); // Do while loops stay in a block if that's where they started test( @@ -335,7 +335,7 @@ mod test { ); test_same("function f(){foo()}"); - test_same("switch(x){case y: foo()}"); + test("switch(x){case y: foo()}", "x === y && foo()"); test( "try{foo()}catch(ex){bar()}finally{baz()}", "try { foo(); } catch { bar(); } finally { baz(); }", diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs new file mode 100644 index 0000000000000..1bb28a60cbd1d --- /dev/null +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -0,0 +1,415 @@ +use super::PeepholeOptimizations; +use crate::ctx::Ctx; +use oxc_allocator::{TakeIn, Vec}; +use oxc_ast::ast::{Statement, SwitchCase}; +use oxc_ast_visit::Visit; +use oxc_ecmascript::side_effects::MayHaveSideEffects; +use oxc_span::{GetSpan, SPAN}; +use oxc_syntax::{operator::BinaryOperator, scope::ScopeFlags}; + +impl<'a> PeepholeOptimizations { + pub fn try_minimize_switch(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + Self::collapse_empty_switch_cases(stmt, ctx); + Self::remove_empty_switch(stmt, ctx); + Self::fold_switch_with_one_case(stmt, ctx); + Self::fold_switch_with_two_cases(stmt, ctx); + } + + fn find_trailing_removable_switch_cases_range( + cases: &Vec<'_, SwitchCase<'a>>, + ctx: &Ctx<'a, '_>, + ) -> Option<(usize, usize)> { + let case_count = cases.len(); + if case_count <= 1 { + return None; + } + + // Position of the (last) default if any + let Some(default_pos) = cases.iter().rposition(|c| c.is_default_case()) else { + return None; + }; + + // Find the last non-removable case (any case whose consequent is non-empty). + let last_non_empty_before_last = cases[..case_count - 1].iter().rposition(|c| { + !c.consequent.is_empty() + || c.test.as_ref().is_none_or(|test| test.may_have_side_effects(ctx)) + }); + + // start is the first index of the removable suffix + let start = match last_non_empty_before_last { + Some(pos) => pos + 1, + None => 0, + }; + + // nothing removable + if start >= case_count - 1 { + return None; + } + + // Reject only when a non-empty default lies inside the removable suffix and it is not the last case. + if default_pos >= start + && default_pos != case_count - 1 + && !&cases[default_pos].consequent.is_empty() + { + return None; + } + Some((start, case_count)) + } + + fn collapse_empty_switch_cases(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + + let Some((start, mut end)) = + Self::find_trailing_removable_switch_cases_range(&switch_stmt.cases, ctx) + else { + return; + }; + + if let Some(last) = switch_stmt.cases.last_mut() { + if !Self::is_empty_switch_case(&last.consequent) { + last.test = None; + end -= 1; + } + } + switch_stmt.cases.drain(start..end); + ctx.state.changed = true; + } + + fn remove_empty_switch(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + if switch_stmt.cases.is_empty() { + if !switch_stmt.discriminant.may_have_side_effects(ctx) { + *stmt = ctx.ast.statement_empty(switch_stmt.span); + } else { + *stmt = ctx.ast.statement_expression( + switch_stmt.span, + switch_stmt.discriminant.take_in(ctx.ast), + ); + } + ctx.state.changed = true; + } + } + + fn fold_switch_with_two_cases(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + if switch_stmt.cases.len() == 2 { + // check whatever its default + case + if switch_stmt.cases[0].test.is_some() == switch_stmt.cases[1].test.is_some() + || !Self::can_case_be_inlined(&switch_stmt.cases[0]) + || !Self::can_case_be_inlined(&switch_stmt.cases[1]) + { + return; + } + + let mut first = switch_stmt.cases.pop().unwrap(); + let mut second = switch_stmt.cases.pop().unwrap(); + Self::drop_break_and_postfix(&mut first.consequent); + Self::drop_break_and_postfix(&mut second.consequent); + + let (test, consequent, alternate) = if first.test.is_some() { + (first.test.unwrap(), first.consequent, second.consequent) + } else { + (second.test.unwrap(), second.consequent, first.consequent) + }; + + *stmt = ctx.ast.statement_if( + switch_stmt.span, + ctx.ast.expression_binary( + SPAN, + switch_stmt.discriminant.take_in(ctx.ast), + BinaryOperator::StrictEquality, + test, + ), + Self::create_if_block_from_switch_case(consequent, ctx), + Some(Self::create_if_block_from_switch_case(alternate, ctx)), + ); + } + } + + fn create_if_block_from_switch_case( + mut vec: Vec<'a, Statement<'a>>, + ctx: &mut Ctx<'a, '_>, + ) -> Statement<'a> { + if vec.len() == 1 && matches!(vec.get(0), Some(Statement::BlockStatement(_))) { + vec.pop().unwrap() + } else { + ctx.ast.statement_block_with_scope_id( + SPAN, + vec, + ctx.create_child_scope_of_current(ScopeFlags::empty()), + ) + } + } + + fn fold_switch_with_one_case(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + if switch_stmt.cases.len() == 1 { + let Some(first_case) = switch_stmt.cases.get(0) else { return }; + if !Self::can_case_be_inlined(&first_case) { + return; + } + let Some(mut case) = switch_stmt.cases.pop() else { + return; + }; + + ctx.state.changed = true; + let discriminant = switch_stmt.discriminant.take_in(ctx.ast); + Self::drop_break_and_postfix(&mut case.consequent); + + if let Some(test) = case.test { + *stmt = ctx.ast.statement_if( + switch_stmt.span, + ctx.ast.expression_binary( + SPAN, + discriminant, + BinaryOperator::StrictEquality, + test, + ), + Self::create_if_block_from_switch_case(case.consequent, ctx), + None, + ); + } else { + let mut stmts = ctx.ast.vec(); + if discriminant.may_have_side_effects(ctx) { + stmts.push(ctx.ast.statement_expression(discriminant.span(), discriminant)); + } + if case.consequent.len() > 0 { + stmts.extend(case.consequent); + } + *stmt = ctx.ast.statement_block_with_scope_id( + switch_stmt.span, + stmts, + ctx.create_child_scope_of_current(ScopeFlags::empty()), + ) + } + } + } + + pub fn drop_break_and_postfix(cons: &mut Vec<'a, Statement<'a>>) { + // TODO: this is wrong, as it doesn't take into account BlockStatement + if cons.len() > 0 + && let Some(terminates_rpos) = cons.iter().rposition(Self::is_terminated) + { + if cons + .get(terminates_rpos) + .is_some_and(|stmt| matches!(stmt, Statement::BreakStatement(_))) + { + cons.truncate(terminates_rpos); + } else { + cons.truncate(terminates_rpos + 1); + } + } + } + + fn is_empty_switch_case(stmt: &Vec) -> bool { + if stmt.len() != 1 { + return stmt.is_empty(); + } + match stmt.last() { + Some(Statement::BlockStatement(block_stmt)) => block_stmt.body.is_empty(), + _ => false, + } + } + + pub fn is_terminated(stmt: &Statement<'a>) -> bool { + match stmt { + Statement::BreakStatement(break_stmt) => break_stmt.label.is_none(), + Statement::BlockStatement(block_stmt) => { + block_stmt.body.last().is_some_and(Self::is_terminated) + } + Statement::ContinueStatement(_) + | Statement::ReturnStatement(_) + | Statement::ThrowStatement(_) => true, + _ => false, + } + } + + pub fn can_case_be_inlined(case: &SwitchCase) -> bool { + let mut break_finder = BreakFinder::new(); + break_finder.visit_switch_case(case); + !break_finder.nested_unlabelled_break + } +} + +struct BreakFinder { + top_level: bool, + nested_unlabelled_break: bool, +} + +impl<'a> BreakFinder { + pub fn new() -> Self { + Self { top_level: true, nested_unlabelled_break: false } + } +} + +impl<'a> Visit<'a> for BreakFinder { + fn visit_statement(&mut self, it: &Statement<'a>) { + // Only visit blocks where vars could be hoisted + match it { + Statement::BlockStatement(it) => self.visit_block_statement(it), + Statement::BreakStatement(it) => { + if !self.top_level && it.label.is_none() { + self.nested_unlabelled_break = true; + } + } + Statement::IfStatement(it) => { + if self.top_level { + self.top_level = false; + self.visit_if_statement(it); + self.top_level = true; + } else { + self.visit_if_statement(it); + } + } + Statement::TryStatement(it) => self.visit_try_statement(it), + Statement::WithStatement(it) => self.visit_with_statement(it), + _ => {} + } + } +} + +#[cfg(test)] +mod test { + use crate::tester::{test, test_same}; + + #[test] + fn minimize_switch() { + // remove empty + test("switch (empty) {}", "empty"); + test("switch (!true) {}", ""); + test("switch (true) {}", ""); + test("switch (fn()) {}", "fn()"); + // truncate cases + test("switch (test) { default: break; case 1: break; }", "if (test === 1) {} else {}"); + test("switch (test) { case 1: break; }", "test;"); + test_same("switch (test) { case 2: case 1: break; default: break }"); + test_same("switch (test) { case 3: foo(); case 2: case 1: break; default: break }"); + // single switch case + test("switch (fn()) { default: a(); break; }", "fn(),a()"); + test("switch (test) { default: fn(); return; }", "test, fn(); return"); + test("switch (fn()) { default: a(); }", "fn(), a()"); + test("switch (test) { case 1: fn(); break; }", "test === 1 && fn()"); + test("switch (test) { case 1: fn(); return; }", "if (test === 1) { fn(); return; }"); + test("switch (test) { case 1: fn(); }", "test === 1 && fn()"); + test("switch (a) { case log(1): default:}", "if (a === log(1)) {} else {}"); + // default + case + test( + "switch (c) { default: a(); break; case 1: b(); break; }", + "if (c === 1) { b(); } else { a(); }", + ); + test( + "switch (c) { default: { a(); break; } case 1: { b(); break; } }", + "c === 1 ? b() : a()", + ); + // evaluate static case + test( + "switch (1) { case 1: a(); break; default: b(); }", + "if (1 === 1) { a(); } else { b(); }", + ); + test( + "switch (1) { case 2: foo(); break; default: bar(); }", + "if (1 === 2) { foo(); } else { bar(); }", + ); + // special cases + test_same("switch (test) { case 1: fn(); break; case 2: bar(); break; }"); + test_same("switch (x) { case 1: a(); case 2: b(); }"); + test( + "switch (2) { default: log(3); case 1: log(2) }", + "if (2 === 1) {log(2);} else {log(3);}", + ); + + test("switch(a){}", "a;"); + test("switch(foo()){}", "foo()"); + test("switch(a){default:}", "a;"); + test("switch(a){default:break;}", "a;"); + test("switch(a){default:var b;break;}", "a;var b"); + test("switch(a){case 1: default:}", "a;"); + test("switch(a){default: case 1:}", "if (a === 1) {} else {}"); + test("switch(a){default: break; case 1:break;}", "if (a === 1) {} else {}"); + test( + "switch(a){default: var b; break; case 1: var c; break;}", + "if (a === 1) { var c; } else { var b; }", + ); + test("var x=1; switch(x) { case 1: var y; }", "var y;"); + test( + "function f() {switch(a){default: return; case 1: break;}}", + "function f() { if (a === 1) {} else { return; } }", + ); + test( + "function f() {switch(1){default: return; case 1: break;}}", + "function f() {if (1 === 1) {} else {return;}}", + ); + test("function f() {switch(a){case 1: foo();}}", "function f() {a === 1 && foo();}"); + test_same("function f() {switch(a){case 3: case 2: case 1: foo();}}"); + test( + "function f() {switch(a){case 2: case 1: default: foo();}}", + "function f() {a, foo();}", + ); + // test cases from closure-compiler + // + test_same("switch(a){case 1: default:break; case 2: foo()}"); + test_same("switch(a){case 1: goo(); default:break; case 2: foo()}"); + test_same("switch(a){case 1: goo(); case 2:break; case 3: foo()}"); + test("switch(1){case 2: var x=0;}", "if (0) var x;"); + test_same( + "switch ('repeated') { case 'repeated': foo(); break; case 'repeated': var x=0; break;}", + ); + test_same( + "switch ('repeated') { case 'repeated': foo(); break; case 'repeated': bar(); break;}", + ); + test("switch(a){case 1: var c =2; break;}", "if (a === 1) var c = 2;"); + test("function f() {switch(a){case 1: return;}}", "function f() {a;}"); + test("x:switch(a){case 1: break x;}", "x: if (a === 1) break x;"); + test("let x;switch (use(x)) { default: {let y;} }", "let x;use(void 0);{let y;}"); + test("let x;switch (use?.(x)) {default: {let y;}}", "let x;use?.(void 0);{let y;}"); + test("let x;switch (use(x)) {default: let y;}", "let x;{use(void 0);let y;}"); + test_same(" function f() { switch('x') { case 'x': var x = 1; break; case 'y': break; } }"); + test( + "function f() { switch(x) { case 'y': break; default: var x = 1; } }", + "function f() { if (x === 'y') {} else { var x = 1; } }", + ); + test("let x = 1; switch('x') { case 'x': let x = 2; break;}", "let x = 1; { let x = 2 }"); + test( + "switch (x) { default: if (a) { break; } bar(); }", + "switch (x) { default: if (a) break; bar(); }", + ); + } + + #[test] + fn minimize_switch_with_literal() { + test("switch ('\\v') {case '\\u000B': foo();}", "foo()"); + test_same("switch ('empty') {case 'empty':case 'foo': foo();}"); + test( + "switch (1) { case 1: case 2: case 3: { break; } case 4: case 5: case 6: default: fail('Should not get here'); break; }", + "switch (1) { case 1: case 2: case 3: break; default: fail('Should not get here'); break; }", + ); + test_same("switch (1) {case 1: foo();break;case 2: bar();break;}"); + test_same("switch (1) {case 1.1: foo();break;case 2: bar();break;}"); + test("outer: switch (2) {case 2: f(); break outer; }", "outer: {f(); break outer;}"); + test("outer: {switch (2) {case 2:f();break outer;}}", "outer: {f(); break outer;}"); + test_same("switch ('foo') {case 'foo': foo();break;case 'bar': bar();break;}"); + test_same("switch ('noMatch') {case 'foo': foo();break;case 'bar': bar();break;}"); + test_same( + "switch ('fallThru') {case 'fallThru': if (foo(123) > 0) {foobar(1);break;} foobar(2);case 'bar': bar();}", + ); + test_same("switch ('fallThru') {case 'fallThru': foo();case 'bar': bar();}"); + test( + "switch ('hasDefaultCase') { case 'foo': foo(); break; default: bar(); break; }", + "if ('hasDefaultCase' === 'foo') { foo(); } else { bar(); }", + ); + test_same( + "switch ('foo') {case 'bar': bar();break;case notConstant: foobar();break;case 'foo': foo();break;}", + ); + test_same( + "switch (0) {case NaN: foobar();break;case -0.0: foo();break;case 2: bar();break;}", + ); + } +} diff --git a/crates/oxc_minifier/src/peephole/mod.rs b/crates/oxc_minifier/src/peephole/mod.rs index 40875ece1040f..4f7dfeabb1ba5 100644 --- a/crates/oxc_minifier/src/peephole/mod.rs +++ b/crates/oxc_minifier/src/peephole/mod.rs @@ -9,6 +9,7 @@ mod minimize_if_statement; mod minimize_logical_expression; mod minimize_not_expression; mod minimize_statements; +mod minimize_switch_statement; mod normalize; mod remove_dead_code; mod remove_unused_declaration; @@ -194,6 +195,9 @@ impl<'a> Traverse<'a, MinifierState<'a>> for PeepholeOptimizations { ctx.state.changed = true; } } + Statement::SwitchStatement(_) => { + Self::try_minimize_switch(stmt, ctx); + } Statement::WhileStatement(s) => { Self::minimize_expression_in_boolean_context(&mut s.test, ctx); } diff --git a/crates/oxc_minifier/tests/peephole/esbuild.rs b/crates/oxc_minifier/tests/peephole/esbuild.rs index 814516471d245..bb262ae75d8cc 100644 --- a/crates/oxc_minifier/tests/peephole/esbuild.rs +++ b/crates/oxc_minifier/tests/peephole/esbuild.rs @@ -285,11 +285,11 @@ fn js_parser_test() { test("while(1) { async function* x() {} }", "for (;;) { async function* x() { }}"); test( "function _() { x(); switch (y) { case z: return w; } }", - "function _() { switch (x(), y) { case z: return w; }}", + "function _() { if (x(), y === z) return w; }", ); test( "function _() { if (t) { x(); switch (y) { case z: return w; } } }", - "function _() { if (t) switch (x(), y) { case z: return w; } }", + "function _() { if (t && (x(), y === z)) return w; }", ); test("a = '' + 0", "a = '0';"); test("a = 0 + ''", "a = '0';"); @@ -2021,7 +2021,7 @@ fn test_inline_single_use_variable() { ); test( "function wrapper(arg0, arg1) { let x = arg0; switch (x) { case 0: return 1; }}", - "function wrapper(arg0, arg1) { switch (arg0) { case 0: return 1; }}", + "function wrapper(arg0, arg1) { if (arg0 === 0) return 1; }", ); test( "function wrapper(arg0, arg1) { let x = arg0; let y = x; return y + y;}", diff --git a/crates/oxc_minifier/tests/peephole/obscure_edge_cases.rs b/crates/oxc_minifier/tests/peephole/obscure_edge_cases.rs index f08401e826907..900eedef68750 100644 --- a/crates/oxc_minifier/tests/peephole/obscure_edge_cases.rs +++ b/crates/oxc_minifier/tests/peephole/obscure_edge_cases.rs @@ -314,7 +314,10 @@ fn test_switch_statement_edge_cases() { // Could be optimized to empty // Test switch with default - test_same("switch (5) { case 1: a(); break; default: b(); break; }"); + test( + "switch (5) { case 1: a(); break; default: b(); break; }", + "if (5 === 1) { a(); } else { b(); }", + ); // Could be optimized to just: b(); // Test switch with fall-through - more complex, keep as same for safety diff --git a/crates/oxc_minifier/tests/peephole/statement_fusion.rs b/crates/oxc_minifier/tests/peephole/statement_fusion.rs index 93b6973fd945c..7e1cb0bc5a828 100644 --- a/crates/oxc_minifier/tests/peephole/statement_fusion.rs +++ b/crates/oxc_minifier/tests/peephole/statement_fusion.rs @@ -39,7 +39,7 @@ fn fold_block_throw() { #[test] fn fold_switch() { - test("a;b;c;switch(x){}", "switch(a,b,c,x){}"); + test("a;b;c;switch(x){}", "a,b,c,x"); } #[test] @@ -94,7 +94,7 @@ fn fuse_into_block() { #[test] fn fuse_into_switch_cases() { - test("switch (_) { case _: a; return b }", "switch (_) { case _: return a, b }"); + test("switch (_) { case _: a; return b }", "if (_ === _) return a, b;"); } #[test] diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 1baa98af471ad..0b9ed3fb4464f 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -3,7 +3,7 @@ Original | minified | minified | gzip | gzip | Iterations | Fi ------------------------------------------------------------------------------------- 72.14 kB | 23.18 kB | 23.70 kB | 8.38 kB | 8.54 kB | 2 | react.development.js -173.90 kB | 59.40 kB | 59.82 kB | 19.16 kB | 19.33 kB | 2 | moment.js +173.90 kB | 59.38 kB | 59.82 kB | 19.15 kB | 19.33 kB | 2 | moment.js 287.63 kB | 89.26 kB | 90.07 kB | 30.91 kB | 31.95 kB | 2 | jquery.js @@ -15,13 +15,13 @@ Original | minified | minified | gzip | gzip | Iterations | Fi 1.01 MB | 439.38 kB | 458.89 kB | 122.06 kB | 126.71 kB | 2 | bundle.min.js -1.25 MB | 642.66 kB | 646.76 kB | 159.40 kB | 163.73 kB | 2 | three.js +1.25 MB | 642.64 kB | 646.76 kB | 159.39 kB | 163.73 kB | 2 | three.js 2.14 MB | 711.14 kB | 724.14 kB | 160.43 kB | 181.07 kB | 2 | victory.js 3.20 MB | 1.00 MB | 1.01 MB | 322.59 kB | 331.56 kB | 3 | echarts.js -6.69 MB | 2.22 MB | 2.31 MB | 458.44 kB | 488.28 kB | 4 | antd.js +6.69 MB | 2.22 MB | 2.31 MB | 458.42 kB | 488.28 kB | 4 | antd.js -10.95 MB | 3.33 MB | 3.49 MB | 853.36 kB | 915.50 kB | 4 | typescript.js +10.95 MB | 3.33 MB | 3.49 MB | 853.39 kB | 915.50 kB | 4 | typescript.js diff --git a/tasks/track_memory_allocations/allocs_minifier.snap b/tasks/track_memory_allocations/allocs_minifier.snap index 94eca5ee1ee0f..8e8fcd67696c4 100644 --- a/tasks/track_memory_allocations/allocs_minifier.snap +++ b/tasks/track_memory_allocations/allocs_minifier.snap @@ -2,13 +2,13 @@ File | File size || Sys allocs | Sys reallocs | ------------------------------------------------------------------------------------------------------------------------------------------- checker.ts | 2.92 MB || 83502 | 14179 || 152592 | 28237 -cal.com.tsx | 1.06 MB || 40447 | 3032 || 37138 | 4586 +cal.com.tsx | 1.06 MB || 40447 | 3032 || 37141 | 4586 RadixUIAdoptionSection.jsx | 2.52 kB || 85 | 9 || 30 | 6 pdf.mjs | 567.30 kB || 20193 | 2899 || 47453 | 7730 -antd.js | 6.69 MB || 99534 | 13524 || 331649 | 69358 +antd.js | 6.69 MB || 99533 | 13524 || 331644 | 69342 binder.ts | 193.08 kB || 4759 | 974 || 7075 | 824 From 1b6fe1dde7797c31cc474922f04257c4b03bc182 Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 06:01:49 +0100 Subject: [PATCH 02/25] fix: correct linting issues --- .../src/peephole/minimize_switch_statement.rs | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 1bb28a60cbd1d..55dde046347ab 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -25,9 +25,7 @@ impl<'a> PeepholeOptimizations { } // Position of the (last) default if any - let Some(default_pos) = cases.iter().rposition(|c| c.is_default_case()) else { - return None; - }; + let default_pos = cases.iter().rposition(SwitchCase::is_default_case)?; // Find the last non-removable case (any case whose consequent is non-empty). let last_non_empty_before_last = cases[..case_count - 1].iter().rposition(|c| { @@ -67,11 +65,11 @@ impl<'a> PeepholeOptimizations { return; }; - if let Some(last) = switch_stmt.cases.last_mut() { - if !Self::is_empty_switch_case(&last.consequent) { - last.test = None; - end -= 1; - } + if let Some(last) = switch_stmt.cases.last_mut() + && !Self::is_empty_switch_case(&last.consequent) + { + last.test = None; + end -= 1; } switch_stmt.cases.drain(start..end); ctx.state.changed = true; @@ -82,13 +80,13 @@ impl<'a> PeepholeOptimizations { return; }; if switch_stmt.cases.is_empty() { - if !switch_stmt.discriminant.may_have_side_effects(ctx) { - *stmt = ctx.ast.statement_empty(switch_stmt.span); - } else { + if switch_stmt.discriminant.may_have_side_effects(ctx) { *stmt = ctx.ast.statement_expression( switch_stmt.span, switch_stmt.discriminant.take_in(ctx.ast), ); + } else { + *stmt = ctx.ast.statement_empty(switch_stmt.span); } ctx.state.changed = true; } @@ -136,7 +134,7 @@ impl<'a> PeepholeOptimizations { mut vec: Vec<'a, Statement<'a>>, ctx: &mut Ctx<'a, '_>, ) -> Statement<'a> { - if vec.len() == 1 && matches!(vec.get(0), Some(Statement::BlockStatement(_))) { + if vec.len() == 1 && matches!(vec.first(), Some(Statement::BlockStatement(_))) { vec.pop().unwrap() } else { ctx.ast.statement_block_with_scope_id( @@ -152,8 +150,8 @@ impl<'a> PeepholeOptimizations { return; }; if switch_stmt.cases.len() == 1 { - let Some(first_case) = switch_stmt.cases.get(0) else { return }; - if !Self::can_case_be_inlined(&first_case) { + let Some(first_case) = switch_stmt.cases.first() else { return }; + if !Self::can_case_be_inlined(first_case) { return; } let Some(mut case) = switch_stmt.cases.pop() else { @@ -181,21 +179,21 @@ impl<'a> PeepholeOptimizations { if discriminant.may_have_side_effects(ctx) { stmts.push(ctx.ast.statement_expression(discriminant.span(), discriminant)); } - if case.consequent.len() > 0 { + if !case.consequent.is_empty() { stmts.extend(case.consequent); } *stmt = ctx.ast.statement_block_with_scope_id( switch_stmt.span, stmts, ctx.create_child_scope_of_current(ScopeFlags::empty()), - ) + ); } } } pub fn drop_break_and_postfix(cons: &mut Vec<'a, Statement<'a>>) { // TODO: this is wrong, as it doesn't take into account BlockStatement - if cons.len() > 0 + if !cons.is_empty() && let Some(terminates_rpos) = cons.iter().rposition(Self::is_terminated) { if cons @@ -244,7 +242,7 @@ struct BreakFinder { nested_unlabelled_break: bool, } -impl<'a> BreakFinder { +impl BreakFinder { pub fn new() -> Self { Self { top_level: true, nested_unlabelled_break: false } } @@ -280,6 +278,7 @@ impl<'a> Visit<'a> for BreakFinder { mod test { use crate::tester::{test, test_same}; + #[expect(clippy::literal_string_with_formatting_args)] #[test] fn minimize_switch() { // remove empty From 19d17a23d6f9156c2c825831747242fc03bde549 Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 11:05:30 +0100 Subject: [PATCH 03/25] fix: correct issues with nested blocks and cleanup switch test --- .../src/peephole/minimize_switch_statement.rs | 247 +++++++----------- 1 file changed, 101 insertions(+), 146 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 55dde046347ab..5dcc02b44b1b5 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -44,7 +44,7 @@ impl<'a> PeepholeOptimizations { return None; } - // Reject only when a non-empty default lies inside the removable suffix and it is not the last case. + // Reject only when a non-empty default lies inside the removable suffix, and it is not the last case. if default_pos >= start && default_pos != case_count - 1 && !&cases[default_pos].consequent.is_empty() @@ -70,8 +70,10 @@ impl<'a> PeepholeOptimizations { { last.test = None; end -= 1; + switch_stmt.cases.drain(start..end); + } else { + switch_stmt.cases.truncate(start); } - switch_stmt.cases.drain(start..end); ctx.state.changed = true; } @@ -107,8 +109,8 @@ impl<'a> PeepholeOptimizations { let mut first = switch_stmt.cases.pop().unwrap(); let mut second = switch_stmt.cases.pop().unwrap(); - Self::drop_break_and_postfix(&mut first.consequent); - Self::drop_break_and_postfix(&mut second.consequent); + Self::remove_last_break(&mut first.consequent); + Self::remove_last_break(&mut second.consequent); let (test, consequent, alternate) = if first.test.is_some() { (first.test.unwrap(), first.consequent, second.consequent) @@ -160,7 +162,7 @@ impl<'a> PeepholeOptimizations { ctx.state.changed = true; let discriminant = switch_stmt.discriminant.take_in(ctx.ast); - Self::drop_break_and_postfix(&mut case.consequent); + Self::remove_last_break(&mut case.consequent); if let Some(test) = case.test { *stmt = ctx.ast.statement_if( @@ -191,22 +193,6 @@ impl<'a> PeepholeOptimizations { } } - pub fn drop_break_and_postfix(cons: &mut Vec<'a, Statement<'a>>) { - // TODO: this is wrong, as it doesn't take into account BlockStatement - if !cons.is_empty() - && let Some(terminates_rpos) = cons.iter().rposition(Self::is_terminated) - { - if cons - .get(terminates_rpos) - .is_some_and(|stmt| matches!(stmt, Statement::BreakStatement(_))) - { - cons.truncate(terminates_rpos); - } else { - cons.truncate(terminates_rpos + 1); - } - } - } - fn is_empty_switch_case(stmt: &Vec) -> bool { if stmt.len() != 1 { return stmt.is_empty(); @@ -217,13 +203,32 @@ impl<'a> PeepholeOptimizations { } } - pub fn is_terminated(stmt: &Statement<'a>) -> bool { + fn remove_last_break(stmt: &mut Vec<'a, Statement<'a>>) { + if stmt.is_empty() { + return; + } + + let len = stmt.len(); + match stmt.last_mut() { + Some(Statement::BreakStatement(break_stmt)) => { + if break_stmt.label.is_none() { + stmt.truncate(len - 1); + } + } + Some(Statement::BlockStatement(block_stmt)) => { + Self::remove_last_break(&mut block_stmt.body); + } + _ => {} + } + } + + fn is_terminated(stmt: &Statement<'a>) -> bool { match stmt { - Statement::BreakStatement(break_stmt) => break_stmt.label.is_none(), Statement::BlockStatement(block_stmt) => { block_stmt.body.last().is_some_and(Self::is_terminated) } - Statement::ContinueStatement(_) + Statement::BreakStatement(_) + | Statement::ContinueStatement(_) | Statement::ReturnStatement(_) | Statement::ThrowStatement(_) => true, _ => false, @@ -258,6 +263,7 @@ impl<'a> Visit<'a> for BreakFinder { self.nested_unlabelled_break = true; } } + // TODO: this is not fully correct, we should allow termination if this if is a last statement Statement::IfStatement(it) => { if self.top_level { self.top_level = false; @@ -267,8 +273,24 @@ impl<'a> Visit<'a> for BreakFinder { self.visit_if_statement(it); } } - Statement::TryStatement(it) => self.visit_try_statement(it), - Statement::WithStatement(it) => self.visit_with_statement(it), + Statement::TryStatement(it) => { + if self.top_level { + self.top_level = false; + self.visit_try_statement(it); + self.top_level = true; + } else { + self.visit_try_statement(it); + } + } + Statement::WithStatement(it) => { + if self.top_level { + self.top_level = false; + self.visit_with_statement(it); + self.top_level = true; + } else { + self.visit_with_statement(it); + } + } _ => {} } } @@ -281,134 +303,67 @@ mod test { #[expect(clippy::literal_string_with_formatting_args)] #[test] fn minimize_switch() { - // remove empty - test("switch (empty) {}", "empty"); - test("switch (!true) {}", ""); - test("switch (true) {}", ""); - test("switch (fn()) {}", "fn()"); - // truncate cases - test("switch (test) { default: break; case 1: break; }", "if (test === 1) {} else {}"); - test("switch (test) { case 1: break; }", "test;"); - test_same("switch (test) { case 2: case 1: break; default: break }"); - test_same("switch (test) { case 3: foo(); case 2: case 1: break; default: break }"); - // single switch case - test("switch (fn()) { default: a(); break; }", "fn(),a()"); - test("switch (test) { default: fn(); return; }", "test, fn(); return"); - test("switch (fn()) { default: a(); }", "fn(), a()"); - test("switch (test) { case 1: fn(); break; }", "test === 1 && fn()"); - test("switch (test) { case 1: fn(); return; }", "if (test === 1) { fn(); return; }"); - test("switch (test) { case 1: fn(); }", "test === 1 && fn()"); - test("switch (a) { case log(1): default:}", "if (a === log(1)) {} else {}"); - // default + case - test( - "switch (c) { default: a(); break; case 1: b(); break; }", - "if (c === 1) { b(); } else { a(); }", - ); - test( - "switch (c) { default: { a(); break; } case 1: { b(); break; } }", - "c === 1 ? b() : a()", - ); - // evaluate static case - test( - "switch (1) { case 1: a(); break; default: b(); }", - "if (1 === 1) { a(); } else { b(); }", - ); - test( - "switch (1) { case 2: foo(); break; default: bar(); }", - "if (1 === 2) { foo(); } else { bar(); }", - ); - // special cases - test_same("switch (test) { case 1: fn(); break; case 2: bar(); break; }"); - test_same("switch (x) { case 1: a(); case 2: b(); }"); - test( - "switch (2) { default: log(3); case 1: log(2) }", - "if (2 === 1) {log(2);} else {log(3);}", - ); + test("switch(a()){}", "a()"); + test("switch(a){default: }", "a;"); + test("switch(a){default: break;}", "a;"); + test("switch(a){default: var b; break;}", "a;var b"); + test("switch(a){default: b()}", "a, b();"); + test("switch(a){default: b(); return;}", "a, b(); return"); - test("switch(a){}", "a;"); - test("switch(foo()){}", "foo()"); - test("switch(a){default:}", "a;"); - test("switch(a){default:break;}", "a;"); - test("switch(a){default:var b;break;}", "a;var b"); - test("switch(a){case 1: default:}", "a;"); - test("switch(a){default: case 1:}", "if (a === 1) {} else {}"); - test("switch(a){default: break; case 1:break;}", "if (a === 1) {} else {}"); - test( - "switch(a){default: var b; break; case 1: var c; break;}", - "if (a === 1) { var c; } else { var b; }", - ); - test("var x=1; switch(x) { case 1: var y; }", "var y;"); - test( - "function f() {switch(a){default: return; case 1: break;}}", - "function f() { if (a === 1) {} else { return; } }", - ); - test( - "function f() {switch(1){default: return; case 1: break;}}", - "function f() {if (1 === 1) {} else {return;}}", - ); - test("function f() {switch(a){case 1: foo();}}", "function f() {a === 1 && foo();}"); - test_same("function f() {switch(a){case 3: case 2: case 1: foo();}}"); - test( - "function f() {switch(a){case 2: case 1: default: foo();}}", - "function f() {a, foo();}", - ); - // test cases from closure-compiler - // - test_same("switch(a){case 1: default:break; case 2: foo()}"); - test_same("switch(a){case 1: goo(); default:break; case 2: foo()}"); - test_same("switch(a){case 1: goo(); case 2:break; case 3: foo()}"); - test("switch(1){case 2: var x=0;}", "if (0) var x;"); - test_same( - "switch ('repeated') { case 'repeated': foo(); break; case 'repeated': var x=0; break;}", - ); - test_same( - "switch ('repeated') { case 'repeated': foo(); break; case 'repeated': bar(); break;}", - ); - test("switch(a){case 1: var c =2; break;}", "if (a === 1) var c = 2;"); - test("function f() {switch(a){case 1: return;}}", "function f() {a;}"); - test("x:switch(a){case 1: break x;}", "x: if (a === 1) break x;"); - test("let x;switch (use(x)) { default: {let y;} }", "let x;use(void 0);{let y;}"); - test("let x;switch (use?.(x)) {default: {let y;}}", "let x;use?.(void 0);{let y;}"); - test("let x;switch (use(x)) {default: let y;}", "let x;{use(void 0);let y;}"); - test_same(" function f() { switch('x') { case 'x': var x = 1; break; case 'y': break; } }"); - test( - "function f() { switch(x) { case 'y': break; default: var x = 1; } }", - "function f() { if (x === 'y') {} else { var x = 1; } }", - ); - test("let x = 1; switch('x') { case 'x': let x = 2; break;}", "let x = 1; { let x = 2 }"); - test( - "switch (x) { default: if (a) { break; } bar(); }", - "switch (x) { default: if (a) break; bar(); }", - ); - } + test("switch(a){case 1: break;}", "a;"); + test("switch(a){case 1: b();}", "a === 1 && b()"); + test("switch(a){case 1: b();break; }", "a === 1 && b()"); + test("switch(a){case 1: b();return; }", "if (a === 1) { b(); return; }"); - #[test] - fn minimize_switch_with_literal() { + test("switch(a){case 1: default: }", "a;"); + test_same("switch(a){case 1: default: break; case 2: b()}"); + test_same("switch(a){case 1: b(); default: break; case 2: c()}"); + test_same("switch(a){case 1: b(); case 2: break; case 3: c()}"); + test_same("switch(a){case 1: b(); break; case 2: c();break;}"); + test_same("switch(a){case 1: b(); case 2: b();}"); + test("switch(a){case 1: var c=2; break;}", "if (a === 1) var c = 2;"); + + test("switch(a){default: case 1: }", "if (a === 1) {} else {}"); + test("switch(a){default: break; case 1: break;}", "if (a === 1) {} else {}"); + test("switch(a){default: b();break;case 1: c();break;}", "if (a === 1) {c();} else {b();}"); + test("switch(a){default: {b();break;} case 1: {c();break;}}", "a === 1 ? c() : b()"); + + test("switch(a){case b(): default:}", "if (a === b()) {} else {}"); + test_same("switch(a){case 2: case 1: break; default: break }"); + test_same("switch(a){case 3: b(); case 2: case 1: break; default: break }"); + + test("var x=1;switch(x){case 1: var y;}", "var y;"); + test("function f(){switch(a){case 1: return;}}", "function f() {a;}"); + test("x:switch(a){case 1: break x;}", "x: if (a === 1) break x;"); + test("switch(a()) { default: {let y;} }", "a();{let y;}"); + test_same("function f(){switch('x'){case 'x': var x = 1;break; case 'y': break; }}"); + test("switch(a){default: if(a) {break;}bar();}", "switch(a){default: if(a) break;bar();}"); test("switch ('\\v') {case '\\u000B': foo();}", "foo()"); - test_same("switch ('empty') {case 'empty':case 'foo': foo();}"); + + test_same("switch('r'){case 'r': a();break; case 'r': var x=0;break;}"); + test_same("switch('r'){case 'r': a();break; case 'r': bar();break;}"); + test("switch(2) {default: a; case 1: b()}", "if(2===1) {b();} else {a;}"); + test("switch(1) {case 1: a();break; default: b();}", "if(1===1) {a();} else {b();}"); + test_same("switch('e') {case 'e': case 'f': a();}"); + test_same("switch('a') {case 'a': a();break; case 'b': b();break;}"); + test_same("switch('c') {case 'a': a();break; case 'b': b();break;}"); + test_same("switch(1) {case 1: a();break; case 2: bar();break;}"); + test_same("switch('f') {case 'f': a(); case 'b': b();}"); + test_same("switch('f') {case 'f': if (a() > 0) {b();break;} c(); case 'd': f();}"); + test_same("switch('f') {case 'b': bar();break; case x: x();break; case 'f': f();break;}"); test( - "switch (1) { case 1: case 2: case 3: { break; } case 4: case 5: case 6: default: fail('Should not get here'); break; }", - "switch (1) { case 1: case 2: case 3: break; default: fail('Should not get here'); break; }", + "switch(1){case 1: case 2: {break;} case 3: case 4: default: b(); break;}", + "switch(1){case 1: case 2: break; default: b(); break;}", ); - test_same("switch (1) {case 1: foo();break;case 2: bar();break;}"); - test_same("switch (1) {case 1.1: foo();break;case 2: bar();break;}"); - test("outer: switch (2) {case 2: f(); break outer; }", "outer: {f(); break outer;}"); - test("outer: {switch (2) {case 2:f();break outer;}}", "outer: {f(); break outer;}"); - test_same("switch ('foo') {case 'foo': foo();break;case 'bar': bar();break;}"); - test_same("switch ('noMatch') {case 'foo': foo();break;case 'bar': bar();break;}"); - test_same( - "switch ('fallThru') {case 'fallThru': if (foo(123) > 0) {foobar(1);break;} foobar(2);case 'bar': bar();}", - ); - test_same("switch ('fallThru') {case 'fallThru': foo();case 'bar': bar();}"); test( - "switch ('hasDefaultCase') { case 'foo': foo(); break; default: bar(); break; }", - "if ('hasDefaultCase' === 'foo') { foo(); } else { bar(); }", - ); - test_same( - "switch ('foo') {case 'bar': bar();break;case notConstant: foobar();break;case 'foo': foo();break;}", + "switch ('d') {case 'foo': foo();break; default: bar();break;}", + "if('d'==='foo') {foo();} else {bar();}", ); + test("outer: switch (2) {case 2: f(); break outer; }", "outer: {f(); break outer;}"); test_same( - "switch (0) {case NaN: foobar();break;case -0.0: foo();break;case 2: bar();break;}", + "switch(0){case NaN: foobar();break;case -0.0: foo();break; case 2: bar();break;}", ); + test("let x = 1; switch('x') { case 'x': let x = 2; break;}", "let x = 1; { let x = 2 }"); + test("switch(1){case 2: var x=0;}", "if (0) var x;"); } } From ff561dde6f60d4d226b2fcf4a3f99f4ec4272a6f Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 12:59:15 +0100 Subject: [PATCH 04/25] fix: remove side effect free last elements if there is no default --- .../src/peephole/minimize_switch_statement.rs | 55 ++++++++++--------- tasks/minsize/minsize.snap | 2 +- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 5dcc02b44b1b5..3ce4c8fe28ebb 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -24,11 +24,18 @@ impl<'a> PeepholeOptimizations { return None; } - // Position of the (last) default if any - let default_pos = cases.iter().rposition(SwitchCase::is_default_case)?; + // if a default case is last we can skip checking if it has body + let default_pos = if let Some(default_pos) = + cases.iter().rposition(SwitchCase::is_default_case) + && default_pos == case_count - 1 + { + case_count - 1 + } else { + case_count + }; // Find the last non-removable case (any case whose consequent is non-empty). - let last_non_empty_before_last = cases[..case_count - 1].iter().rposition(|c| { + let last_non_empty_before_last = cases[..default_pos].iter().rposition(|c| { !c.consequent.is_empty() || c.test.as_ref().is_none_or(|test| test.may_have_side_effects(ctx)) }); @@ -44,13 +51,6 @@ impl<'a> PeepholeOptimizations { return None; } - // Reject only when a non-empty default lies inside the removable suffix, and it is not the last case. - if default_pos >= start - && default_pos != case_count - 1 - && !&cases[default_pos].consequent.is_empty() - { - return None; - } Some((start, case_count)) } @@ -222,18 +222,18 @@ impl<'a> PeepholeOptimizations { } } - fn is_terminated(stmt: &Statement<'a>) -> bool { - match stmt { - Statement::BlockStatement(block_stmt) => { - block_stmt.body.last().is_some_and(Self::is_terminated) - } - Statement::BreakStatement(_) - | Statement::ContinueStatement(_) - | Statement::ReturnStatement(_) - | Statement::ThrowStatement(_) => true, - _ => false, - } - } + // fn is_terminated(stmt: &Statement<'a>) -> bool { + // match stmt { + // Statement::BlockStatement(block_stmt) => { + // block_stmt.body.last().is_some_and(Self::is_terminated) + // } + // Statement::BreakStatement(_) + // | Statement::ContinueStatement(_) + // | Statement::ReturnStatement(_) + // | Statement::ThrowStatement(_) => true, + // _ => false, + // } + // } pub fn can_case_be_inlined(case: &SwitchCase) -> bool { let mut break_finder = BreakFinder::new(); @@ -255,6 +255,7 @@ impl BreakFinder { impl<'a> Visit<'a> for BreakFinder { fn visit_statement(&mut self, it: &Statement<'a>) { + // TODO: check if there we have access to control flow instead?? // Only visit blocks where vars could be hoisted match it { Statement::BlockStatement(it) => self.visit_block_statement(it), @@ -321,7 +322,9 @@ mod test { test_same("switch(a){case 1: b(); case 2: break; case 3: c()}"); test_same("switch(a){case 1: b(); break; case 2: c();break;}"); test_same("switch(a){case 1: b(); case 2: b();}"); - test("switch(a){case 1: var c=2; break;}", "if (a === 1) var c = 2;"); + test("switch(a){case 1: var c=2; break;}", "if (a === 1) var c = 2"); + test("switch(a){case 1: case 2: default: b(); break;}", "a, b()"); + test("switch(a){case 1: c(); case 2: default: b(); break;}", "a === 1 ? c() : b()"); test("switch(a){default: case 1: }", "if (a === 1) {} else {}"); test("switch(a){default: break; case 1: break;}", "if (a === 1) {} else {}"); @@ -330,14 +333,16 @@ mod test { test("switch(a){case b(): default:}", "if (a === b()) {} else {}"); test_same("switch(a){case 2: case 1: break; default: break }"); - test_same("switch(a){case 3: b(); case 2: case 1: break; default: break }"); + test_same("switch(a){case 3: b(); case 2: case 1: break }"); + test("switch(a){case 3: b(); case 2: case 1: }", "a === 3 && b()"); test("var x=1;switch(x){case 1: var y;}", "var y;"); test("function f(){switch(a){case 1: return;}}", "function f() {a;}"); test("x:switch(a){case 1: break x;}", "x: if (a === 1) break x;"); test("switch(a()) { default: {let y;} }", "a();{let y;}"); test_same("function f(){switch('x'){case 'x': var x = 1;break; case 'y': break; }}"); - test("switch(a){default: if(a) {break;}bar();}", "switch(a){default: if(a) break;bar();}"); + test("switch(a){default: if(a) {break;}c();}", "switch(a){default: if(a) break;c();}"); + test("switch(a){case 1: if(a) {b();}c();}", "a === 1 && (a && b(), c())"); test("switch ('\\v') {case '\\u000B': foo();}", "foo()"); test_same("switch('r'){case 'r': a();break; case 'r': var x=0;break;}"); diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 0b9ed3fb4464f..1d3248f47906e 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -23,5 +23,5 @@ Original | minified | minified | gzip | gzip | Iterations | Fi 6.69 MB | 2.22 MB | 2.31 MB | 458.42 kB | 488.28 kB | 4 | antd.js -10.95 MB | 3.33 MB | 3.49 MB | 853.39 kB | 915.50 kB | 4 | typescript.js +10.95 MB | 3.33 MB | 3.49 MB | 853.38 kB | 915.50 kB | 4 | typescript.js From 8b21ed6d47488964c33a484ffea5583f79a81099 Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 13:31:25 +0100 Subject: [PATCH 05/25] fix: remove last break and correct state.changed --- .../src/peephole/minimize_switch_statement.rs | 93 ++++++++++++++----- tasks/minsize/minsize.snap | 20 ++-- .../allocs_minifier.snap | 8 +- 3 files changed, 83 insertions(+), 38 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 3ce4c8fe28ebb..2c1bad84e8a71 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -9,16 +9,30 @@ use oxc_syntax::{operator::BinaryOperator, scope::ScopeFlags}; impl<'a> PeepholeOptimizations { pub fn try_minimize_switch(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + Self::try_remove_last_break_from_case(stmt, ctx); Self::collapse_empty_switch_cases(stmt, ctx); Self::remove_empty_switch(stmt, ctx); Self::fold_switch_with_one_case(stmt, ctx); Self::fold_switch_with_two_cases(stmt, ctx); } + fn try_remove_last_break_from_case(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + + if let Some(last_case) = switch_stmt.cases.last_mut() + && Self::remove_last_break(&mut last_case.consequent) + { + ctx.state.changed = true; + } + } + fn find_trailing_removable_switch_cases_range( cases: &Vec<'_, SwitchCase<'a>>, ctx: &Ctx<'a, '_>, ) -> Option<(usize, usize)> { + // TODO: this is not ideal, if there is default case that is empty, we can remove other cases even if they have break let case_count = cases.len(); if case_count <= 1 { return None; @@ -107,6 +121,8 @@ impl<'a> PeepholeOptimizations { return; } + // TODO: missing check for fallthrough + let mut first = switch_stmt.cases.pop().unwrap(); let mut second = switch_stmt.cases.pop().unwrap(); Self::remove_last_break(&mut first.consequent); @@ -118,6 +134,7 @@ impl<'a> PeepholeOptimizations { (second.test.unwrap(), second.consequent, first.consequent) }; + ctx.state.changed = true; *stmt = ctx.ast.statement_if( switch_stmt.span, ctx.ast.expression_binary( @@ -203,23 +220,26 @@ impl<'a> PeepholeOptimizations { } } - fn remove_last_break(stmt: &mut Vec<'a, Statement<'a>>) { + fn remove_last_break(stmt: &mut Vec<'a, Statement<'a>>) -> bool { if stmt.is_empty() { - return; + return false; } + let mut changed = false; let len = stmt.len(); match stmt.last_mut() { Some(Statement::BreakStatement(break_stmt)) => { if break_stmt.label.is_none() { stmt.truncate(len - 1); + changed |= true } } Some(Statement::BlockStatement(block_stmt)) => { - Self::remove_last_break(&mut block_stmt.body); + changed |= Self::remove_last_break(&mut block_stmt.body); } _ => {} } + changed } // fn is_terminated(stmt: &Statement<'a>) -> bool { @@ -320,53 +340,78 @@ mod test { test_same("switch(a){case 1: default: break; case 2: b()}"); test_same("switch(a){case 1: b(); default: break; case 2: c()}"); test_same("switch(a){case 1: b(); case 2: break; case 3: c()}"); - test_same("switch(a){case 1: b(); break; case 2: c();break;}"); + test( + "switch(a){case 1: b(); break; case 2: c();break;}", + "switch(a){case 1: b(); break; case 2: c();}", + ); test_same("switch(a){case 1: b(); case 2: b();}"); test("switch(a){case 1: var c=2; break;}", "if (a === 1) var c = 2"); test("switch(a){case 1: case 2: default: b(); break;}", "a, b()"); test("switch(a){case 1: c(); case 2: default: b(); break;}", "a === 1 ? c() : b()"); - test("switch(a){default: case 1: }", "if (a === 1) {} else {}"); - test("switch(a){default: break; case 1: break;}", "if (a === 1) {} else {}"); - test("switch(a){default: b();break;case 1: c();break;}", "if (a === 1) {c();} else {b();}"); + test("switch(a){default: case 1: }", "a"); + test("switch(a){default: break; case 1: break;}", "a"); + test("switch(a){default: b();break;case 1: c();break;}", "a === 1 ? c() : b()"); test("switch(a){default: {b();break;} case 1: {c();break;}}", "a === 1 ? c() : b()"); - test("switch(a){case b(): default:}", "if (a === b()) {} else {}"); - test_same("switch(a){case 2: case 1: break; default: break }"); - test_same("switch(a){case 3: b(); case 2: case 1: break }"); + test("switch(a){case b(): default:}", "a, b()"); + test( + "switch(a){case 2: case 1: break; default: break;}", + "switch(a){case 2: case 1: break; default: }", + ); + test("switch(a){case 3: b(); case 2: case 1: break;}", "a === 3 && b()"); test("switch(a){case 3: b(); case 2: case 1: }", "a === 3 && b()"); test("var x=1;switch(x){case 1: var y;}", "var y;"); test("function f(){switch(a){case 1: return;}}", "function f() {a;}"); test("x:switch(a){case 1: break x;}", "x: if (a === 1) break x;"); test("switch(a()) { default: {let y;} }", "a();{let y;}"); - test_same("function f(){switch('x'){case 'x': var x = 1;break; case 'y': break; }}"); + test( + "function f(){switch('x'){case 'x': var x = 1;break; case 'y': break; }}", + "function f(){switch('x'){case 'x': var x = 1;break; case 'y': }}", + ); test("switch(a){default: if(a) {break;}c();}", "switch(a){default: if(a) break;c();}"); test("switch(a){case 1: if(a) {b();}c();}", "a === 1 && (a && b(), c())"); test("switch ('\\v') {case '\\u000B': foo();}", "foo()"); - test_same("switch('r'){case 'r': a();break; case 'r': var x=0;break;}"); - test_same("switch('r'){case 'r': a();break; case 'r': bar();break;}"); - test("switch(2) {default: a; case 1: b()}", "if(2===1) {b();} else {a;}"); - test("switch(1) {case 1: a();break; default: b();}", "if(1===1) {a();} else {b();}"); + test( + "switch('r'){case 'r': a();break; case 'r': var x=0;break;}", + "switch('r'){case 'r': a();break; case 'r': var x=0;}", + ); + test( + "switch('r'){case 'r': a();break; case 'r': bar();break;}", + "switch('r'){case 'r': a();break; case 'r': bar()}", + ); + test("switch(2) {default: a; case 1: b()}", "a;"); + test("switch(1) {case 1: a();break; default: b();}", "a()"); test_same("switch('e') {case 'e': case 'f': a();}"); - test_same("switch('a') {case 'a': a();break; case 'b': b();break;}"); - test_same("switch('c') {case 'a': a();break; case 'b': b();break;}"); - test_same("switch(1) {case 1: a();break; case 2: bar();break;}"); + test( + "switch('a') {case 'a': a();break; case 'b': b();break;}", + "switch('a') {case 'a': a();break; case 'b': b();}", + ); + test( + "switch('c') {case 'a': a();break; case 'b': b();break;}", + "switch('c') {case 'a': a();break; case 'b': b();}", + ); + test( + "switch(1) {case 1: a();break; case 2: bar();break;}", + "switch(1) {case 1: a();break; case 2: bar();}", + ); test_same("switch('f') {case 'f': a(); case 'b': b();}"); test_same("switch('f') {case 'f': if (a() > 0) {b();break;} c(); case 'd': f();}"); - test_same("switch('f') {case 'b': bar();break; case x: x();break; case 'f': f();break;}"); test( - "switch(1){case 1: case 2: {break;} case 3: case 4: default: b(); break;}", - "switch(1){case 1: case 2: break; default: b(); break;}", + "switch('f') {case 'b': bar();break; case x: x();break; case 'f': f();break;}", + "switch('f') {case 'b': bar();break; case x: x();break; case 'f': f();}", ); test( - "switch ('d') {case 'foo': foo();break; default: bar();break;}", - "if('d'==='foo') {foo();} else {bar();}", + "switch(1){case 1: case 2: {break;} case 3: case 4: default: b(); break;}", + "switch(1){case 1: case 2: break; default: b();}", ); + test("switch ('d') {case 'foo': foo();break; default: bar();break;}", "bar()"); test("outer: switch (2) {case 2: f(); break outer; }", "outer: {f(); break outer;}"); - test_same( + test( "switch(0){case NaN: foobar();break;case -0.0: foo();break; case 2: bar();break;}", + "switch(0){case NaN: foobar();break;case -0.0: foo();break; case 2: bar();}", ); test("let x = 1; switch('x') { case 'x': let x = 2; break;}", "let x = 1; { let x = 2 }"); test("switch(1){case 2: var x=0;}", "if (0) var x;"); diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 1d3248f47906e..a83ad0aa70be8 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -3,25 +3,25 @@ Original | minified | minified | gzip | gzip | Iterations | Fi ------------------------------------------------------------------------------------- 72.14 kB | 23.18 kB | 23.70 kB | 8.38 kB | 8.54 kB | 2 | react.development.js -173.90 kB | 59.38 kB | 59.82 kB | 19.15 kB | 19.33 kB | 2 | moment.js +173.90 kB | 59.36 kB | 59.82 kB | 19.15 kB | 19.33 kB | 2 | moment.js 287.63 kB | 89.26 kB | 90.07 kB | 30.91 kB | 31.95 kB | 2 | jquery.js -342.15 kB | 116.98 kB | 118.14 kB | 43.17 kB | 44.37 kB | 2 | vue.js +342.15 kB | 116.97 kB | 118.14 kB | 43.17 kB | 44.37 kB | 2 | vue.js -544.10 kB | 71.04 kB | 72.48 kB | 25.78 kB | 26.20 kB | 2 | lodash.js +544.10 kB | 71.04 kB | 72.48 kB | 25.77 kB | 26.20 kB | 2 | lodash.js -555.77 kB | 267.39 kB | 270.13 kB | 88.00 kB | 90.80 kB | 2 | d3.js +555.77 kB | 267.21 kB | 270.13 kB | 87.99 kB | 90.80 kB | 2 | d3.js -1.01 MB | 439.38 kB | 458.89 kB | 122.06 kB | 126.71 kB | 2 | bundle.min.js +1.01 MB | 439.32 kB | 458.89 kB | 122.04 kB | 126.71 kB | 2 | bundle.min.js -1.25 MB | 642.64 kB | 646.76 kB | 159.39 kB | 163.73 kB | 2 | three.js +1.25 MB | 642.55 kB | 646.76 kB | 159.37 kB | 163.73 kB | 2 | three.js -2.14 MB | 711.14 kB | 724.14 kB | 160.43 kB | 181.07 kB | 2 | victory.js +2.14 MB | 711.00 kB | 724.14 kB | 160.42 kB | 181.07 kB | 2 | victory.js -3.20 MB | 1.00 MB | 1.01 MB | 322.59 kB | 331.56 kB | 3 | echarts.js +3.20 MB | 1.00 MB | 1.01 MB | 322.58 kB | 331.56 kB | 3 | echarts.js -6.69 MB | 2.22 MB | 2.31 MB | 458.42 kB | 488.28 kB | 4 | antd.js +6.69 MB | 2.22 MB | 2.31 MB | 458.41 kB | 488.28 kB | 4 | antd.js -10.95 MB | 3.33 MB | 3.49 MB | 853.38 kB | 915.50 kB | 4 | typescript.js +10.95 MB | 3.33 MB | 3.49 MB | 853.25 kB | 915.50 kB | 4 | typescript.js diff --git a/tasks/track_memory_allocations/allocs_minifier.snap b/tasks/track_memory_allocations/allocs_minifier.snap index 8e8fcd67696c4..e6238e3f36e20 100644 --- a/tasks/track_memory_allocations/allocs_minifier.snap +++ b/tasks/track_memory_allocations/allocs_minifier.snap @@ -1,14 +1,14 @@ File | File size || Sys allocs | Sys reallocs || Arena allocs | Arena reallocs | Arena bytes ------------------------------------------------------------------------------------------------------------------------------------------- -checker.ts | 2.92 MB || 83502 | 14179 || 152592 | 28237 +checker.ts | 2.92 MB || 83502 | 14179 || 152529 | 28180 cal.com.tsx | 1.06 MB || 40447 | 3032 || 37141 | 4586 RadixUIAdoptionSection.jsx | 2.52 kB || 85 | 9 || 30 | 6 -pdf.mjs | 567.30 kB || 20193 | 2899 || 47453 | 7730 +pdf.mjs | 567.30 kB || 20193 | 2899 || 47423 | 7704 -antd.js | 6.69 MB || 99533 | 13524 || 331644 | 69342 +antd.js | 6.69 MB || 99533 | 13524 || 331536 | 69298 -binder.ts | 193.08 kB || 4759 | 974 || 7075 | 824 +binder.ts | 193.08 kB || 4759 | 974 || 7073 | 822 From 5a3b0ef4cb0e856fc9ad7d6e07b531e5c4a388ed Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 13:33:33 +0100 Subject: [PATCH 06/25] fix: correct unit tests --- .../tests/peephole/obscure_edge_cases.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/oxc_minifier/tests/peephole/obscure_edge_cases.rs b/crates/oxc_minifier/tests/peephole/obscure_edge_cases.rs index 900eedef68750..4212ee337beb9 100644 --- a/crates/oxc_minifier/tests/peephole/obscure_edge_cases.rs +++ b/crates/oxc_minifier/tests/peephole/obscure_edge_cases.rs @@ -303,21 +303,24 @@ fn test_loop_optimization_edge_cases() { #[test] fn test_switch_statement_edge_cases() { // Test switch with constant discriminant - might be optimized in future - test_same("switch (2) { case 1: a(); break; case 2: b(); break; case 3: c(); break; }"); + test( + "switch (2) { case 1: a(); break; case 2: b(); break; case 3: c(); break; }", + "switch (2) { case 1: a(); break; case 2: b(); break; case 3: c(); }", + ); // Could be optimized to just: b(); test_same("switch ('test') { case 'foo': a(); break; case 'test': b(); break; default: c(); }"); // Could be optimized to just: b(); // Test switch with no matching case - test_same("switch (5) { case 1: a(); break; case 2: b(); break; }"); + test( + "switch (5) { case 1: a(); break; case 2: b(); break; }", + "switch (5) { case 1: a(); break; case 2: b(); }", + ); // Could be optimized to empty // Test switch with default - test( - "switch (5) { case 1: a(); break; default: b(); break; }", - "if (5 === 1) { a(); } else { b(); }", - ); + test("switch (5) { case 1: a(); break; default: b(); break; }", "b()"); // Could be optimized to just: b(); // Test switch with fall-through - more complex, keep as same for safety From 807c2538d4c321b88a0d696789a407ec5a71ba4a Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 13:57:36 +0100 Subject: [PATCH 07/25] fix: correct linting --- crates/oxc_minifier/src/peephole/minimize_switch_statement.rs | 2 +- crates/oxc_minifier/tests/peephole/statement_fusion.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 2c1bad84e8a71..a9a17cccc4104 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -231,7 +231,7 @@ impl<'a> PeepholeOptimizations { Some(Statement::BreakStatement(break_stmt)) => { if break_stmt.label.is_none() { stmt.truncate(len - 1); - changed |= true + changed |= true; } } Some(Statement::BlockStatement(block_stmt)) => { diff --git a/crates/oxc_minifier/tests/peephole/statement_fusion.rs b/crates/oxc_minifier/tests/peephole/statement_fusion.rs index 7e1cb0bc5a828..8b9aea942bcbb 100644 --- a/crates/oxc_minifier/tests/peephole/statement_fusion.rs +++ b/crates/oxc_minifier/tests/peephole/statement_fusion.rs @@ -94,7 +94,7 @@ fn fuse_into_block() { #[test] fn fuse_into_switch_cases() { - test("switch (_) { case _: a; return b }", "if (_ === _) return a, b;"); + test("switch (_) {case 1: case _: a; return b}", "switch (_) {case 1: case _: return a, b;}"); } #[test] From 7dc926422ec24fedcbc3808b34594a3d9ad669ad Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 14:14:06 +0100 Subject: [PATCH 08/25] fix: do not inline cases with side effects --- .../src/peephole/minimize_switch_statement.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index a9a17cccc4104..7f97c68bd4e40 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -115,8 +115,8 @@ impl<'a> PeepholeOptimizations { if switch_stmt.cases.len() == 2 { // check whatever its default + case if switch_stmt.cases[0].test.is_some() == switch_stmt.cases[1].test.is_some() - || !Self::can_case_be_inlined(&switch_stmt.cases[0]) - || !Self::can_case_be_inlined(&switch_stmt.cases[1]) + || !Self::can_case_be_inlined(&switch_stmt.cases[0], ctx) + || !Self::can_case_be_inlined(&switch_stmt.cases[1], ctx) { return; } @@ -170,7 +170,7 @@ impl<'a> PeepholeOptimizations { }; if switch_stmt.cases.len() == 1 { let Some(first_case) = switch_stmt.cases.first() else { return }; - if !Self::can_case_be_inlined(first_case) { + if !Self::can_case_be_inlined(first_case, ctx) { return; } let Some(mut case) = switch_stmt.cases.pop() else { @@ -255,7 +255,11 @@ impl<'a> PeepholeOptimizations { // } // } - pub fn can_case_be_inlined(case: &SwitchCase) -> bool { + pub fn can_case_be_inlined(case: &SwitchCase<'a>, ctx: &Ctx<'a, '_>) -> bool { + if case.test.as_ref().is_some_and(|test| test.may_have_side_effects(ctx)) { + return false; + } + let mut break_finder = BreakFinder::new(); break_finder.visit_switch_case(case); !break_finder.nested_unlabelled_break @@ -354,7 +358,7 @@ mod test { test("switch(a){default: b();break;case 1: c();break;}", "a === 1 ? c() : b()"); test("switch(a){default: {b();break;} case 1: {c();break;}}", "a === 1 ? c() : b()"); - test("switch(a){case b(): default:}", "a, b()"); + test_same("switch(a){case b(): default:}"); test( "switch(a){case 2: case 1: break; default: break;}", "switch(a){case 2: case 1: break; default: }", From aaf37dad4849d380199073bcdc3ad42c1066af7f Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 14:17:33 +0100 Subject: [PATCH 09/25] fix: correct generated ast --- apps/oxlint/src-js/generated/walk.js | 1 - napi/parser/src-js/generated/visit/walk.js | 1 - 2 files changed, 2 deletions(-) diff --git a/apps/oxlint/src-js/generated/walk.js b/apps/oxlint/src-js/generated/walk.js index 6bea75c377f89..b094ea8c90010 100644 --- a/apps/oxlint/src-js/generated/walk.js +++ b/apps/oxlint/src-js/generated/walk.js @@ -520,7 +520,6 @@ function walkNode(node, visitors) { break; case "TSUnionType": walkTSUnionType(node, visitors); - break; } } diff --git a/napi/parser/src-js/generated/visit/walk.js b/napi/parser/src-js/generated/visit/walk.js index 739ee1a4e6ece..243a00a17aebb 100644 --- a/napi/parser/src-js/generated/visit/walk.js +++ b/napi/parser/src-js/generated/visit/walk.js @@ -506,7 +506,6 @@ function walkNode(node, visitors) { break; case "TSUnionType": walkTSUnionType(node, visitors); - break; } } From da20e75804d2ac2cca5b6acda8f1f479dd1f7a0c Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 14:28:46 +0100 Subject: [PATCH 10/25] fix: correct tests --- crates/oxc_minifier/src/peephole/minimize_conditions.rs | 2 +- crates/oxc_minifier/tests/peephole/esbuild.rs | 4 ++-- crates/oxc_minifier/tests/peephole/statement_fusion.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_conditions.rs b/crates/oxc_minifier/src/peephole/minimize_conditions.rs index 532c584d898ce..4734f1902e0c4 100644 --- a/crates/oxc_minifier/src/peephole/minimize_conditions.rs +++ b/crates/oxc_minifier/src/peephole/minimize_conditions.rs @@ -335,7 +335,7 @@ mod test { ); test_same("function f(){foo()}"); - test("switch(x){case y: foo()}", "x === y && foo()"); + test_same("switch(x){case y: foo()}"); test( "try{foo()}catch(ex){bar()}finally{baz()}", "try { foo(); } catch { bar(); } finally { baz(); }", diff --git a/crates/oxc_minifier/tests/peephole/esbuild.rs b/crates/oxc_minifier/tests/peephole/esbuild.rs index bb262ae75d8cc..e1c1e69d32086 100644 --- a/crates/oxc_minifier/tests/peephole/esbuild.rs +++ b/crates/oxc_minifier/tests/peephole/esbuild.rs @@ -285,11 +285,11 @@ fn js_parser_test() { test("while(1) { async function* x() {} }", "for (;;) { async function* x() { }}"); test( "function _() { x(); switch (y) { case z: return w; } }", - "function _() { if (x(), y === z) return w; }", + "function _() { switch (x(), y) { case z: return w; }}", ); test( "function _() { if (t) { x(); switch (y) { case z: return w; } } }", - "function _() { if (t && (x(), y === z)) return w; }", + "function _() { if (t) switch (x(), y) { case z: return w; } }", ); test("a = '' + 0", "a = '0';"); test("a = 0 + ''", "a = '0';"); diff --git a/crates/oxc_minifier/tests/peephole/statement_fusion.rs b/crates/oxc_minifier/tests/peephole/statement_fusion.rs index 8b9aea942bcbb..df83ee51a5d08 100644 --- a/crates/oxc_minifier/tests/peephole/statement_fusion.rs +++ b/crates/oxc_minifier/tests/peephole/statement_fusion.rs @@ -94,7 +94,7 @@ fn fuse_into_block() { #[test] fn fuse_into_switch_cases() { - test("switch (_) {case 1: case _: a; return b}", "switch (_) {case 1: case _: return a, b;}"); + test("switch (_) { case _: a; return b }", "switch (_) { case _: return a, b }"); } #[test] From af30748de57d1b45f246591f0a0cb23976aafc7b Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 16:44:24 +0100 Subject: [PATCH 11/25] fix: update code to support fallthrough --- .../src/peephole/minimize_switch_statement.rs | 75 +++++++++++-------- tasks/minsize/minsize.snap | 2 +- 2 files changed, 43 insertions(+), 34 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 7f97c68bd4e40..53394335123cd 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -32,25 +32,27 @@ impl<'a> PeepholeOptimizations { cases: &Vec<'_, SwitchCase<'a>>, ctx: &Ctx<'a, '_>, ) -> Option<(usize, usize)> { - // TODO: this is not ideal, if there is default case that is empty, we can remove other cases even if they have break let case_count = cases.len(); if case_count <= 1 { return None; } // if a default case is last we can skip checking if it has body - let default_pos = if let Some(default_pos) = + let (end, allow_break) = if let Some(default_pos) = cases.iter().rposition(SwitchCase::is_default_case) - && default_pos == case_count - 1 { - case_count - 1 + if default_pos == case_count - 1 { + (case_count - 1, Self::is_empty_switch_case(&cases[default_pos].consequent, true)) + } else { + (case_count, false) + } } else { - case_count + (case_count, true) }; // Find the last non-removable case (any case whose consequent is non-empty). - let last_non_empty_before_last = cases[..default_pos].iter().rposition(|c| { - !c.consequent.is_empty() + let last_non_empty_before_last = cases[..end].iter().rposition(|c| { + !Self::is_empty_switch_case(&c.consequent, allow_break) || c.test.as_ref().is_none_or(|test| test.may_have_side_effects(ctx)) }); @@ -73,18 +75,17 @@ impl<'a> PeepholeOptimizations { return; }; - let Some((start, mut end)) = + let Some((start, end)) = Self::find_trailing_removable_switch_cases_range(&switch_stmt.cases, ctx) else { return; }; if let Some(last) = switch_stmt.cases.last_mut() - && !Self::is_empty_switch_case(&last.consequent) + && !Self::is_empty_switch_case(&last.consequent, false) { last.test = None; - end -= 1; - switch_stmt.cases.drain(start..end); + switch_stmt.cases.drain(start..end - 1); } else { switch_stmt.cases.truncate(start); } @@ -115,14 +116,13 @@ impl<'a> PeepholeOptimizations { if switch_stmt.cases.len() == 2 { // check whatever its default + case if switch_stmt.cases[0].test.is_some() == switch_stmt.cases[1].test.is_some() + || !Self::is_terminated_switch_case(&switch_stmt.cases[0].consequent) || !Self::can_case_be_inlined(&switch_stmt.cases[0], ctx) || !Self::can_case_be_inlined(&switch_stmt.cases[1], ctx) { return; } - // TODO: missing check for fallthrough - let mut first = switch_stmt.cases.pop().unwrap(); let mut second = switch_stmt.cases.pop().unwrap(); Self::remove_last_break(&mut first.consequent); @@ -198,7 +198,7 @@ impl<'a> PeepholeOptimizations { if discriminant.may_have_side_effects(ctx) { stmts.push(ctx.ast.statement_expression(discriminant.span(), discriminant)); } - if !case.consequent.is_empty() { + if !Self::is_empty_switch_case(&case.consequent, true) { stmts.extend(case.consequent); } *stmt = ctx.ast.statement_block_with_scope_id( @@ -210,12 +210,15 @@ impl<'a> PeepholeOptimizations { } } - fn is_empty_switch_case(stmt: &Vec) -> bool { + fn is_empty_switch_case(stmt: &Vec<'a, Statement<'a>>, allow_break: bool) -> bool { if stmt.len() != 1 { return stmt.is_empty(); } match stmt.last() { - Some(Statement::BlockStatement(block_stmt)) => block_stmt.body.is_empty(), + Some(Statement::BlockStatement(block_stmt)) => { + Self::is_empty_switch_case(&block_stmt.body, allow_break) + } + Some(Statement::BreakStatement(_)) => allow_break, _ => false, } } @@ -242,18 +245,23 @@ impl<'a> PeepholeOptimizations { changed } - // fn is_terminated(stmt: &Statement<'a>) -> bool { - // match stmt { - // Statement::BlockStatement(block_stmt) => { - // block_stmt.body.last().is_some_and(Self::is_terminated) - // } - // Statement::BreakStatement(_) - // | Statement::ContinueStatement(_) - // | Statement::ReturnStatement(_) - // | Statement::ThrowStatement(_) => true, - // _ => false, - // } - // } + fn is_terminated_switch_case(stmt: &Vec<'a, Statement<'a>>) -> bool { + if stmt.is_empty() { + return false; + } + match stmt.last() { + Some(Statement::BlockStatement(block_stmt)) => { + Self::is_terminated_switch_case(&block_stmt.body) + } + Some( + Statement::BreakStatement(_) + | Statement::ContinueStatement(_) + | Statement::ReturnStatement(_) + | Statement::ThrowStatement(_), + ) => true, + _ => false, + } + } pub fn can_case_be_inlined(case: &SwitchCase<'a>, ctx: &Ctx<'a, '_>) -> bool { if case.test.as_ref().is_some_and(|test| test.may_have_side_effects(ctx)) { @@ -342,6 +350,7 @@ mod test { test("switch(a){case 1: default: }", "a;"); test_same("switch(a){case 1: default: break; case 2: b()}"); + test_same("switch(a){case 1: b(); default: c()}"); test_same("switch(a){case 1: b(); default: break; case 2: c()}"); test_same("switch(a){case 1: b(); case 2: break; case 3: c()}"); test( @@ -351,7 +360,6 @@ mod test { test_same("switch(a){case 1: b(); case 2: b();}"); test("switch(a){case 1: var c=2; break;}", "if (a === 1) var c = 2"); test("switch(a){case 1: case 2: default: b(); break;}", "a, b()"); - test("switch(a){case 1: c(); case 2: default: b(); break;}", "a === 1 ? c() : b()"); test("switch(a){default: case 1: }", "a"); test("switch(a){default: break; case 1: break;}", "a"); @@ -359,13 +367,14 @@ mod test { test("switch(a){default: {b();break;} case 1: {c();break;}}", "a === 1 ? c() : b()"); test_same("switch(a){case b(): default:}"); - test( - "switch(a){case 2: case 1: break; default: break;}", - "switch(a){case 2: case 1: break; default: }", - ); + test("switch(a){case 2: case 1: break; default: break;}", "a;"); test("switch(a){case 3: b(); case 2: case 1: break;}", "a === 3 && b()"); test("switch(a){case 3: b(); case 2: case 1: }", "a === 3 && b()"); + test( + "switch(a){case 1: c(); case 2: default: b();break;}", + "switch(a){case 1: c(); default: b();}", + ); test("var x=1;switch(x){case 1: var y;}", "var y;"); test("function f(){switch(a){case 1: return;}}", "function f() {a;}"); test("x:switch(a){case 1: break x;}", "x: if (a === 1) break x;"); diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index a83ad0aa70be8..f6c93384ab5df 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -23,5 +23,5 @@ Original | minified | minified | gzip | gzip | Iterations | Fi 6.69 MB | 2.22 MB | 2.31 MB | 458.41 kB | 488.28 kB | 4 | antd.js -10.95 MB | 3.33 MB | 3.49 MB | 853.25 kB | 915.50 kB | 4 | typescript.js +10.95 MB | 3.33 MB | 3.49 MB | 853.26 kB | 915.50 kB | 4 | typescript.js From 478b11cebee292c7874a4cbd4424114f865af51c Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 17:32:14 +0100 Subject: [PATCH 12/25] fix: correct logic used to check removal range --- .../src/peephole/minimize_switch_statement.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 53394335123cd..a2f43db2ae6d0 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -51,9 +51,9 @@ impl<'a> PeepholeOptimizations { }; // Find the last non-removable case (any case whose consequent is non-empty). - let last_non_empty_before_last = cases[..end].iter().rposition(|c| { - !Self::is_empty_switch_case(&c.consequent, allow_break) - || c.test.as_ref().is_none_or(|test| test.may_have_side_effects(ctx)) + let last_non_empty_before_last = cases[..end].iter().rposition(|case| { + !Self::is_empty_switch_case(&case.consequent, allow_break) + || case.test.as_ref().is_some_and(|test| test.may_have_side_effects(ctx)) }); // start is the first index of the removable suffix @@ -348,7 +348,8 @@ mod test { test("switch(a){case 1: b();break; }", "a === 1 && b()"); test("switch(a){case 1: b();return; }", "if (a === 1) { b(); return; }"); - test("switch(a){case 1: default: }", "a;"); + test("switch(a){default: case 1: }", "a"); + test("switch(a){case 1: default: }", "a"); test_same("switch(a){case 1: default: break; case 2: b()}"); test_same("switch(a){case 1: b(); default: c()}"); test_same("switch(a){case 1: b(); default: break; case 2: c()}"); @@ -361,7 +362,6 @@ mod test { test("switch(a){case 1: var c=2; break;}", "if (a === 1) var c = 2"); test("switch(a){case 1: case 2: default: b(); break;}", "a, b()"); - test("switch(a){default: case 1: }", "a"); test("switch(a){default: break; case 1: break;}", "a"); test("switch(a){default: b();break;case 1: c();break;}", "a === 1 ? c() : b()"); test("switch(a){default: {b();break;} case 1: {c();break;}}", "a === 1 ? c() : b()"); @@ -395,7 +395,7 @@ mod test { "switch('r'){case 'r': a();break; case 'r': bar();break;}", "switch('r'){case 'r': a();break; case 'r': bar()}", ); - test("switch(2) {default: a; case 1: b()}", "a;"); + test_same("switch(2) {default: a; case 1: b()}"); test("switch(1) {case 1: a();break; default: b();}", "a()"); test_same("switch('e') {case 'e': case 'f': a();}"); test( From 76c7b3985fc35621e5d42c2bbd7d4ef4f5f283d3 Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 19:20:00 +0100 Subject: [PATCH 13/25] fix: cleanup code and add comments --- .../src/peephole/minimize_switch_statement.rs | 132 +++++++++--------- 1 file changed, 68 insertions(+), 64 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index a2f43db2ae6d0..42c1c4675821a 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -16,6 +16,7 @@ impl<'a> PeepholeOptimizations { Self::fold_switch_with_two_cases(stmt, ctx); } + /// Attempts to remove the last `break` statement from the last case of a switch statement. fn try_remove_last_break_from_case(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { let Statement::SwitchStatement(switch_stmt) = stmt else { return; @@ -28,21 +29,35 @@ impl<'a> PeepholeOptimizations { } } - fn find_trailing_removable_switch_cases_range( - cases: &Vec<'_, SwitchCase<'a>>, - ctx: &Ctx<'a, '_>, - ) -> Option<(usize, usize)> { - let case_count = cases.len(); + /// Collapses empty cases in a `SwitchStatement` by removing redundant cases with empty + /// consequent's and consolidating them into a more concise representation. + /// + /// - If the switch statement contains one or fewer cases, it is considered already optimal, and no actions are taken. + /// - If the `default` case is the last case, it is treated as a special case where its emptiness directly + /// influences the analysis of the rest of the cases. + /// - The function identifies a `removable suffix` of cases at the end of the statement, starting from the first + /// non-empty case or case with side-effect-producing expressions backward to the last case. + /// - All cases in the identified removable suffix are eliminated, except for the last case, + /// which is preserved and its test is removed (if applicable). + fn collapse_empty_switch_cases(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + + let case_count = switch_stmt.cases.len(); if case_count <= 1 { - return None; + return; } // if a default case is last we can skip checking if it has body let (end, allow_break) = if let Some(default_pos) = - cases.iter().rposition(SwitchCase::is_default_case) + switch_stmt.cases.iter().rposition(SwitchCase::is_default_case) { if default_pos == case_count - 1 { - (case_count - 1, Self::is_empty_switch_case(&cases[default_pos].consequent, true)) + ( + case_count - 1, + Self::is_empty_switch_case(&switch_stmt.cases[default_pos].consequent, true), + ) } else { (case_count, false) } @@ -51,7 +66,7 @@ impl<'a> PeepholeOptimizations { }; // Find the last non-removable case (any case whose consequent is non-empty). - let last_non_empty_before_last = cases[..end].iter().rposition(|case| { + let last_non_empty_before_last = switch_stmt.cases[..end].iter().rposition(|case| { !Self::is_empty_switch_case(&case.consequent, allow_break) || case.test.as_ref().is_some_and(|test| test.may_have_side_effects(ctx)) }); @@ -64,30 +79,17 @@ impl<'a> PeepholeOptimizations { // nothing removable if start >= case_count - 1 { - return None; - } - - Some((start, case_count)) - } - - fn collapse_empty_switch_cases(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { - let Statement::SwitchStatement(switch_stmt) = stmt else { return; - }; + } - let Some((start, end)) = - Self::find_trailing_removable_switch_cases_range(&switch_stmt.cases, ctx) - else { + let Some(mut last) = switch_stmt.cases.pop() else { return; }; + switch_stmt.cases.truncate(start); - if let Some(last) = switch_stmt.cases.last_mut() - && !Self::is_empty_switch_case(&last.consequent, false) - { + if !Self::is_empty_switch_case(&last.consequent, true) { last.test = None; - switch_stmt.cases.drain(start..end - 1); - } else { - switch_stmt.cases.truncate(start); + switch_stmt.cases.push(last); } ctx.state.changed = true; } @@ -109,44 +111,51 @@ impl<'a> PeepholeOptimizations { } } + /// Simplifies a `switch` statement with exactly two cases into an equivalent `if` statement. + /// + /// This transformation is applicable when the `switch` statement meets the following criteria: + /// - It contains exactly two cases. + /// - One of the cases represents the `default` case, and the other defines a condition (`test`). + /// - Both cases can be safely inlined without reordering or modifying program behavior. + /// - Both cases are terminated properly (e.g., with a `break` statement). fn fold_switch_with_two_cases(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { let Statement::SwitchStatement(switch_stmt) = stmt else { return; }; - if switch_stmt.cases.len() == 2 { - // check whatever its default + case - if switch_stmt.cases[0].test.is_some() == switch_stmt.cases[1].test.is_some() - || !Self::is_terminated_switch_case(&switch_stmt.cases[0].consequent) - || !Self::can_case_be_inlined(&switch_stmt.cases[0], ctx) - || !Self::can_case_be_inlined(&switch_stmt.cases[1], ctx) - { - return; - } - let mut first = switch_stmt.cases.pop().unwrap(); - let mut second = switch_stmt.cases.pop().unwrap(); - Self::remove_last_break(&mut first.consequent); - Self::remove_last_break(&mut second.consequent); + // check whatever its default + case + if switch_stmt.cases.len() != 2 + || switch_stmt.cases[0].test.is_some() == switch_stmt.cases[1].test.is_some() + || !Self::is_terminated_switch_case(&switch_stmt.cases[0].consequent) + || !Self::can_case_be_inlined(&switch_stmt.cases[0], ctx) + || !Self::can_case_be_inlined(&switch_stmt.cases[1], ctx) + { + return; + } - let (test, consequent, alternate) = if first.test.is_some() { - (first.test.unwrap(), first.consequent, second.consequent) - } else { - (second.test.unwrap(), second.consequent, first.consequent) - }; + let mut first = switch_stmt.cases.pop().unwrap(); + let mut second = switch_stmt.cases.pop().unwrap(); + Self::remove_last_break(&mut first.consequent); + Self::remove_last_break(&mut second.consequent); - ctx.state.changed = true; - *stmt = ctx.ast.statement_if( - switch_stmt.span, - ctx.ast.expression_binary( - SPAN, - switch_stmt.discriminant.take_in(ctx.ast), - BinaryOperator::StrictEquality, - test, - ), - Self::create_if_block_from_switch_case(consequent, ctx), - Some(Self::create_if_block_from_switch_case(alternate, ctx)), - ); - } + let (test, consequent, alternate) = if first.test.is_some() { + (first.test.unwrap(), first.consequent, second.consequent) + } else { + (second.test.unwrap(), second.consequent, first.consequent) + }; + + ctx.state.changed = true; + *stmt = ctx.ast.statement_if( + switch_stmt.span, + ctx.ast.expression_binary( + SPAN, + switch_stmt.discriminant.take_in(ctx.ast), + BinaryOperator::StrictEquality, + test, + ), + Self::create_if_block_from_switch_case(consequent, ctx), + Some(Self::create_if_block_from_switch_case(alternate, ctx)), + ); } fn create_if_block_from_switch_case( @@ -253,12 +262,7 @@ impl<'a> PeepholeOptimizations { Some(Statement::BlockStatement(block_stmt)) => { Self::is_terminated_switch_case(&block_stmt.body) } - Some( - Statement::BreakStatement(_) - | Statement::ContinueStatement(_) - | Statement::ReturnStatement(_) - | Statement::ThrowStatement(_), - ) => true, + Some(last) => last.is_jump_statement(), _ => false, } } From 6db0a6fe28e2a6c8ea504e60a434ebc6586764da Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 20:18:09 +0100 Subject: [PATCH 14/25] fix: correct issue with inlining break with label --- .../src/peephole/minimize_switch_statement.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 42c1c4675821a..3a1b60c2b57ac 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -227,7 +227,12 @@ impl<'a> PeepholeOptimizations { Some(Statement::BlockStatement(block_stmt)) => { Self::is_empty_switch_case(&block_stmt.body, allow_break) } - Some(Statement::BreakStatement(_)) => allow_break, + Some(Statement::BreakStatement(break_stmt)) => { + if break_stmt.label.is_none() { + return allow_break; + } + false + } _ => false, } } @@ -382,6 +387,7 @@ mod test { test("var x=1;switch(x){case 1: var y;}", "var y;"); test("function f(){switch(a){case 1: return;}}", "function f() {a;}"); test("x:switch(a){case 1: break x;}", "x: if (a === 1) break x;"); + test_same("x:switch(a){case 2: break x; case 1: break x;}"); test("switch(a()) { default: {let y;} }", "a();{let y;}"); test( "function f(){switch('x'){case 'x': var x = 1;break; case 'y': break; }}", From a31e9bf1847f80cd1dd27a50128127ef53243f00 Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 20:20:42 +0100 Subject: [PATCH 15/25] fix: cleanup --- .../oxc_minifier/src/peephole/minimize_switch_statement.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 3a1b60c2b57ac..25cb6a7b6d5e9 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -228,10 +228,7 @@ impl<'a> PeepholeOptimizations { Self::is_empty_switch_case(&block_stmt.body, allow_break) } Some(Statement::BreakStatement(break_stmt)) => { - if break_stmt.label.is_none() { - return allow_break; - } - false + break_stmt.label.is_none() && allow_break } _ => false, } From af1e3362a71f5db7939a8d0f601d0dd2a464eed2 Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 22:07:20 +0100 Subject: [PATCH 16/25] fix: apply changes from code review and add documentation --- .../src/peephole/minimize_switch_statement.rs | 75 +++++++++++-------- 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 25cb6a7b6d5e9..d20d569d44860 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -8,6 +8,11 @@ use oxc_span::{GetSpan, SPAN}; use oxc_syntax::{operator::BinaryOperator, scope::ScopeFlags}; impl<'a> PeepholeOptimizations { + /// Attempts to minimize a `switch` statement by applying a series of transformations + /// - Removes the trailing `break` statement from the last case of the `switch`, if it's unnecessary. + /// - Merges or removes consecutive empty cases within the switch to simplify its structure. + /// - Eliminates the entire `switch` statement if it contains no meaningful cases or logic. + /// - Converts the `switch` if it contains only one or two cases to `if`/`else` statements. pub fn try_minimize_switch(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { Self::try_remove_last_break_from_case(stmt, ctx); Self::collapse_empty_switch_cases(stmt, ctx); @@ -94,6 +99,7 @@ impl<'a> PeepholeOptimizations { ctx.state.changed = true; } + /// Removes an empty switch statement from the given AST statement. fn remove_empty_switch(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { let Statement::SwitchStatement(switch_stmt) = stmt else { return; @@ -238,22 +244,22 @@ impl<'a> PeepholeOptimizations { if stmt.is_empty() { return false; } - let mut changed = false; let len = stmt.len(); match stmt.last_mut() { Some(Statement::BreakStatement(break_stmt)) => { if break_stmt.label.is_none() { stmt.truncate(len - 1); - changed |= true; + true + } else { + false } } Some(Statement::BlockStatement(block_stmt)) => { - changed |= Self::remove_last_break(&mut block_stmt.body); + Self::remove_last_break(&mut block_stmt.body) } - _ => {} + _ => false, } - changed } fn is_terminated_switch_case(stmt: &Vec<'a, Statement<'a>>) -> bool { @@ -293,8 +299,7 @@ impl BreakFinder { impl<'a> Visit<'a> for BreakFinder { fn visit_statement(&mut self, it: &Statement<'a>) { - // TODO: check if there we have access to control flow instead?? - // Only visit blocks where vars could be hoisted + // TODO: This is to aggressive, we should allow `break` for last statements match it { Statement::BlockStatement(it) => self.visit_block_statement(it), Statement::BreakStatement(it) => { @@ -302,33 +307,23 @@ impl<'a> Visit<'a> for BreakFinder { self.nested_unlabelled_break = true; } } - // TODO: this is not fully correct, we should allow termination if this if is a last statement Statement::IfStatement(it) => { - if self.top_level { - self.top_level = false; - self.visit_if_statement(it); - self.top_level = true; - } else { - self.visit_if_statement(it); - } + let was_top = self.top_level; + self.top_level = false; + self.visit_if_statement(it); + self.top_level = was_top; } Statement::TryStatement(it) => { - if self.top_level { - self.top_level = false; - self.visit_try_statement(it); - self.top_level = true; - } else { - self.visit_try_statement(it); - } + let was_top = self.top_level; + self.top_level = false; + self.visit_try_statement(it); + self.top_level = was_top; } Statement::WithStatement(it) => { - if self.top_level { - self.top_level = false; - self.visit_with_statement(it); - self.top_level = true; - } else { - self.visit_with_statement(it); - } + let was_top = self.top_level; + self.top_level = false; + self.visit_with_statement(it); + self.top_level = was_top; } _ => {} } @@ -376,6 +371,16 @@ mod test { test("switch(a){case 2: case 1: break; default: break;}", "a;"); test("switch(a){case 3: b(); case 2: case 1: break;}", "a === 3 && b()"); test("switch(a){case 3: b(); case 2: case 1: }", "a === 3 && b()"); + test_same("switch(a){case 3: if (b) break }"); + test("switch(a){case 3: { if (b) break } }", "switch(a){case 3: if (b) break;}"); + test( + "switch(a){case 3: {if(b) {c()} else {break;}}}", + "switch(a){case 3: if (b) c();else break;}", + ); + test("switch(a){case 3: { for (;;) break } }", "if(a === 3) for (;;) break;"); + test("switch(a){case 3: { for (b of c) break } }", "if (a === 3) for (b of c) break;"); + test_same("switch(a){case 3: with(b) break}"); + test("switch(a){case 3: while(!0) break}", "if (a === 3) for (;;) break;"); test( "switch(a){case 1: c(); case 2: default: b();break;}", @@ -383,8 +388,6 @@ mod test { ); test("var x=1;switch(x){case 1: var y;}", "var y;"); test("function f(){switch(a){case 1: return;}}", "function f() {a;}"); - test("x:switch(a){case 1: break x;}", "x: if (a === 1) break x;"); - test_same("x:switch(a){case 2: break x; case 1: break x;}"); test("switch(a()) { default: {let y;} }", "a();{let y;}"); test( "function f(){switch('x'){case 'x': var x = 1;break; case 'y': break; }}", @@ -394,6 +397,15 @@ mod test { test("switch(a){case 1: if(a) {b();}c();}", "a === 1 && (a && b(), c())"); test("switch ('\\v') {case '\\u000B': foo();}", "foo()"); + test("x: switch(a){case 1: break x;}", "x: if (a === 1) break x;"); + test_same("x: switch(a){case 2: break x; case 1: break x;}"); + test("x: switch(2){case 2: f(); break outer; }", "x: {f(); break outer;}"); + test( + "x: switch(x){case 2: f(); for (;;){break outer;}}", + "x: if(x===2) for(f();;) break outer", + ); + test("x: switch(a){case 2: if(b) { break outer; } }", "x: if(a===2 && b) break outer;"); + test( "switch('r'){case 'r': a();break; case 'r': var x=0;break;}", "switch('r'){case 'r': a();break; case 'r': var x=0;}", @@ -428,7 +440,6 @@ mod test { "switch(1){case 1: case 2: break; default: b();}", ); test("switch ('d') {case 'foo': foo();break; default: bar();break;}", "bar()"); - test("outer: switch (2) {case 2: f(); break outer; }", "outer: {f(); break outer;}"); test( "switch(0){case NaN: foobar();break;case -0.0: foo();break; case 2: bar();break;}", "switch(0){case NaN: foobar();break;case -0.0: foo();break; case 2: bar();}", From 2be1b7cfb039ea11238accd2eb063e7d6b09493a Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 23:03:08 +0100 Subject: [PATCH 17/25] fix: update BreakFinder based on review request --- .../src/peephole/minimize_switch_statement.rs | 66 +++++++++++-------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index d20d569d44860..e248cd1f9df19 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -1,8 +1,8 @@ use super::PeepholeOptimizations; use crate::ctx::Ctx; use oxc_allocator::{TakeIn, Vec}; -use oxc_ast::ast::{Statement, SwitchCase}; -use oxc_ast_visit::Visit; +use oxc_ast::ast::*; +use oxc_ast_visit::{walk, Visit}; use oxc_ecmascript::side_effects::MayHaveSideEffects; use oxc_span::{GetSpan, SPAN}; use oxc_syntax::{operator::BinaryOperator, scope::ScopeFlags}; @@ -297,37 +297,47 @@ impl BreakFinder { } } +// TODO: This is to aggressive, we should allow `break` for last elements in statements impl<'a> Visit<'a> for BreakFinder { fn visit_statement(&mut self, it: &Statement<'a>) { - // TODO: This is to aggressive, we should allow `break` for last statements match it { - Statement::BlockStatement(it) => self.visit_block_statement(it), - Statement::BreakStatement(it) => { - if !self.top_level && it.label.is_none() { - self.nested_unlabelled_break = true; - } - } - Statement::IfStatement(it) => { - let was_top = self.top_level; - self.top_level = false; - self.visit_if_statement(it); - self.top_level = was_top; - } - Statement::TryStatement(it) => { - let was_top = self.top_level; - self.top_level = false; - self.visit_try_statement(it); - self.top_level = was_top; - } - Statement::WithStatement(it) => { - let was_top = self.top_level; - self.top_level = false; - self.visit_with_statement(it); - self.top_level = was_top; - } - _ => {} + Statement::ForInStatement(_) + | Statement::ForOfStatement(_) + | Statement::WhileStatement(_) + | Statement::ForStatement(_) + | Statement::ReturnStatement(_) + | Statement::ThrowStatement(_) + | Statement::ExpressionStatement(_) => {} + _ => walk::walk_statement(self, it), + } + } + + fn visit_if_statement(&mut self, it: &IfStatement<'a>) { + let was_top = self.top_level; + self.top_level = false; + walk::walk_if_statement(self, it); + self.top_level = was_top; + } + + fn visit_break_statement(&mut self, it: &BreakStatement<'a>) { + if !self.top_level && it.label.is_none() { + self.nested_unlabelled_break = true; } } + + fn visit_with_statement(&mut self, it: &WithStatement<'a>) { + let was_top = self.top_level; + self.top_level = false; + walk::walk_with_statement(self, it); + self.top_level = was_top; + } + + fn visit_try_statement(&mut self, it: &TryStatement<'a>) { + let was_top = self.top_level; + self.top_level = false; + walk::walk_try_statement(self, it); + self.top_level = was_top; + } } #[cfg(test)] From 806ef4afbcedb69672d1c5ba860d8d0016aba386 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 31 Dec 2025 22:04:41 +0000 Subject: [PATCH 18/25] [autofix.ci] apply automated fixes --- .../src/peephole/minimize_switch_statement.rs | 920 +++++++++--------- 1 file changed, 460 insertions(+), 460 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index e248cd1f9df19..ec53687a3a001 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -1,460 +1,460 @@ -use super::PeepholeOptimizations; -use crate::ctx::Ctx; -use oxc_allocator::{TakeIn, Vec}; -use oxc_ast::ast::*; -use oxc_ast_visit::{walk, Visit}; -use oxc_ecmascript::side_effects::MayHaveSideEffects; -use oxc_span::{GetSpan, SPAN}; -use oxc_syntax::{operator::BinaryOperator, scope::ScopeFlags}; - -impl<'a> PeepholeOptimizations { - /// Attempts to minimize a `switch` statement by applying a series of transformations - /// - Removes the trailing `break` statement from the last case of the `switch`, if it's unnecessary. - /// - Merges or removes consecutive empty cases within the switch to simplify its structure. - /// - Eliminates the entire `switch` statement if it contains no meaningful cases or logic. - /// - Converts the `switch` if it contains only one or two cases to `if`/`else` statements. - pub fn try_minimize_switch(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { - Self::try_remove_last_break_from_case(stmt, ctx); - Self::collapse_empty_switch_cases(stmt, ctx); - Self::remove_empty_switch(stmt, ctx); - Self::fold_switch_with_one_case(stmt, ctx); - Self::fold_switch_with_two_cases(stmt, ctx); - } - - /// Attempts to remove the last `break` statement from the last case of a switch statement. - fn try_remove_last_break_from_case(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { - let Statement::SwitchStatement(switch_stmt) = stmt else { - return; - }; - - if let Some(last_case) = switch_stmt.cases.last_mut() - && Self::remove_last_break(&mut last_case.consequent) - { - ctx.state.changed = true; - } - } - - /// Collapses empty cases in a `SwitchStatement` by removing redundant cases with empty - /// consequent's and consolidating them into a more concise representation. - /// - /// - If the switch statement contains one or fewer cases, it is considered already optimal, and no actions are taken. - /// - If the `default` case is the last case, it is treated as a special case where its emptiness directly - /// influences the analysis of the rest of the cases. - /// - The function identifies a `removable suffix` of cases at the end of the statement, starting from the first - /// non-empty case or case with side-effect-producing expressions backward to the last case. - /// - All cases in the identified removable suffix are eliminated, except for the last case, - /// which is preserved and its test is removed (if applicable). - fn collapse_empty_switch_cases(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { - let Statement::SwitchStatement(switch_stmt) = stmt else { - return; - }; - - let case_count = switch_stmt.cases.len(); - if case_count <= 1 { - return; - } - - // if a default case is last we can skip checking if it has body - let (end, allow_break) = if let Some(default_pos) = - switch_stmt.cases.iter().rposition(SwitchCase::is_default_case) - { - if default_pos == case_count - 1 { - ( - case_count - 1, - Self::is_empty_switch_case(&switch_stmt.cases[default_pos].consequent, true), - ) - } else { - (case_count, false) - } - } else { - (case_count, true) - }; - - // Find the last non-removable case (any case whose consequent is non-empty). - let last_non_empty_before_last = switch_stmt.cases[..end].iter().rposition(|case| { - !Self::is_empty_switch_case(&case.consequent, allow_break) - || case.test.as_ref().is_some_and(|test| test.may_have_side_effects(ctx)) - }); - - // start is the first index of the removable suffix - let start = match last_non_empty_before_last { - Some(pos) => pos + 1, - None => 0, - }; - - // nothing removable - if start >= case_count - 1 { - return; - } - - let Some(mut last) = switch_stmt.cases.pop() else { - return; - }; - switch_stmt.cases.truncate(start); - - if !Self::is_empty_switch_case(&last.consequent, true) { - last.test = None; - switch_stmt.cases.push(last); - } - ctx.state.changed = true; - } - - /// Removes an empty switch statement from the given AST statement. - fn remove_empty_switch(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { - let Statement::SwitchStatement(switch_stmt) = stmt else { - return; - }; - if switch_stmt.cases.is_empty() { - if switch_stmt.discriminant.may_have_side_effects(ctx) { - *stmt = ctx.ast.statement_expression( - switch_stmt.span, - switch_stmt.discriminant.take_in(ctx.ast), - ); - } else { - *stmt = ctx.ast.statement_empty(switch_stmt.span); - } - ctx.state.changed = true; - } - } - - /// Simplifies a `switch` statement with exactly two cases into an equivalent `if` statement. - /// - /// This transformation is applicable when the `switch` statement meets the following criteria: - /// - It contains exactly two cases. - /// - One of the cases represents the `default` case, and the other defines a condition (`test`). - /// - Both cases can be safely inlined without reordering or modifying program behavior. - /// - Both cases are terminated properly (e.g., with a `break` statement). - fn fold_switch_with_two_cases(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { - let Statement::SwitchStatement(switch_stmt) = stmt else { - return; - }; - - // check whatever its default + case - if switch_stmt.cases.len() != 2 - || switch_stmt.cases[0].test.is_some() == switch_stmt.cases[1].test.is_some() - || !Self::is_terminated_switch_case(&switch_stmt.cases[0].consequent) - || !Self::can_case_be_inlined(&switch_stmt.cases[0], ctx) - || !Self::can_case_be_inlined(&switch_stmt.cases[1], ctx) - { - return; - } - - let mut first = switch_stmt.cases.pop().unwrap(); - let mut second = switch_stmt.cases.pop().unwrap(); - Self::remove_last_break(&mut first.consequent); - Self::remove_last_break(&mut second.consequent); - - let (test, consequent, alternate) = if first.test.is_some() { - (first.test.unwrap(), first.consequent, second.consequent) - } else { - (second.test.unwrap(), second.consequent, first.consequent) - }; - - ctx.state.changed = true; - *stmt = ctx.ast.statement_if( - switch_stmt.span, - ctx.ast.expression_binary( - SPAN, - switch_stmt.discriminant.take_in(ctx.ast), - BinaryOperator::StrictEquality, - test, - ), - Self::create_if_block_from_switch_case(consequent, ctx), - Some(Self::create_if_block_from_switch_case(alternate, ctx)), - ); - } - - fn create_if_block_from_switch_case( - mut vec: Vec<'a, Statement<'a>>, - ctx: &mut Ctx<'a, '_>, - ) -> Statement<'a> { - if vec.len() == 1 && matches!(vec.first(), Some(Statement::BlockStatement(_))) { - vec.pop().unwrap() - } else { - ctx.ast.statement_block_with_scope_id( - SPAN, - vec, - ctx.create_child_scope_of_current(ScopeFlags::empty()), - ) - } - } - - fn fold_switch_with_one_case(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { - let Statement::SwitchStatement(switch_stmt) = stmt else { - return; - }; - if switch_stmt.cases.len() == 1 { - let Some(first_case) = switch_stmt.cases.first() else { return }; - if !Self::can_case_be_inlined(first_case, ctx) { - return; - } - let Some(mut case) = switch_stmt.cases.pop() else { - return; - }; - - ctx.state.changed = true; - let discriminant = switch_stmt.discriminant.take_in(ctx.ast); - Self::remove_last_break(&mut case.consequent); - - if let Some(test) = case.test { - *stmt = ctx.ast.statement_if( - switch_stmt.span, - ctx.ast.expression_binary( - SPAN, - discriminant, - BinaryOperator::StrictEquality, - test, - ), - Self::create_if_block_from_switch_case(case.consequent, ctx), - None, - ); - } else { - let mut stmts = ctx.ast.vec(); - if discriminant.may_have_side_effects(ctx) { - stmts.push(ctx.ast.statement_expression(discriminant.span(), discriminant)); - } - if !Self::is_empty_switch_case(&case.consequent, true) { - stmts.extend(case.consequent); - } - *stmt = ctx.ast.statement_block_with_scope_id( - switch_stmt.span, - stmts, - ctx.create_child_scope_of_current(ScopeFlags::empty()), - ); - } - } - } - - fn is_empty_switch_case(stmt: &Vec<'a, Statement<'a>>, allow_break: bool) -> bool { - if stmt.len() != 1 { - return stmt.is_empty(); - } - match stmt.last() { - Some(Statement::BlockStatement(block_stmt)) => { - Self::is_empty_switch_case(&block_stmt.body, allow_break) - } - Some(Statement::BreakStatement(break_stmt)) => { - break_stmt.label.is_none() && allow_break - } - _ => false, - } - } - - fn remove_last_break(stmt: &mut Vec<'a, Statement<'a>>) -> bool { - if stmt.is_empty() { - return false; - } - - let len = stmt.len(); - match stmt.last_mut() { - Some(Statement::BreakStatement(break_stmt)) => { - if break_stmt.label.is_none() { - stmt.truncate(len - 1); - true - } else { - false - } - } - Some(Statement::BlockStatement(block_stmt)) => { - Self::remove_last_break(&mut block_stmt.body) - } - _ => false, - } - } - - fn is_terminated_switch_case(stmt: &Vec<'a, Statement<'a>>) -> bool { - if stmt.is_empty() { - return false; - } - match stmt.last() { - Some(Statement::BlockStatement(block_stmt)) => { - Self::is_terminated_switch_case(&block_stmt.body) - } - Some(last) => last.is_jump_statement(), - _ => false, - } - } - - pub fn can_case_be_inlined(case: &SwitchCase<'a>, ctx: &Ctx<'a, '_>) -> bool { - if case.test.as_ref().is_some_and(|test| test.may_have_side_effects(ctx)) { - return false; - } - - let mut break_finder = BreakFinder::new(); - break_finder.visit_switch_case(case); - !break_finder.nested_unlabelled_break - } -} - -struct BreakFinder { - top_level: bool, - nested_unlabelled_break: bool, -} - -impl BreakFinder { - pub fn new() -> Self { - Self { top_level: true, nested_unlabelled_break: false } - } -} - -// TODO: This is to aggressive, we should allow `break` for last elements in statements -impl<'a> Visit<'a> for BreakFinder { - fn visit_statement(&mut self, it: &Statement<'a>) { - match it { - Statement::ForInStatement(_) - | Statement::ForOfStatement(_) - | Statement::WhileStatement(_) - | Statement::ForStatement(_) - | Statement::ReturnStatement(_) - | Statement::ThrowStatement(_) - | Statement::ExpressionStatement(_) => {} - _ => walk::walk_statement(self, it), - } - } - - fn visit_if_statement(&mut self, it: &IfStatement<'a>) { - let was_top = self.top_level; - self.top_level = false; - walk::walk_if_statement(self, it); - self.top_level = was_top; - } - - fn visit_break_statement(&mut self, it: &BreakStatement<'a>) { - if !self.top_level && it.label.is_none() { - self.nested_unlabelled_break = true; - } - } - - fn visit_with_statement(&mut self, it: &WithStatement<'a>) { - let was_top = self.top_level; - self.top_level = false; - walk::walk_with_statement(self, it); - self.top_level = was_top; - } - - fn visit_try_statement(&mut self, it: &TryStatement<'a>) { - let was_top = self.top_level; - self.top_level = false; - walk::walk_try_statement(self, it); - self.top_level = was_top; - } -} - -#[cfg(test)] -mod test { - use crate::tester::{test, test_same}; - - #[expect(clippy::literal_string_with_formatting_args)] - #[test] - fn minimize_switch() { - test("switch(a()){}", "a()"); - test("switch(a){default: }", "a;"); - test("switch(a){default: break;}", "a;"); - test("switch(a){default: var b; break;}", "a;var b"); - test("switch(a){default: b()}", "a, b();"); - test("switch(a){default: b(); return;}", "a, b(); return"); - - test("switch(a){case 1: break;}", "a;"); - test("switch(a){case 1: b();}", "a === 1 && b()"); - test("switch(a){case 1: b();break; }", "a === 1 && b()"); - test("switch(a){case 1: b();return; }", "if (a === 1) { b(); return; }"); - - test("switch(a){default: case 1: }", "a"); - test("switch(a){case 1: default: }", "a"); - test_same("switch(a){case 1: default: break; case 2: b()}"); - test_same("switch(a){case 1: b(); default: c()}"); - test_same("switch(a){case 1: b(); default: break; case 2: c()}"); - test_same("switch(a){case 1: b(); case 2: break; case 3: c()}"); - test( - "switch(a){case 1: b(); break; case 2: c();break;}", - "switch(a){case 1: b(); break; case 2: c();}", - ); - test_same("switch(a){case 1: b(); case 2: b();}"); - test("switch(a){case 1: var c=2; break;}", "if (a === 1) var c = 2"); - test("switch(a){case 1: case 2: default: b(); break;}", "a, b()"); - - test("switch(a){default: break; case 1: break;}", "a"); - test("switch(a){default: b();break;case 1: c();break;}", "a === 1 ? c() : b()"); - test("switch(a){default: {b();break;} case 1: {c();break;}}", "a === 1 ? c() : b()"); - - test_same("switch(a){case b(): default:}"); - test("switch(a){case 2: case 1: break; default: break;}", "a;"); - test("switch(a){case 3: b(); case 2: case 1: break;}", "a === 3 && b()"); - test("switch(a){case 3: b(); case 2: case 1: }", "a === 3 && b()"); - test_same("switch(a){case 3: if (b) break }"); - test("switch(a){case 3: { if (b) break } }", "switch(a){case 3: if (b) break;}"); - test( - "switch(a){case 3: {if(b) {c()} else {break;}}}", - "switch(a){case 3: if (b) c();else break;}", - ); - test("switch(a){case 3: { for (;;) break } }", "if(a === 3) for (;;) break;"); - test("switch(a){case 3: { for (b of c) break } }", "if (a === 3) for (b of c) break;"); - test_same("switch(a){case 3: with(b) break}"); - test("switch(a){case 3: while(!0) break}", "if (a === 3) for (;;) break;"); - - test( - "switch(a){case 1: c(); case 2: default: b();break;}", - "switch(a){case 1: c(); default: b();}", - ); - test("var x=1;switch(x){case 1: var y;}", "var y;"); - test("function f(){switch(a){case 1: return;}}", "function f() {a;}"); - test("switch(a()) { default: {let y;} }", "a();{let y;}"); - test( - "function f(){switch('x'){case 'x': var x = 1;break; case 'y': break; }}", - "function f(){switch('x'){case 'x': var x = 1;break; case 'y': }}", - ); - test("switch(a){default: if(a) {break;}c();}", "switch(a){default: if(a) break;c();}"); - test("switch(a){case 1: if(a) {b();}c();}", "a === 1 && (a && b(), c())"); - test("switch ('\\v') {case '\\u000B': foo();}", "foo()"); - - test("x: switch(a){case 1: break x;}", "x: if (a === 1) break x;"); - test_same("x: switch(a){case 2: break x; case 1: break x;}"); - test("x: switch(2){case 2: f(); break outer; }", "x: {f(); break outer;}"); - test( - "x: switch(x){case 2: f(); for (;;){break outer;}}", - "x: if(x===2) for(f();;) break outer", - ); - test("x: switch(a){case 2: if(b) { break outer; } }", "x: if(a===2 && b) break outer;"); - - test( - "switch('r'){case 'r': a();break; case 'r': var x=0;break;}", - "switch('r'){case 'r': a();break; case 'r': var x=0;}", - ); - test( - "switch('r'){case 'r': a();break; case 'r': bar();break;}", - "switch('r'){case 'r': a();break; case 'r': bar()}", - ); - test_same("switch(2) {default: a; case 1: b()}"); - test("switch(1) {case 1: a();break; default: b();}", "a()"); - test_same("switch('e') {case 'e': case 'f': a();}"); - test( - "switch('a') {case 'a': a();break; case 'b': b();break;}", - "switch('a') {case 'a': a();break; case 'b': b();}", - ); - test( - "switch('c') {case 'a': a();break; case 'b': b();break;}", - "switch('c') {case 'a': a();break; case 'b': b();}", - ); - test( - "switch(1) {case 1: a();break; case 2: bar();break;}", - "switch(1) {case 1: a();break; case 2: bar();}", - ); - test_same("switch('f') {case 'f': a(); case 'b': b();}"); - test_same("switch('f') {case 'f': if (a() > 0) {b();break;} c(); case 'd': f();}"); - test( - "switch('f') {case 'b': bar();break; case x: x();break; case 'f': f();break;}", - "switch('f') {case 'b': bar();break; case x: x();break; case 'f': f();}", - ); - test( - "switch(1){case 1: case 2: {break;} case 3: case 4: default: b(); break;}", - "switch(1){case 1: case 2: break; default: b();}", - ); - test("switch ('d') {case 'foo': foo();break; default: bar();break;}", "bar()"); - test( - "switch(0){case NaN: foobar();break;case -0.0: foo();break; case 2: bar();break;}", - "switch(0){case NaN: foobar();break;case -0.0: foo();break; case 2: bar();}", - ); - test("let x = 1; switch('x') { case 'x': let x = 2; break;}", "let x = 1; { let x = 2 }"); - test("switch(1){case 2: var x=0;}", "if (0) var x;"); - } -} +use super::PeepholeOptimizations; +use crate::ctx::Ctx; +use oxc_allocator::{TakeIn, Vec}; +use oxc_ast::ast::*; +use oxc_ast_visit::{Visit, walk}; +use oxc_ecmascript::side_effects::MayHaveSideEffects; +use oxc_span::{GetSpan, SPAN}; +use oxc_syntax::{operator::BinaryOperator, scope::ScopeFlags}; + +impl<'a> PeepholeOptimizations { + /// Attempts to minimize a `switch` statement by applying a series of transformations + /// - Removes the trailing `break` statement from the last case of the `switch`, if it's unnecessary. + /// - Merges or removes consecutive empty cases within the switch to simplify its structure. + /// - Eliminates the entire `switch` statement if it contains no meaningful cases or logic. + /// - Converts the `switch` if it contains only one or two cases to `if`/`else` statements. + pub fn try_minimize_switch(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + Self::try_remove_last_break_from_case(stmt, ctx); + Self::collapse_empty_switch_cases(stmt, ctx); + Self::remove_empty_switch(stmt, ctx); + Self::fold_switch_with_one_case(stmt, ctx); + Self::fold_switch_with_two_cases(stmt, ctx); + } + + /// Attempts to remove the last `break` statement from the last case of a switch statement. + fn try_remove_last_break_from_case(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + + if let Some(last_case) = switch_stmt.cases.last_mut() + && Self::remove_last_break(&mut last_case.consequent) + { + ctx.state.changed = true; + } + } + + /// Collapses empty cases in a `SwitchStatement` by removing redundant cases with empty + /// consequent's and consolidating them into a more concise representation. + /// + /// - If the switch statement contains one or fewer cases, it is considered already optimal, and no actions are taken. + /// - If the `default` case is the last case, it is treated as a special case where its emptiness directly + /// influences the analysis of the rest of the cases. + /// - The function identifies a `removable suffix` of cases at the end of the statement, starting from the first + /// non-empty case or case with side-effect-producing expressions backward to the last case. + /// - All cases in the identified removable suffix are eliminated, except for the last case, + /// which is preserved and its test is removed (if applicable). + fn collapse_empty_switch_cases(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + + let case_count = switch_stmt.cases.len(); + if case_count <= 1 { + return; + } + + // if a default case is last we can skip checking if it has body + let (end, allow_break) = if let Some(default_pos) = + switch_stmt.cases.iter().rposition(SwitchCase::is_default_case) + { + if default_pos == case_count - 1 { + ( + case_count - 1, + Self::is_empty_switch_case(&switch_stmt.cases[default_pos].consequent, true), + ) + } else { + (case_count, false) + } + } else { + (case_count, true) + }; + + // Find the last non-removable case (any case whose consequent is non-empty). + let last_non_empty_before_last = switch_stmt.cases[..end].iter().rposition(|case| { + !Self::is_empty_switch_case(&case.consequent, allow_break) + || case.test.as_ref().is_some_and(|test| test.may_have_side_effects(ctx)) + }); + + // start is the first index of the removable suffix + let start = match last_non_empty_before_last { + Some(pos) => pos + 1, + None => 0, + }; + + // nothing removable + if start >= case_count - 1 { + return; + } + + let Some(mut last) = switch_stmt.cases.pop() else { + return; + }; + switch_stmt.cases.truncate(start); + + if !Self::is_empty_switch_case(&last.consequent, true) { + last.test = None; + switch_stmt.cases.push(last); + } + ctx.state.changed = true; + } + + /// Removes an empty switch statement from the given AST statement. + fn remove_empty_switch(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + if switch_stmt.cases.is_empty() { + if switch_stmt.discriminant.may_have_side_effects(ctx) { + *stmt = ctx.ast.statement_expression( + switch_stmt.span, + switch_stmt.discriminant.take_in(ctx.ast), + ); + } else { + *stmt = ctx.ast.statement_empty(switch_stmt.span); + } + ctx.state.changed = true; + } + } + + /// Simplifies a `switch` statement with exactly two cases into an equivalent `if` statement. + /// + /// This transformation is applicable when the `switch` statement meets the following criteria: + /// - It contains exactly two cases. + /// - One of the cases represents the `default` case, and the other defines a condition (`test`). + /// - Both cases can be safely inlined without reordering or modifying program behavior. + /// - Both cases are terminated properly (e.g., with a `break` statement). + fn fold_switch_with_two_cases(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + + // check whatever its default + case + if switch_stmt.cases.len() != 2 + || switch_stmt.cases[0].test.is_some() == switch_stmt.cases[1].test.is_some() + || !Self::is_terminated_switch_case(&switch_stmt.cases[0].consequent) + || !Self::can_case_be_inlined(&switch_stmt.cases[0], ctx) + || !Self::can_case_be_inlined(&switch_stmt.cases[1], ctx) + { + return; + } + + let mut first = switch_stmt.cases.pop().unwrap(); + let mut second = switch_stmt.cases.pop().unwrap(); + Self::remove_last_break(&mut first.consequent); + Self::remove_last_break(&mut second.consequent); + + let (test, consequent, alternate) = if first.test.is_some() { + (first.test.unwrap(), first.consequent, second.consequent) + } else { + (second.test.unwrap(), second.consequent, first.consequent) + }; + + ctx.state.changed = true; + *stmt = ctx.ast.statement_if( + switch_stmt.span, + ctx.ast.expression_binary( + SPAN, + switch_stmt.discriminant.take_in(ctx.ast), + BinaryOperator::StrictEquality, + test, + ), + Self::create_if_block_from_switch_case(consequent, ctx), + Some(Self::create_if_block_from_switch_case(alternate, ctx)), + ); + } + + fn create_if_block_from_switch_case( + mut vec: Vec<'a, Statement<'a>>, + ctx: &mut Ctx<'a, '_>, + ) -> Statement<'a> { + if vec.len() == 1 && matches!(vec.first(), Some(Statement::BlockStatement(_))) { + vec.pop().unwrap() + } else { + ctx.ast.statement_block_with_scope_id( + SPAN, + vec, + ctx.create_child_scope_of_current(ScopeFlags::empty()), + ) + } + } + + fn fold_switch_with_one_case(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { + let Statement::SwitchStatement(switch_stmt) = stmt else { + return; + }; + if switch_stmt.cases.len() == 1 { + let Some(first_case) = switch_stmt.cases.first() else { return }; + if !Self::can_case_be_inlined(first_case, ctx) { + return; + } + let Some(mut case) = switch_stmt.cases.pop() else { + return; + }; + + ctx.state.changed = true; + let discriminant = switch_stmt.discriminant.take_in(ctx.ast); + Self::remove_last_break(&mut case.consequent); + + if let Some(test) = case.test { + *stmt = ctx.ast.statement_if( + switch_stmt.span, + ctx.ast.expression_binary( + SPAN, + discriminant, + BinaryOperator::StrictEquality, + test, + ), + Self::create_if_block_from_switch_case(case.consequent, ctx), + None, + ); + } else { + let mut stmts = ctx.ast.vec(); + if discriminant.may_have_side_effects(ctx) { + stmts.push(ctx.ast.statement_expression(discriminant.span(), discriminant)); + } + if !Self::is_empty_switch_case(&case.consequent, true) { + stmts.extend(case.consequent); + } + *stmt = ctx.ast.statement_block_with_scope_id( + switch_stmt.span, + stmts, + ctx.create_child_scope_of_current(ScopeFlags::empty()), + ); + } + } + } + + fn is_empty_switch_case(stmt: &Vec<'a, Statement<'a>>, allow_break: bool) -> bool { + if stmt.len() != 1 { + return stmt.is_empty(); + } + match stmt.last() { + Some(Statement::BlockStatement(block_stmt)) => { + Self::is_empty_switch_case(&block_stmt.body, allow_break) + } + Some(Statement::BreakStatement(break_stmt)) => { + break_stmt.label.is_none() && allow_break + } + _ => false, + } + } + + fn remove_last_break(stmt: &mut Vec<'a, Statement<'a>>) -> bool { + if stmt.is_empty() { + return false; + } + + let len = stmt.len(); + match stmt.last_mut() { + Some(Statement::BreakStatement(break_stmt)) => { + if break_stmt.label.is_none() { + stmt.truncate(len - 1); + true + } else { + false + } + } + Some(Statement::BlockStatement(block_stmt)) => { + Self::remove_last_break(&mut block_stmt.body) + } + _ => false, + } + } + + fn is_terminated_switch_case(stmt: &Vec<'a, Statement<'a>>) -> bool { + if stmt.is_empty() { + return false; + } + match stmt.last() { + Some(Statement::BlockStatement(block_stmt)) => { + Self::is_terminated_switch_case(&block_stmt.body) + } + Some(last) => last.is_jump_statement(), + _ => false, + } + } + + pub fn can_case_be_inlined(case: &SwitchCase<'a>, ctx: &Ctx<'a, '_>) -> bool { + if case.test.as_ref().is_some_and(|test| test.may_have_side_effects(ctx)) { + return false; + } + + let mut break_finder = BreakFinder::new(); + break_finder.visit_switch_case(case); + !break_finder.nested_unlabelled_break + } +} + +struct BreakFinder { + top_level: bool, + nested_unlabelled_break: bool, +} + +impl BreakFinder { + pub fn new() -> Self { + Self { top_level: true, nested_unlabelled_break: false } + } +} + +// TODO: This is to aggressive, we should allow `break` for last elements in statements +impl<'a> Visit<'a> for BreakFinder { + fn visit_statement(&mut self, it: &Statement<'a>) { + match it { + Statement::ForInStatement(_) + | Statement::ForOfStatement(_) + | Statement::WhileStatement(_) + | Statement::ForStatement(_) + | Statement::ReturnStatement(_) + | Statement::ThrowStatement(_) + | Statement::ExpressionStatement(_) => {} + _ => walk::walk_statement(self, it), + } + } + + fn visit_if_statement(&mut self, it: &IfStatement<'a>) { + let was_top = self.top_level; + self.top_level = false; + walk::walk_if_statement(self, it); + self.top_level = was_top; + } + + fn visit_break_statement(&mut self, it: &BreakStatement<'a>) { + if !self.top_level && it.label.is_none() { + self.nested_unlabelled_break = true; + } + } + + fn visit_with_statement(&mut self, it: &WithStatement<'a>) { + let was_top = self.top_level; + self.top_level = false; + walk::walk_with_statement(self, it); + self.top_level = was_top; + } + + fn visit_try_statement(&mut self, it: &TryStatement<'a>) { + let was_top = self.top_level; + self.top_level = false; + walk::walk_try_statement(self, it); + self.top_level = was_top; + } +} + +#[cfg(test)] +mod test { + use crate::tester::{test, test_same}; + + #[expect(clippy::literal_string_with_formatting_args)] + #[test] + fn minimize_switch() { + test("switch(a()){}", "a()"); + test("switch(a){default: }", "a;"); + test("switch(a){default: break;}", "a;"); + test("switch(a){default: var b; break;}", "a;var b"); + test("switch(a){default: b()}", "a, b();"); + test("switch(a){default: b(); return;}", "a, b(); return"); + + test("switch(a){case 1: break;}", "a;"); + test("switch(a){case 1: b();}", "a === 1 && b()"); + test("switch(a){case 1: b();break; }", "a === 1 && b()"); + test("switch(a){case 1: b();return; }", "if (a === 1) { b(); return; }"); + + test("switch(a){default: case 1: }", "a"); + test("switch(a){case 1: default: }", "a"); + test_same("switch(a){case 1: default: break; case 2: b()}"); + test_same("switch(a){case 1: b(); default: c()}"); + test_same("switch(a){case 1: b(); default: break; case 2: c()}"); + test_same("switch(a){case 1: b(); case 2: break; case 3: c()}"); + test( + "switch(a){case 1: b(); break; case 2: c();break;}", + "switch(a){case 1: b(); break; case 2: c();}", + ); + test_same("switch(a){case 1: b(); case 2: b();}"); + test("switch(a){case 1: var c=2; break;}", "if (a === 1) var c = 2"); + test("switch(a){case 1: case 2: default: b(); break;}", "a, b()"); + + test("switch(a){default: break; case 1: break;}", "a"); + test("switch(a){default: b();break;case 1: c();break;}", "a === 1 ? c() : b()"); + test("switch(a){default: {b();break;} case 1: {c();break;}}", "a === 1 ? c() : b()"); + + test_same("switch(a){case b(): default:}"); + test("switch(a){case 2: case 1: break; default: break;}", "a;"); + test("switch(a){case 3: b(); case 2: case 1: break;}", "a === 3 && b()"); + test("switch(a){case 3: b(); case 2: case 1: }", "a === 3 && b()"); + test_same("switch(a){case 3: if (b) break }"); + test("switch(a){case 3: { if (b) break } }", "switch(a){case 3: if (b) break;}"); + test( + "switch(a){case 3: {if(b) {c()} else {break;}}}", + "switch(a){case 3: if (b) c();else break;}", + ); + test("switch(a){case 3: { for (;;) break } }", "if(a === 3) for (;;) break;"); + test("switch(a){case 3: { for (b of c) break } }", "if (a === 3) for (b of c) break;"); + test_same("switch(a){case 3: with(b) break}"); + test("switch(a){case 3: while(!0) break}", "if (a === 3) for (;;) break;"); + + test( + "switch(a){case 1: c(); case 2: default: b();break;}", + "switch(a){case 1: c(); default: b();}", + ); + test("var x=1;switch(x){case 1: var y;}", "var y;"); + test("function f(){switch(a){case 1: return;}}", "function f() {a;}"); + test("switch(a()) { default: {let y;} }", "a();{let y;}"); + test( + "function f(){switch('x'){case 'x': var x = 1;break; case 'y': break; }}", + "function f(){switch('x'){case 'x': var x = 1;break; case 'y': }}", + ); + test("switch(a){default: if(a) {break;}c();}", "switch(a){default: if(a) break;c();}"); + test("switch(a){case 1: if(a) {b();}c();}", "a === 1 && (a && b(), c())"); + test("switch ('\\v') {case '\\u000B': foo();}", "foo()"); + + test("x: switch(a){case 1: break x;}", "x: if (a === 1) break x;"); + test_same("x: switch(a){case 2: break x; case 1: break x;}"); + test("x: switch(2){case 2: f(); break outer; }", "x: {f(); break outer;}"); + test( + "x: switch(x){case 2: f(); for (;;){break outer;}}", + "x: if(x===2) for(f();;) break outer", + ); + test("x: switch(a){case 2: if(b) { break outer; } }", "x: if(a===2 && b) break outer;"); + + test( + "switch('r'){case 'r': a();break; case 'r': var x=0;break;}", + "switch('r'){case 'r': a();break; case 'r': var x=0;}", + ); + test( + "switch('r'){case 'r': a();break; case 'r': bar();break;}", + "switch('r'){case 'r': a();break; case 'r': bar()}", + ); + test_same("switch(2) {default: a; case 1: b()}"); + test("switch(1) {case 1: a();break; default: b();}", "a()"); + test_same("switch('e') {case 'e': case 'f': a();}"); + test( + "switch('a') {case 'a': a();break; case 'b': b();break;}", + "switch('a') {case 'a': a();break; case 'b': b();}", + ); + test( + "switch('c') {case 'a': a();break; case 'b': b();break;}", + "switch('c') {case 'a': a();break; case 'b': b();}", + ); + test( + "switch(1) {case 1: a();break; case 2: bar();break;}", + "switch(1) {case 1: a();break; case 2: bar();}", + ); + test_same("switch('f') {case 'f': a(); case 'b': b();}"); + test_same("switch('f') {case 'f': if (a() > 0) {b();break;} c(); case 'd': f();}"); + test( + "switch('f') {case 'b': bar();break; case x: x();break; case 'f': f();break;}", + "switch('f') {case 'b': bar();break; case x: x();break; case 'f': f();}", + ); + test( + "switch(1){case 1: case 2: {break;} case 3: case 4: default: b(); break;}", + "switch(1){case 1: case 2: break; default: b();}", + ); + test("switch ('d') {case 'foo': foo();break; default: bar();break;}", "bar()"); + test( + "switch(0){case NaN: foobar();break;case -0.0: foo();break; case 2: bar();break;}", + "switch(0){case NaN: foobar();break;case -0.0: foo();break; case 2: bar();}", + ); + test("let x = 1; switch('x') { case 'x': let x = 2; break;}", "let x = 1; { let x = 2 }"); + test("switch(1){case 2: var x=0;}", "if (0) var x;"); + } +} From 414dff18c6e6ca045d2bf1a38b8849ff8f4d21de Mon Sep 17 00:00:00 2001 From: armano Date: Wed, 31 Dec 2025 23:14:10 +0100 Subject: [PATCH 19/25] fix: disable all declarations and iterators in BreakFinder --- .../src/peephole/minimize_switch_statement.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index ec53687a3a001..836a8a75fba84 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -299,14 +299,18 @@ impl BreakFinder { // TODO: This is to aggressive, we should allow `break` for last elements in statements impl<'a> Visit<'a> for BreakFinder { + fn visit_expression(&mut self, _it: &Expression<'a>) { + // do nothing + } + fn visit_statement(&mut self, it: &Statement<'a>) { + if it.is_declaration() || it.is_iteration_statement() { + return; + } match it { - Statement::ForInStatement(_) - | Statement::ForOfStatement(_) - | Statement::WhileStatement(_) - | Statement::ForStatement(_) + Statement::ThrowStatement(_) + | Statement::ContinueStatement(_) | Statement::ReturnStatement(_) - | Statement::ThrowStatement(_) | Statement::ExpressionStatement(_) => {} _ => walk::walk_statement(self, it), } From d34e48d7b373b096af6a0814b2e39028c1f133be Mon Sep 17 00:00:00 2001 From: armano Date: Thu, 1 Jan 2026 08:25:03 +0100 Subject: [PATCH 20/25] fix: apply nit from code review --- .../oxc_minifier/src/peephole/minimize_switch_statement.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 836a8a75fba84..febe43779db26 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -188,9 +188,7 @@ impl<'a> PeepholeOptimizations { if !Self::can_case_be_inlined(first_case, ctx) { return; } - let Some(mut case) = switch_stmt.cases.pop() else { - return; - }; + let mut case = switch_stmt.cases.pop().unwrap(); ctx.state.changed = true; let discriminant = switch_stmt.discriminant.take_in(ctx.ast); @@ -230,6 +228,7 @@ impl<'a> PeepholeOptimizations { return stmt.is_empty(); } match stmt.last() { + Some(Statement::EmptyStatement(_)) => true, Some(Statement::BlockStatement(block_stmt)) => { Self::is_empty_switch_case(&block_stmt.body, allow_break) } From c25274fcbc10b558b8f20efbb830ce3a6946593e Mon Sep 17 00:00:00 2001 From: armano Date: Thu, 1 Jan 2026 12:37:23 +0100 Subject: [PATCH 21/25] fix(minifier): improve removal of switch breaks from switch cases --- .../src/peephole/minimize_switch_statement.rs | 47 ++++++++++++++----- .../allocs_minifier.snap | 2 +- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index febe43779db26..22e16b2c24a16 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -28,7 +28,7 @@ impl<'a> PeepholeOptimizations { }; if let Some(last_case) = switch_stmt.cases.last_mut() - && Self::remove_last_break(&mut last_case.consequent) + && Self::remove_last_break(&mut last_case.consequent, ctx) { ctx.state.changed = true; } @@ -141,8 +141,8 @@ impl<'a> PeepholeOptimizations { let mut first = switch_stmt.cases.pop().unwrap(); let mut second = switch_stmt.cases.pop().unwrap(); - Self::remove_last_break(&mut first.consequent); - Self::remove_last_break(&mut second.consequent); + Self::remove_last_break(&mut first.consequent, ctx); + Self::remove_last_break(&mut second.consequent, ctx); let (test, consequent, alternate) = if first.test.is_some() { (first.test.unwrap(), first.consequent, second.consequent) @@ -192,7 +192,7 @@ impl<'a> PeepholeOptimizations { ctx.state.changed = true; let discriminant = switch_stmt.discriminant.take_in(ctx.ast); - Self::remove_last_break(&mut case.consequent); + Self::remove_last_break(&mut case.consequent, ctx); if let Some(test) = case.test { *stmt = ctx.ast.statement_if( @@ -239,7 +239,31 @@ impl<'a> PeepholeOptimizations { } } - fn remove_last_break(stmt: &mut Vec<'a, Statement<'a>>) -> bool { + fn remove_break_from_statement(stmt: &mut Statement<'a>, ctx: &Ctx<'a, '_>) -> bool { + match stmt { + Statement::BreakStatement(break_stmt) => { + if break_stmt.label.is_none() { + *stmt = ctx.ast.statement_empty(break_stmt.span); + true + } else { + false + } + } + Statement::BlockStatement(block_stmt) => { + Self::remove_last_break(&mut block_stmt.body, ctx) + } + Statement::IfStatement(if_stmt) => { + let mut changed = Self::remove_break_from_statement(&mut if_stmt.consequent, ctx); + if let Some(alternate) = &mut if_stmt.alternate { + changed |= Self::remove_break_from_statement(alternate, ctx); + } + changed + } + _ => false, + } + } + + fn remove_last_break(stmt: &mut Vec<'a, Statement<'a>>, ctx: &Ctx<'a, '_>) -> bool { if stmt.is_empty() { return false; } @@ -254,9 +278,7 @@ impl<'a> PeepholeOptimizations { false } } - Some(Statement::BlockStatement(block_stmt)) => { - Self::remove_last_break(&mut block_stmt.body) - } + Some(stmt) => Self::remove_break_from_statement(stmt, ctx), _ => false, } } @@ -384,11 +406,12 @@ mod test { test("switch(a){case 2: case 1: break; default: break;}", "a;"); test("switch(a){case 3: b(); case 2: case 1: break;}", "a === 3 && b()"); test("switch(a){case 3: b(); case 2: case 1: }", "a === 3 && b()"); - test_same("switch(a){case 3: if (b) break }"); - test("switch(a){case 3: { if (b) break } }", "switch(a){case 3: if (b) break;}"); + test("switch(a){case 3: if (b) break }", "a === 3 && b"); + test("switch(a){case 3: { if (b) break } }", "a === 3 && b"); + test("switch(a){case 3: { if(b) {c()} else {break;} }}", "a === 3 && b && c()"); test( - "switch(a){case 3: {if(b) {c()} else {break;}}}", - "switch(a){case 3: if (b) c();else break;}", + "switch(a){case 3: { if(b) {c(); break;} else { d(); break;} }}", + "a === 3 && (b ? c() : d())", ); test("switch(a){case 3: { for (;;) break } }", "if(a === 3) for (;;) break;"); test("switch(a){case 3: { for (b of c) break } }", "if (a === 3) for (b of c) break;"); diff --git a/tasks/track_memory_allocations/allocs_minifier.snap b/tasks/track_memory_allocations/allocs_minifier.snap index e6238e3f36e20..fe7f9f1969e43 100644 --- a/tasks/track_memory_allocations/allocs_minifier.snap +++ b/tasks/track_memory_allocations/allocs_minifier.snap @@ -1,6 +1,6 @@ File | File size || Sys allocs | Sys reallocs || Arena allocs | Arena reallocs | Arena bytes ------------------------------------------------------------------------------------------------------------------------------------------- -checker.ts | 2.92 MB || 83502 | 14179 || 152529 | 28180 +checker.ts | 2.92 MB || 83502 | 14179 || 152526 | 28177 cal.com.tsx | 1.06 MB || 40447 | 3032 || 37141 | 4586 From 0a9836b81240d2bcc2c0913f5fd6909333be82b0 Mon Sep 17 00:00:00 2001 From: armano Date: Fri, 2 Jan 2026 17:44:22 +0100 Subject: [PATCH 22/25] fix: allow to remove empty case from end of switch if there is no default --- .../oxc_minifier/src/peephole/minimize_switch_statement.rs | 7 +++++-- tasks/minsize/minsize.snap | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 22e16b2c24a16..8c1ca275d1067 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -83,7 +83,7 @@ impl<'a> PeepholeOptimizations { }; // nothing removable - if start >= case_count - 1 { + if start >= end { return; } @@ -404,6 +404,9 @@ mod test { test_same("switch(a){case b(): default:}"); test("switch(a){case 2: case 1: break; default: break;}", "a;"); + test("switch(a){case 3: b(); break; case 2: break;}", "a === 3 && b()"); + test("switch(a){case 3: b(); case 2: break;}", "a === 3 && b()"); + test("switch(a){case 3: b(); case 2: c(); break;}", "switch(a){case 3: b(); case 2: c();}"); test("switch(a){case 3: b(); case 2: case 1: break;}", "a === 3 && b()"); test("switch(a){case 3: b(); case 2: case 1: }", "a === 3 && b()"); test("switch(a){case 3: if (b) break }", "a === 3 && b"); @@ -427,7 +430,7 @@ mod test { test("switch(a()) { default: {let y;} }", "a();{let y;}"); test( "function f(){switch('x'){case 'x': var x = 1;break; case 'y': break; }}", - "function f(){switch('x'){case 'x': var x = 1;break; case 'y': }}", + "function f(){var x = 1;}", ); test("switch(a){default: if(a) {break;}c();}", "switch(a){default: if(a) break;c();}"); test("switch(a){case 1: if(a) {b();}c();}", "a === 1 && (a && b(), c())"); diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index f6c93384ab5df..74286f62a7d14 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -23,5 +23,5 @@ Original | minified | minified | gzip | gzip | Iterations | Fi 6.69 MB | 2.22 MB | 2.31 MB | 458.41 kB | 488.28 kB | 4 | antd.js -10.95 MB | 3.33 MB | 3.49 MB | 853.26 kB | 915.50 kB | 4 | typescript.js +10.95 MB | 3.33 MB | 3.49 MB | 853.24 kB | 915.50 kB | 4 | typescript.js From 87a59b3435888946eca2e910ef3913c3e13081d5 Mon Sep 17 00:00:00 2001 From: armano Date: Sat, 3 Jan 2026 04:27:32 +0100 Subject: [PATCH 23/25] fix: correct bug with nested switches --- .../src/peephole/minimize_switch_statement.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs index 8c1ca275d1067..b0386b6701cd6 100644 --- a/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs +++ b/crates/oxc_minifier/src/peephole/minimize_switch_statement.rs @@ -301,25 +301,25 @@ impl<'a> PeepholeOptimizations { return false; } - let mut break_finder = BreakFinder::new(); + let mut break_finder = FindNestedBreak::new(); break_finder.visit_switch_case(case); !break_finder.nested_unlabelled_break } } -struct BreakFinder { +struct FindNestedBreak { top_level: bool, nested_unlabelled_break: bool, } -impl BreakFinder { +impl FindNestedBreak { pub fn new() -> Self { Self { top_level: true, nested_unlabelled_break: false } } } // TODO: This is to aggressive, we should allow `break` for last elements in statements -impl<'a> Visit<'a> for BreakFinder { +impl<'a> Visit<'a> for FindNestedBreak { fn visit_expression(&mut self, _it: &Expression<'a>) { // do nothing } @@ -330,6 +330,7 @@ impl<'a> Visit<'a> for BreakFinder { } match it { Statement::ThrowStatement(_) + | Statement::SwitchStatement(_) | Statement::ContinueStatement(_) | Statement::ReturnStatement(_) | Statement::ExpressionStatement(_) => {} @@ -485,5 +486,10 @@ mod test { ); test("let x = 1; switch('x') { case 'x': let x = 2; break;}", "let x = 1; { let x = 2 }"); test("switch(1){case 2: var x=0;}", "if (0) var x;"); + test( + "switch(b){case 2: switch(a){case 2: a();break;case 3: foo();break;}}", + "if (b === 2) switch (a) {case 2: a(); break; case 3: foo();}", + ); + test("switch(b){case 2: switch(a){case 2: foo()}}", "b === 2 && a === 2 && foo();"); } } From 5662f4b553e458c8d0e072543f756470ffce04be Mon Sep 17 00:00:00 2001 From: armano Date: Thu, 15 Jan 2026 12:27:23 +0100 Subject: [PATCH 24/25] chore: update snapshots --- tasks/minsize/minsize.snap | 20 +++++++++---------- .../allocs_minifier.snap | 10 +++++----- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 8d79741b5275b..c76120ac2e915 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -3,25 +3,25 @@ Original | minified | minified | gzip | gzip | Iterations | Fi ------------------------------------------------------------------------------------- 72.14 kB | 23.18 kB | 23.70 kB | 8.38 kB | 8.54 kB | 2 | react.development.js -173.90 kB | 59.40 kB | 59.82 kB | 19.16 kB | 19.33 kB | 2 | moment.js +173.90 kB | 59.36 kB | 59.82 kB | 19.15 kB | 19.33 kB | 2 | moment.js 287.63 kB | 89.26 kB | 90.07 kB | 30.91 kB | 31.95 kB | 2 | jquery.js -342.15 kB | 116.98 kB | 118.14 kB | 43.17 kB | 44.37 kB | 2 | vue.js +342.15 kB | 116.97 kB | 118.14 kB | 43.17 kB | 44.37 kB | 2 | vue.js -544.10 kB | 71.04 kB | 72.48 kB | 25.78 kB | 26.20 kB | 2 | lodash.js +544.10 kB | 71.04 kB | 72.48 kB | 25.77 kB | 26.20 kB | 2 | lodash.js -555.77 kB | 267.39 kB | 270.13 kB | 88.00 kB | 90.80 kB | 2 | d3.js +555.77 kB | 267.21 kB | 270.13 kB | 87.99 kB | 90.80 kB | 2 | d3.js -1.01 MB | 439.37 kB | 458.89 kB | 122.06 kB | 126.71 kB | 2 | bundle.min.js +1.01 MB | 439.32 kB | 458.89 kB | 122.03 kB | 126.71 kB | 2 | bundle.min.js -1.25 MB | 642.66 kB | 646.76 kB | 159.40 kB | 163.73 kB | 2 | three.js +1.25 MB | 642.55 kB | 646.76 kB | 159.37 kB | 163.73 kB | 2 | three.js -2.14 MB | 711.11 kB | 724.14 kB | 160.43 kB | 181.07 kB | 2 | victory.js +2.14 MB | 710.98 kB | 724.14 kB | 160.41 kB | 181.07 kB | 2 | victory.js -3.20 MB | 1.00 MB | 1.01 MB | 322.58 kB | 331.56 kB | 3 | echarts.js +3.20 MB | 1.00 MB | 1.01 MB | 322.57 kB | 331.56 kB | 3 | echarts.js -6.69 MB | 2.22 MB | 2.31 MB | 458.44 kB | 488.28 kB | 4 | antd.js +6.69 MB | 2.22 MB | 2.31 MB | 458.41 kB | 488.28 kB | 4 | antd.js -10.95 MB | 3.33 MB | 3.49 MB | 853.30 kB | 915.50 kB | 4 | typescript.js +10.95 MB | 3.33 MB | 3.49 MB | 853.20 kB | 915.50 kB | 4 | typescript.js diff --git a/tasks/track_memory_allocations/allocs_minifier.snap b/tasks/track_memory_allocations/allocs_minifier.snap index 2f33000316ad0..a0bfbb6ff3898 100644 --- a/tasks/track_memory_allocations/allocs_minifier.snap +++ b/tasks/track_memory_allocations/allocs_minifier.snap @@ -1,14 +1,14 @@ File | File size || Sys allocs | Sys reallocs || Arena allocs | Arena reallocs | Arena bytes ------------------------------------------------------------------------------------------------------------------------------------------- -checker.ts | 2.92 MB || 4128 | 1669 || 152600 | 28244 +checker.ts | 2.92 MB || 4128 | 1669 || 152534 | 28184 -cal.com.tsx | 1.06 MB || 21098 | 474 || 37138 | 4586 +cal.com.tsx | 1.06 MB || 21098 | 474 || 37141 | 4586 RadixUIAdoptionSection.jsx | 2.52 kB || 65 | 3 || 30 | 6 -pdf.mjs | 567.30 kB || 4691 | 569 || 47464 | 7730 +pdf.mjs | 567.30 kB || 4691 | 569 || 47434 | 7704 -antd.js | 6.69 MB || 10723 | 2505 || 331648 | 69358 +antd.js | 6.69 MB || 10723 | 2505 || 331535 | 69298 -binder.ts | 193.08 kB || 431 | 119 || 7075 | 824 +binder.ts | 193.08 kB || 431 | 119 || 7073 | 822 From 206fc44e3acb004aaf5facaaeb74731b6c5e6ab0 Mon Sep 17 00:00:00 2001 From: armano Date: Thu, 15 Jan 2026 13:00:18 +0100 Subject: [PATCH 25/25] test: update tests --- crates/oxc_minifier/tests/peephole/esbuild.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/oxc_minifier/tests/peephole/esbuild.rs b/crates/oxc_minifier/tests/peephole/esbuild.rs index 07381be83b6e6..143144c48a30a 100644 --- a/crates/oxc_minifier/tests/peephole/esbuild.rs +++ b/crates/oxc_minifier/tests/peephole/esbuild.rs @@ -2382,15 +2382,12 @@ fn test_remove_dead_expr_other() { #[test] fn prune_empty_case_before_default() { // Basic case: empty case with literal before default - test( - "switch (x) { case 0: foo(); break; case 1: default: bar() }", - "switch (x) { case 0: foo(); break; default: bar();}", - ); + test("switch (x) { case 0: foo(); break; case 1: default: bar() }", "x === 0 ? foo() : bar();"); // Multiple empty cases with literals before default test( "switch (x) { case 0: foo(); break; case 1: case 2: case 3: default: bar() }", - "switch (x) { case 0: foo(); break; default: bar();}", + "x === 0 ? foo() : bar()", ); // Empty case with non-literal (identifier) should NOT be removed @@ -2403,13 +2400,13 @@ fn prune_empty_case_before_default() { test("switch (x) { default: case 1: bar() }", "switch (x) { default: case 1: bar();}"); // String literals should also be removed - test("switch (x) { case 'a': case 'b': default: bar() }", "switch (x) { default: bar();}"); + test("switch (x) { case 'a': case 'b': default: bar() }", "x, bar()"); // null literal (booleans get transformed to !0/!1 before this optimization runs) - test("switch (x) { case null: default: bar() }", "switch (x) { default: bar();}"); + test("switch (x) { case null: default: bar() }", "x, bar()"); // BigInt literals - test("switch (x) { case 1n: case 2n: default: bar() }", "switch (x) { default: bar();}"); + test("switch (x) { case 1n: case 2n: default: bar() }", "x, bar()"); // Non-empty case should stop the pruning test( @@ -2418,8 +2415,8 @@ fn prune_empty_case_before_default() { ); // Only default - nothing to prune - test("switch (x) { default: bar() }", "switch (x) { default: bar();}"); + test("switch (x) { default: bar() }", "x, bar()"); // No default - nothing to prune - test("switch (x) { case 0: foo(); case 1: }", "switch (x) { case 0: foo(); case 1:}"); + test("switch (x) { case 0: foo(); case 1: }", "x === 0 && foo()"); }