refactor(minifier): make DCE remove more code to align with rollup (#12427)

Author: Boshen

crates/oxc_minifier/examples/dce.rs

@@ -40,7 +40,7 @@ fn main() -> std::io::Result<()> {
 fn dce(allocator: &Allocator, source_text: &str, source_type: SourceType, nospace: bool) -> String {
     let ret = Parser::new(allocator, source_text, source_type).parse();
     let mut program = ret.program;
-    Compressor::new(allocator).dead_code_elimination(&mut program, CompressOptions::smallest());
+    Compressor::new(allocator).dead_code_elimination(&mut program, CompressOptions::dce());
     Codegen::new()
         .with_options(CodegenOptions { minify: nospace, ..CodegenOptions::default() })
         .build(&program)

crates/oxc_minifier/src/keep_var.rs

@@ -59,10 +59,6 @@ impl<'a> KeepVar<'a> {
         Self { ast, vars: std::vec![], all_hoisted: true }
     }
 
-    pub fn all_hoisted(&self) -> bool {
-        self.all_hoisted
-    }
-
     pub fn get_variable_declaration(self) -> Option<ArenaBox<'a, VariableDeclaration<'a>>> {
         if self.vars.is_empty() {
             return None;

crates/oxc_minifier/src/options.rs

@@ -24,6 +24,18 @@ pub struct CompressOptions {
     /// Default `false`
     pub drop_console: bool,
 
+    /// Join consecutive var, let and const statements.
+    ///
+    /// Default `true`
+    pub join_vars: bool,
+
+    /// Join consecutive simple statements using the comma operator.
+    ///
+    /// `a; b` -> `a, b`
+    ///
+    /// Default `true`
+    pub sequences: bool,
+
     /// Drop unreferenced functions and variables.
     pub unused: CompressOptionsUnused,
 
@@ -48,6 +60,8 @@ impl CompressOptions {
             keep_names: CompressOptionsKeepNames::all_false(),
             drop_debugger: true,
             drop_console: false,
+            join_vars: true,
+            sequences: true,
             unused: CompressOptionsUnused::Remove,
             treeshake: TreeShakeOptions::default(),
         }
@@ -59,10 +73,25 @@ impl CompressOptions {
             keep_names: CompressOptionsKeepNames::all_true(),
             drop_debugger: false,
             drop_console: false,
+            join_vars: true,
+            sequences: true,
             unused: CompressOptionsUnused::Keep,
             treeshake: TreeShakeOptions::default(),
         }
     }
+
+    pub fn dce() -> Self {
+        Self {
+            target: ESTarget::ESNext,
+            keep_names: CompressOptionsKeepNames::all_true(),
+            drop_debugger: false,
+            drop_console: false,
+            join_vars: false,
+            sequences: false,
+            unused: CompressOptionsUnused::Remove,
+            treeshake: TreeShakeOptions::default(),
+        }
+    }
 }
 
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Default)]

crates/oxc_minifier/src/peephole/minimize_statements.rs

@@ -483,7 +483,10 @@ impl<'a> Traverse<'a, MinifierState<'a>> for DeadCodeElimination {
     fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) {
         let mut state = State::default();
         let mut ctx = Ctx::new(ctx);
-        self.inner.remove_dead_code_exit_statements(stmts, &mut state, &mut ctx);
+        self.inner.minimize_statements(stmts, &mut state, &mut ctx);
+        if state.changed {
+            self.inner.minimize_statements(stmts, &mut state, &mut ctx);
+        }
         stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_)));
     }
 

crates/oxc_minifier/src/peephole/mod.rs

@@ -64,71 +64,6 @@ impl<'a> PeepholeOptimizations {
         }
     }
 
