refactor(minifier): use oxc_traverse for AST passes (#4725)

crates/oxc_minifier/Cargo.toml

@@ -30,6 +30,7 @@ oxc_parser      = { workspace = true }
 oxc_diagnostics = { workspace = true }
 oxc_codegen     = { workspace = true }
 oxc_mangler     = { workspace = true }
+oxc_traverse    = { workspace = true }
 
 num-bigint = { workspace = true }
 num-traits = { workspace = true }

crates/oxc_minifier/src/ast_passes/collapse.rs

@@ -1,7 +1,8 @@
 use oxc_allocator::Vec;
-use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, VisitMut};
+use oxc_ast::{ast::*, AstBuilder};
+use oxc_traverse::{Traverse, TraverseCtx};
 
-use crate::CompressOptions;
+use crate::{CompressOptions, CompressorPass};
 
 /// Collapse variable declarations (TODO: and assignments).
 ///
@@ -12,13 +13,13 @@ pub struct Collapse<'a> {
     options: CompressOptions,
 }
 
-impl<'a> VisitMut<'a> for Collapse<'a> {
-    fn visit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>) {
+impl<'a> CompressorPass<'a> for Collapse<'a> {}
+
+impl<'a> Traverse<'a> for Collapse<'a> {
+    fn enter_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) {
         if self.options.join_vars {
             self.join_vars(stmts);
         }
-
-        walk_mut::walk_statements(self, stmts);
     }
 }
 
@@ -27,10 +28,6 @@ impl<'a> Collapse<'a> {
         Self { ast, options }
     }
 
-    pub fn build(&mut self, program: &mut Program<'a>) {
-        self.visit_program(program);
-    }
-
     /// Join consecutive var statements
     fn join_vars(&mut self, stmts: &mut Vec<'a, Statement<'a>>) {
         // Collect all the consecutive ranges that contain joinable vars.

crates/oxc_minifier/src/ast_passes/fold_constants.rs

@@ -6,12 +6,13 @@ use std::{cmp::Ordering, mem};
 
 use num_bigint::BigInt;
 
-use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, Visit, VisitMut};
+use oxc_ast::{ast::*, AstBuilder, Visit};
 use oxc_span::{GetSpan, Span, SPAN};
 use oxc_syntax::{
     number::NumberBase,
     operator::{BinaryOperator, LogicalOperator, UnaryOperator},
 };
