refactor(minifier): move dce conditional expression to RemoveDeadCode

This is aligned to closure compiler
oxc-project · Sep 22, 2024 · e561e17 · e561e17
1 parent 612f638
commit e561e17
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 51 deletions.
diff --git a/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs b/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs
@@ -1,7 +1,7 @@
 use oxc_ast::ast::*;
-use oxc_traverse::{Ancestor, Traverse, TraverseCtx};
+use oxc_traverse::{Traverse, TraverseCtx};
 
-use crate::{node_util::NodeUtil, tri::Tri, CompressorPass};
+use crate::CompressorPass;
 
 /// Minimize Conditions
 ///
@@ -16,44 +16,18 @@ impl<'a> CompressorPass<'a> for PeepholeMinimizeConditions {}
 
 impl<'a> Traverse<'a> for PeepholeMinimizeConditions {
     fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
-        self.fold_expression(expr, ctx);
-    }
-}
-
-impl<'a> PeepholeMinimizeConditions {
-    pub fn new() -> Self {
-        Self
-    }
-
-    fn fold_expression(&self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
         if let Some(folded_expr) = match expr {
-            Expression::ConditionalExpression(e) => self.try_fold_conditional_expression(e, ctx),
             Expression::UnaryExpression(e) if e.operator.is_not() => self.try_minimize_not(e, ctx),
             _ => None,
         } {
             *expr = folded_expr;
         };
     }
+}
 
-    fn try_fold_conditional_expression(
-        &self,
-        expr: &mut ConditionalExpression<'a>,
-        ctx: &mut TraverseCtx<'a>,
-    ) -> Option<Expression<'a>> {
-        match ctx.get_boolean_value(&expr.test) {
-            Tri::True => {
-                // Bail `let o = { f() { assert.ok(this !== o); } }; (true ? o.f : false)(); (true ? o.f : false)``;`
-                let parent = ctx.ancestry.parent();
-                if parent.is_tagged_template_expression()
-                    || matches!(parent, Ancestor::CallExpressionCallee(_))
-                {
-                    return None;
-                }
-                Some(ctx.ast.move_expression(&mut expr.consequent))
-            }
-            Tri::False => Some(ctx.ast.move_expression(&mut expr.alternate)),
-            Tri::Unknown => None,
-        }
+impl<'a> PeepholeMinimizeConditions {
+    pub fn new() -> Self {
+        Self
     }
 
     /// Try to minimize NOT nodes such as `!(x==y)`.

diff --git a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs
@@ -1,7 +1,7 @@
 use oxc_allocator::Vec;
 use oxc_ast::{ast::*, Visit};
 use oxc_span::SPAN;
-use oxc_traverse::{Traverse, TraverseCtx};
+use oxc_traverse::{Ancestor, Traverse, TraverseCtx};
 
 use crate::{keep_var::KeepVar, node_util::NodeUtil, tri::Tri, CompressorPass};
 
@@ -23,6 +23,15 @@ impl<'a> Traverse<'a> for PeepholeRemoveDeadCode {
         stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_)));
         self.dead_code_elimination(stmts, ctx);
     }
+
+    fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
+        if let Some(folded_expr) = match expr {
+            Expression::ConditionalExpression(e) => self.try_fold_conditional_expression(e, ctx),
+            _ => None,
+        } {
+            *expr = folded_expr;
+        }
+    }
 }
 
 impl<'a> PeepholeRemoveDeadCode {
@@ -114,22 +123,26 @@ impl<'a> PeepholeRemoveDeadCode {
             Tri::Unknown => {}
         }
     }
-}
-
-// /// <https://github.com/google/closure-compiler/blob/master/test/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java>
-// #[cfg(test)]
-// mod test {
-// use oxc_allocator::Allocator;
-
-// use crate::{tester, CompressOptions};
-
-// fn test(source_text: &str, expected: &str) {
-// let allocator = Allocator::default();
-// let mut pass = super::PeepholeRemoveDeadCode::new();
-// tester::test(&allocator, source_text, expected, &mut pass);
-// }
 
-// fn test_same(source_text: &str) {
-// test(source_text, source_text);
-// }
-// }
+    /// Try folding conditional expression (?:) if the condition results of the condition is known.
+    fn try_fold_conditional_expression(
+        &self,
+        expr: &mut ConditionalExpression<'a>,
+        ctx: &mut TraverseCtx<'a>,
+    ) -> Option<Expression<'a>> {
+        match ctx.get_boolean_value(&expr.test) {
+            Tri::True => {
+                // Bail `let o = { f() { assert.ok(this !== o); } }; (true ? o.f : false)(); (true ? o.f : false)``;`
+                let parent = ctx.ancestry.parent();
+                if parent.is_tagged_template_expression()
+                    || matches!(parent, Ancestor::CallExpressionCallee(_))
+                {
+                    return None;
+                }
+                Some(ctx.ast.move_expression(&mut expr.consequent))
+            }
+            Tri::False => Some(ctx.ast.move_expression(&mut expr.alternate)),
+            Tri::Unknown => None,
+        }
+    }
+}