-    /// Removes dead code thats comes after `return`, `throw`, `continue` and `break` statements.
-    pub fn remove_dead_code_exit_statements(
-        &self,
-        stmts: &mut Vec<'a, Statement<'a>>,
-        state: &mut State,
-        ctx: &mut Ctx<'a, '_>,
-    ) {
-        // Remove code after `return` and `throw` statements
-        let mut index = None;
-        'outer: for (i, stmt) in stmts.iter().enumerate() {
-            if stmt.is_jump_statement() {
-                index.replace(i);
-                break;
-            }
-            // Double check block statements folded by if statements above
-            if let Statement::BlockStatement(block_stmt) = stmt {
-                for stmt in &block_stmt.body {
-                    if stmt.is_jump_statement() {
-                        index.replace(i);
-                        break 'outer;
-                    }
-                }
-            }
-        }
-
-        let Some(index) = index else { return };
-        if index == stmts.len() - 1 {
-            return;
-        }
-
-        let mut keep_var = KeepVar::new(ctx.ast);
-
-        for stmt in stmts.iter().skip(index + 1) {
-            keep_var.visit_statement(stmt);
-        }
-
-        let mut i = 0;
-        let len = stmts.len();
-        stmts.retain(|s| {
-            i += 1;
-            if i - 1 <= index {
-                return true;
-            }
-            // Keep module syntax and function declaration
-            if s.is_module_declaration()
-                || matches!(s.as_declaration(), Some(Declaration::FunctionDeclaration(_)))
-            {
-                return true;
-            }
-            false
-        });
-
-        let all_hoisted = keep_var.all_hoisted();
-        if let Some(stmt) = keep_var.get_variable_declaration_statement() {
-            stmts.push(stmt);
-            if !all_hoisted {
-                state.changed = true;
-            }
-        }
-
-        if stmts.len() != len {
-            state.changed = true;
-        }
-    }
-
     /// Remove block from single line blocks
     /// `{ block } -> block`
     fn try_optimize_block(

crates/oxc_minifier/src/peephole/remove_dead_code.rs

@@ -7,8 +7,6 @@ use oxc_minifier::Compressor;
 use oxc_parser::Parser;
 use oxc_span::SourceType;
 
-use super::default_options;
-
 #[track_caller]
 fn run(source_text: &str, source_type: SourceType, options: Option<CompressOptions>) -> String {
     let allocator = Allocator::default();
@@ -28,7 +26,7 @@ fn test(source_text: &str, expected: &str) {
     let source_text = source_text.cow_replace("false", f);
 
     let source_type = SourceType::default();
-    let result = run(&source_text, source_type, Some(default_options()));
+    let result = run(&source_text, source_type, Some(CompressOptions::dce()));
     let expected = run(expected, source_type, None);
     assert_eq!(result, expected, "\nfor source\n{source_text}\nexpect\n{expected}\ngot\n{result}");
 }
@@ -64,8 +62,8 @@ fn dce_if_statement() {
         "if (xxx) foo; else bar",
     );
     test(
-        "if (xxx) { foo } else if (false) { var a; var b; } else if (false) { var c; var d; }",
-        "if (xxx) foo; else if (0) var a, b; else if (0) var c, d;",
+        "if (xxx) { foo } else if (false) { var a; var b; } else if (false) { var c; var d; } f(a,b,c,d)",
+        "if (xxx) foo; else if (0) var a, b; else if (0) var c, d; f(a,b,c,d)",
     );
 
     test("if (!false) { foo }", "foo");
@@ -88,18 +86,18 @@ fn dce_if_statement() {
     // Shadowed `undefined` as a variable should not be erased.
     // This is a rollup test.
     // <https://github.com/rollup/rollup/blob/master/test/function/samples/allow-undefined-as-parameter/main.js>
-    test_same("function foo(undefined) { if (!undefined) throw Error('') }");
+    test_same("function foo(undefined) { if (!undefined) throw Error('') } foo()");
 
-    test("function foo() { if (undefined) { bar } }", "function foo() { }");
-    test("function foo() { { bar } }", "function foo() { bar }");
+    test("function foo() { if (undefined) { bar } } foo()", "function foo() { } foo()");
+    test("function foo() { { bar } } foo()", "function foo() { bar } foo()");
 
     test("if (true) { foo; } if (true) { foo; }", "foo; foo;");
-    test("if (true) { foo; return } foo; if (true) { bar; return } bar;", "{ foo; return }");
+    test("if (true) { foo; return } foo; if (true) { bar; return } bar;", "foo; return");
 
     // nested expression
     test(
-        "const a = { fn: function() { if (true) { foo; } } }",
-        "const a = { fn: function() { foo; } }",
+        "const a = { fn: function() { if (true) { foo; } } } bar(a)",
+        "const a = { fn: function() { foo; } } bar(a)",
     );
 
     // parenthesized
@@ -128,61 +126,67 @@ fn dce_conditional_expression() {
     test("!!false ? foo : bar;", "bar");
     test("!!true ? foo : bar;", "foo");
 
-    test("const foo = true ? A : B", "const foo = A");
-    test("const foo = false ? A : B", "const foo = B");
+    test("const foo = true ? A : B", "A");
+    test("const foo = false ? A : B", "B");
 }
 
 #[test]
 fn dce_logical_expression() {
     test("false && bar()", "");
     test("true && bar()", "bar()");
 
-    test("const foo = false && bar()", "const foo = false");
-    test("const foo = true && bar()", "const foo = bar()");
+    test("var foo = false && bar(); baz(foo)", "var foo = false; baz(foo)");
+    test("var foo = true && bar(); baz(foo)", "var foo = bar(); baz(foo)");
+
+    test("foo = false && bar()", "foo = false");
+    test("foo = true && bar()", "foo = bar()");
 }
 
 #[test]
 fn dce_var_hoisting() {
     test(
         "function f() {
+          KEEP();
           return () => {
             var x;
           }
           REMOVE;
-          function KEEP() {}
+          function KEEP() { FOO }
           REMOVE;
-        }",
+        } f()",
         "function f() {
-          return () => {
-            var x;
-          }
-          function KEEP() {}
-        }",
+          KEEP();
+          return () => { }
+          function KEEP() { FOO }
+        } f()",
     );
     test(
         "function f() {
+          KEEP();
           return function g() {
             var x;
           }
           REMOVE;
           function KEEP() {}
           REMOVE;
-        }",
+        } f()",
         "function f() {