+use oxc_traverse::{Traverse, TraverseCtx};
 
 use crate::{
     ast_util::{
@@ -22,21 +23,22 @@ use crate::{
     keep_var::KeepVar,
     tri::Tri,
     ty::Ty,
+    CompressorPass,
 };
 
 pub struct FoldConstants<'a> {
     ast: AstBuilder<'a>,
     evaluate: bool,
 }
 
-impl<'a> VisitMut<'a> for FoldConstants<'a> {
-    fn visit_statement(&mut self, stmt: &mut Statement<'a>) {
-        walk_mut::walk_statement(self, stmt);
+impl<'a> CompressorPass<'a> for FoldConstants<'a> {}
+
+impl<'a> Traverse<'a> for FoldConstants<'a> {
+    fn exit_statement(&mut self, stmt: &mut Statement<'a>, _ctx: &mut TraverseCtx<'a>) {
         self.fold_condition(stmt);
     }
 
-    fn visit_expression(&mut self, expr: &mut Expression<'a>) {
-        walk_mut::walk_expression(self, expr);
+    fn exit_expression(&mut self, expr: &mut Expression<'a>, _ctx: &mut TraverseCtx<'a>) {
         self.fold_expression(expr);
         self.fold_conditional_expression(expr);
     }
@@ -52,10 +54,6 @@ impl<'a> FoldConstants<'a> {
         self
     }
 
-    pub fn build(&mut self, program: &mut Program<'a>) {
-        self.visit_program(program);
-    }
-
     fn fold_expression_and_get_boolean_value(&mut self, expr: &mut Expression<'a>) -> Option<bool> {
         self.fold_expression(expr);
         get_boolean_value(expr)

crates/oxc_minifier/src/ast_passes/mod.rs

@@ -9,3 +9,16 @@ pub use fold_constants::FoldConstants;
 pub use remove_dead_code::RemoveDeadCode;
 pub use remove_syntax::RemoveSyntax;
 pub use substitute_alternate_syntax::SubstituteAlternateSyntax;
+
+use oxc_ast::ast::Program;
+use oxc_traverse::{walk_program, Traverse, TraverseCtx};
+
+pub trait CompressorPass<'a> {
+    fn build(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>)
+    where
+        Self: Traverse<'a>,
+        Self: Sized,
+    {
+        walk_program(self, program, ctx);
+    }
+}

crates/oxc_minifier/src/ast_passes/remove_dead_code.rs

@@ -1,7 +1,8 @@
 use oxc_allocator::Vec;
-use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, Visit, VisitMut};
+use oxc_ast::{ast::*, AstBuilder, Visit};
+use oxc_traverse::{Traverse, TraverseCtx};
 
-use crate::keep_var::KeepVar;
+use crate::{keep_var::KeepVar, CompressorPass};
 
 /// Remove Dead Code from the AST.
 ///
@@ -12,15 +13,12 @@ pub struct RemoveDeadCode<'a> {
     ast: AstBuilder<'a>,
 }
 
-impl<'a> VisitMut<'a> for RemoveDeadCode<'a> {
-    fn visit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>) {
+impl<'a> CompressorPass<'a> for RemoveDeadCode<'a> {}
+
+impl<'a> Traverse<'a> for RemoveDeadCode<'a> {
+    fn enter_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) {
         stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_)));
         self.dead_code_elimination(stmts);
-        walk_mut::walk_statements(self, stmts);
-    }
-
-    fn visit_expression(&mut self, expr: &mut Expression<'a>) {
-        walk_mut::walk_expression(self, expr);
     }
 }
 
@@ -29,10 +27,6 @@ impl<'a> RemoveDeadCode<'a> {
         Self { ast }
     }
 
-    pub fn build(&mut self, program: &mut Program<'a>) {
-        self.visit_program(program);
-    }
-
     /// Removes dead code thats comes after `return` statements after inlining `if` statements
     fn dead_code_elimination(&mut self, stmts: &mut Vec<'a, Statement<'a>>) {
         // Remove code after `return` and `throw` statements

crates/oxc_minifier/src/ast_passes/remove_syntax.rs

@@ -1,7 +1,8 @@
 use oxc_allocator::Vec;
-use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, VisitMut};
+use oxc_ast::{ast::*, AstBuilder};
+use oxc_traverse::{Traverse, TraverseCtx};
 
-use crate::CompressOptions;
+use crate::{CompressOptions, CompressorPass};
 
 /// Remove syntax from the AST.
 ///
@@ -13,23 +14,27 @@ pub struct RemoveSyntax<'a> {
     options: CompressOptions,
 }
 
-impl<'a> VisitMut<'a> for RemoveSyntax<'a> {
-    fn visit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>) {
+impl<'a> CompressorPass<'a> for RemoveSyntax<'a> {}
+
+impl<'a> Traverse<'a> for RemoveSyntax<'a> {
+    fn enter_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) {
         stmts.retain(|stmt| {
             !(matches!(stmt, Statement::EmptyStatement(_))
                 || self.drop_debugger(stmt)
                 || self.drop_console(stmt))
         });
-        walk_mut::walk_statements(self, stmts);
     }
 
