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

Cargo.lock

@@ -1591,6 +1591,7 @@ dependencies = [
  "oxc_semantic",
  "oxc_span",
  "oxc_syntax",
+ "oxc_traverse",
  "pico-args",
 ]
 

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,6 +23,7 @@ use crate::{
     keep_var::KeepVar,
     tri::Tri,
     ty::Ty,
+    CompressorPass,
 };
 
 pub struct FoldConstants<'a> {
@@ -29,14 +31,14 @@ pub struct FoldConstants<'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,11 +1,12 @@
-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
@@ -13,52 +14,80 @@ use crate::CompressOptions;
 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 },

crates/oxc_minifier/src/compressor.rs

@@ -1,61 +1,77 @@
 use oxc_allocator::Allocator;
-use oxc_ast::{ast::*, AstBuilder};
+use oxc_ast::ast::*;
+use oxc_semantic::{ScopeTree, SemanticBuilder, SymbolTable};
+use oxc_traverse::TraverseCtx;
 
 use crate::{
     ast_passes::{
         Collapse, FoldConstants, RemoveDeadCode, RemoveSyntax, SubstituteAlternateSyntax,
     },
-    CompressOptions,
+    CompressOptions, CompressorPass,
 };
 
 pub struct Compressor<'a> {
-    ast: AstBuilder<'a>,
+    allocator: &'a Allocator,
     options: CompressOptions,
 }
 
 impl<'a> Compressor<'a> {
     pub fn new(allocator: &'a Allocator, options: CompressOptions) -> Self {
-        let ast = AstBuilder::new(allocator);
-        Self { ast, options }
+        Self { allocator, options }
     }
 
     pub fn build(self, program: &mut Program<'a>) {
+        let (symbols, scopes) = SemanticBuilder::new("", program.source_type)
+            .build(program)
+            .semantic
+            .into_symbol_table_and_scope_tree();
+        self.build_with_symbols_and_scopes(symbols, scopes, program);
+    }
+
+    pub fn build_with_symbols_and_scopes(
+        self,
+        symbols: SymbolTable,
+        scopes: ScopeTree,
+        program: &mut Program<'a>,
+    ) {
+        let mut ctx = TraverseCtx::new(scopes, symbols, self.allocator);
+        // Run separate AST passes
         // TODO: inline variables
-        self.remove_syntax(program);
-        self.fold_constants(program);
-        self.remove_dead_code(program);
+        self.remove_syntax(program, &mut ctx);
+        self.fold_constants(program, &mut ctx);
+        self.remove_dead_code(program, &mut ctx);
         // TODO: StatementFusion
-        self.substitute_alternate_syntax(program);
-        self.collapse(program);
+        self.substitute_alternate_syntax(program, &mut ctx);
+        self.collapse(program, &mut ctx);
     }
 
-    fn remove_syntax(&self, program: &mut Program<'a>) {
+    fn remove_syntax(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
         if self.options.remove_syntax {
-            RemoveSyntax::new(self.ast, self.options).build(program);
+            RemoveSyntax::new(ctx.ast, self.options).build(program, ctx);
         }
     }
 
-    fn fold_constants(&self, program: &mut Program<'a>) {
+    fn fold_constants(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
         if self.options.fold_constants {
-            FoldConstants::new(self.ast).with_evaluate(self.options.evaluate).build(program);
+            FoldConstants::new(ctx.ast).with_evaluate(self.options.evaluate).build(program, ctx);
         }
     }
 
-    fn substitute_alternate_syntax(&self, program: &mut Program<'a>) {
+    fn substitute_alternate_syntax(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
         if self.options.substitute_alternate_syntax {
-            SubstituteAlternateSyntax::new(self.ast, self.options).build(program);
+            SubstituteAlternateSyntax::new(ctx.ast, self.options).build(program, ctx);
         }
     }
 
-    fn remove_dead_code(&self, program: &mut Program<'a>) {
+    fn remove_dead_code(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
         if self.options.remove_dead_code {
-            RemoveDeadCode::new(self.ast).build(program);
+            RemoveDeadCode::new(ctx.ast).build(program, ctx);
         }
     }
 
-    fn collapse(&self, program: &mut Program<'a>) {
+    fn collapse(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
         if self.options.collapse {
-            Collapse::new(self.ast, self.options).build(program);
+            Collapse::new(ctx.ast, self.options).build(program, ctx);
         }
     }
 }

crates/oxc_minifier/src/lib.rs

@@ -16,7 +16,7 @@ use oxc_ast::ast::Program;
 use oxc_mangler::{Mangler, ManglerBuilder};
 
 pub use crate::{
-    ast_passes::{RemoveDeadCode, RemoveSyntax},
+    ast_passes::{CompressorPass, RemoveDeadCode, RemoveSyntax},
     compressor::Compressor,
     options::CompressOptions,
     plugins::{ReplaceGlobalDefines, ReplaceGlobalDefinesConfig},

crates/oxc_traverse/src/context/mod.rs

@@ -112,7 +112,7 @@ pub struct TraverseCtx<'a> {
 // Public methods
 impl<'a> TraverseCtx<'a> {
     /// Create new traversal context.
-    pub(crate) fn new(scopes: ScopeTree, symbols: SymbolTable, allocator: &'a Allocator) -> Self {
+    pub fn new(scopes: ScopeTree, symbols: SymbolTable, allocator: &'a Allocator) -> Self {
         let ancestry = TraverseAncestry::new();
         let scoping = TraverseScoping::new(scopes, symbols);
         let ast = AstBuilder::new(allocator);

crates/oxc_traverse/src/lib.rs

@@ -147,8 +147,16 @@ pub fn traverse_mut<'a, Tr: Traverse<'a>>(
     scopes: ScopeTree,
 ) -> (SymbolTable, ScopeTree) {
     let mut ctx = TraverseCtx::new(scopes, symbols, allocator);
-    // SAFETY: Walk functions are constructed to avoid unsoundness
-    unsafe { walk::walk_program(traverser, program as *mut Program, &mut ctx) };
+    walk_program(traverser, program, &mut ctx);
     debug_assert!(ctx.ancestors_depth() == 1);
     ctx.scoping.into_symbol_table_and_scope_tree()
 }
+
+pub fn walk_program<'a, Tr: Traverse<'a>>(
+    traverser: &mut Tr,
+    program: &mut Program<'a>,
+    ctx: &mut TraverseCtx<'a>,
+) {
+    // SAFETY: Walk functions are constructed to avoid unsoundness
+    unsafe { walk::walk_program(traverser, program as *mut Program, ctx) };
+}