-          return function g() {
-            var x;
-          }
+          KEEP();
+          return function g() {}
           function KEEP() {}
-        }",
+        } f()",
     );
 }
 
 #[test]
 fn pure_comment_for_pure_global_constructors() {
-    test("var x = new WeakSet", "var x = /* @__PURE__ */ new WeakSet();\n");
-    test("var x = new WeakSet(null)", "var x = /* @__PURE__ */ new WeakSet(null);\n");
-    test("var x = new WeakSet(undefined)", "var x = /* @__PURE__ */ new WeakSet(void 0);\n");
-    test("var x = new WeakSet([])", "var x = /* @__PURE__ */ new WeakSet([]);\n");
+    test("var x = new WeakSet; foo(x)", "var x = /* @__PURE__ */ new WeakSet();\nfoo(x)");
+    test("var x = new WeakSet(null); foo(x)", "var x = /* @__PURE__ */ new WeakSet(null);\nfoo(x)");
+    test(
+        "var x = new WeakSet(undefined); foo(x)",
+        "var x = /* @__PURE__ */ new WeakSet(void 0);\nfoo(x)",
+    );
+    test("var x = new WeakSet([]); foo(x)", "var x = /* @__PURE__ */ new WeakSet([]);\nfoo(x)");
 }
 
 #[test]
@@ -203,13 +207,12 @@ fn dce_from_terser() {
             if (x) {
                 y();
             }
-        }",
+        } f()",
         "function f() {
             a();
             b();
             x = 10;
-            return;
-        }",
+        } f()",
     );
 
     test(
@@ -248,9 +251,6 @@ fn dce_from_terser() {
         }
         console.log(foo, bar, Baz);
         ",
-        "
-        if (0) var qux;
-        console.log(foo, bar, Baz);
-        ",
+        "console.log(foo, bar, Baz);",
     );
 }