-    fn visit_expression(&mut self, expr: &mut Expression<'a>) {
+    fn enter_expression(&mut self, expr: &mut Expression<'a>, _ctx: &mut TraverseCtx<'a>) {
         self.strip_parenthesized_expression(expr);
         self.compress_console(expr);
-        walk_mut::walk_expression(self, expr);
     }
 
-    fn visit_arrow_function_expression(&mut self, expr: &mut ArrowFunctionExpression<'a>) {
+    fn exit_arrow_function_expression(
+        &mut self,
+        expr: &mut ArrowFunctionExpression<'a>,
+        _ctx: &mut TraverseCtx<'a>,
+    ) {
         self.recover_arrow_expression_after_drop_console(expr);
     }
 }
@@ -39,10 +44,6 @@ impl<'a> RemoveSyntax<'a> {
         Self { ast, options }
     }
 
-    pub fn build(&mut self, program: &mut Program<'a>) {
-        self.visit_program(program);
-    }
-
     fn strip_parenthesized_expression(&self, expr: &mut Expression<'a>) {
         if let Expression::ParenthesizedExpression(paren_expr) = expr {
             *expr = self.ast.move_expression(&mut paren_expr.expression);

crates/oxc_minifier/src/ast_passes/substitute_alternate_syntax.rs

@@ -1,64 +1,93 @@
-use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, VisitMut};
+use oxc_ast::{ast::*, AstBuilder};
 use oxc_span::SPAN;
 use oxc_syntax::{
     number::NumberBase,
     operator::{BinaryOperator, UnaryOperator},
 };
+use oxc_traverse::{Ancestor, Traverse, TraverseCtx};
 
-use crate::CompressOptions;
+use crate::{CompressOptions, CompressorPass};
 
 /// A peephole optimization that minimizes code by simplifying conditional
 /// expressions, replacing IFs with HOOKs, replacing object constructors
 /// with literals, and simplifying returns.
 pub struct SubstituteAlternateSyntax<'a> {
     ast: AstBuilder<'a>,
     options: CompressOptions,
+    in_define_export: bool,
 }
 
-impl<'a> VisitMut<'a> for SubstituteAlternateSyntax<'a> {
-    fn visit_statement(&mut self, stmt: &mut Statement<'a>) {
+impl<'a> CompressorPass<'a> for SubstituteAlternateSyntax<'a> {}
+
+impl<'a> Traverse<'a> for SubstituteAlternateSyntax<'a> {
+    fn enter_statement(&mut self, stmt: &mut Statement<'a>, _ctx: &mut TraverseCtx<'a>) {
         self.compress_block(stmt);
         // self.compress_while(stmt);
-        walk_mut::walk_statement(self, stmt);
     }
 
-    fn visit_return_statement(&mut self, stmt: &mut ReturnStatement<'a>) {
-        walk_mut::walk_return_statement(self, stmt);
+    fn exit_return_statement(
+        &mut self,
+        stmt: &mut ReturnStatement<'a>,
+        _ctx: &mut TraverseCtx<'a>,
+    ) {
         // We may fold `void 1` to `void 0`, so compress it after visiting
         Self::compress_return_statement(stmt);
     }
 
-    fn visit_variable_declaration(&mut self, decl: &mut VariableDeclaration<'a>) {
+    fn enter_variable_declaration(
+        &mut self,
+        decl: &mut VariableDeclaration<'a>,
+        _ctx: &mut TraverseCtx<'a>,
+    ) {
         for declarator in decl.declarations.iter_mut() {
-            self.visit_variable_declarator(declarator);
             Self::compress_variable_declarator(declarator);
         }
     }
 
-    fn visit_expression(&mut self, expr: &mut Expression<'a>) {
-        // Bail cjs `Object.defineProperty(exports, ...)`
-        if Self::is_object_define_property_exports(expr) {
-            return;
+    /// Set `in_define_export` flag if this is a top-level statement of form:
+    /// ```js
+    /// Object.defineProperty(exports, 'Foo', {
+    ///   enumerable: true,
+    ///   get: function() { return Foo_1.Foo; }
+    /// });
+    /// ```
+    fn enter_call_expression(
+        &mut self,
+        call_expr: &mut CallExpression<'a>,
+        ctx: &mut TraverseCtx<'a>,
+    ) {
+        // Check if this call expression is a top level `ExpressionStatement`.
+        // NB: 1 = global, 2 = Program, 3 = ExpressionStatement
+        if ctx.ancestors_depth() == 3
+            && matches!(ctx.parent(), Ancestor::ExpressionStatementExpression(_))
+            && Self::is_object_define_property_exports(call_expr)
+        {
+            self.in_define_export = true;
         }
-        walk_mut::walk_expression(self, expr);
+    }
+
+    fn exit_call_expression(&mut self, _expr: &mut CallExpression<'a>, _ctx: &mut TraverseCtx<'a>) {
+        self.in_define_export = false;
+    }
+
+    fn enter_expression(&mut self, expr: &mut Expression<'a>, _ctx: &mut TraverseCtx<'a>) {
         if !self.compress_undefined(expr) {
             self.compress_boolean(expr);
         }
     }
 
-    fn visit_binary_expression(&mut self, expr: &mut BinaryExpression<'a>) {
-        walk_mut::walk_binary_expression(self, expr);
+    fn exit_binary_expression(
+        &mut self,
+        expr: &mut BinaryExpression<'a>,
+        _ctx: &mut TraverseCtx<'a>,
+    ) {
         self.compress_typeof_undefined(expr);
     }
 }
 
 impl<'a> SubstituteAlternateSyntax<'a> {
     pub fn new(ast: AstBuilder<'a>, options: CompressOptions) -> Self {
-        Self { ast, options }
-    }
-
-    pub fn build(&mut self, program: &mut Program<'a>) {
-        self.visit_program(program);
+        Self { ast, options, in_define_export: false }
     }
 
     /* Utilities */
@@ -77,13 +106,22 @@ impl<'a> SubstituteAlternateSyntax<'a> {
     }
 
     /// Test `Object.defineProperty(exports, ...)`
-    fn is_object_define_property_exports(expr: &Expression<'a>) -> bool {
-        let Expression::CallExpression(call_expr) = expr else { return false };
+    fn is_object_define_property_exports(call_expr: &CallExpression<'a>) -> bool {
         let Some(Argument::Identifier(ident)) = call_expr.arguments.first() else { return false };
         if ident.name != "exports" {
             return false;
         }
-        call_expr.callee.is_specific_member_access("Object", "defineProperty")
+
+        // Use tighter check than `call_expr.callee.is_specific_member_access("Object", "defineProperty")`
+        // because we're looking for `Object.defineProperty` specifically, not e.g. `Object['defineProperty']`
+        if let Expression::StaticMemberExpression(callee) = &call_expr.callee {
+            if let Expression::Identifier(id) = &callee.object {
+                if id.name == "Object" && callee.property.name == "defineProperty" {
+                    return true;
+                }
+            }
+        }
+        false
     }
 
     /* Statements */
@@ -115,11 +153,12 @@ impl<'a> SubstituteAlternateSyntax<'a> {
 
     /* Expressions */
 
-    /// Transforms boolean expression `true` => `!0` `false` => `!1`
-    /// Enabled by `compress.booleans`
+    /// Transforms boolean expression `true` => `!0` `false` => `!1`.
+    /// Enabled by `compress.booleans`.
+    /// Do not compress `true` in `Object.defineProperty(exports, 'Foo', {enumerable: true, ...})`.
     fn compress_boolean(&mut self, expr: &mut Expression<'a>) -> bool {
         let Expression::BooleanLiteral(lit) = expr else { return false };
-        if self.options.booleans {
+        if self.options.booleans && !self.in_define_export {
             let num = self.ast.expression_numeric_literal(
                 SPAN,
                 if lit.value { 0.0 } else { 1.0 },