From ca16ba83b5b6e5986dbaf64201bc903f86804ff9 Mon Sep 17 00:00:00 2001 From: Boshen Date: Wed, 11 Sep 2024 23:58:33 +0800 Subject: [PATCH] refactor(minifier): replace `VisitMut` with `Traverse` for inject and define plugins closes #5704 --- .../src/plugins/inject_global_variables.rs | 57 ++++++++++-------- .../src/plugins/replace_global_defines.rs | 59 ++++++++++--------- .../tests/plugins/inject_global_variables.rs | 4 +- .../tests/plugins/replace_global_defines.rs | 4 +- 4 files changed, 68 insertions(+), 56 deletions(-) diff --git a/crates/oxc_minifier/src/plugins/inject_global_variables.rs b/crates/oxc_minifier/src/plugins/inject_global_variables.rs index 41c3be918cf3ce..b46d8c9044b66e 100644 --- a/crates/oxc_minifier/src/plugins/inject_global_variables.rs +++ b/crates/oxc_minifier/src/plugins/inject_global_variables.rs @@ -1,9 +1,12 @@ +use std::sync::Arc; + use cow_utils::CowUtils; + use oxc_allocator::Allocator; -use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, VisitMut}; +use oxc_ast::{ast::*, AstBuilder}; use oxc_semantic::{ScopeTree, SymbolTable}; use oxc_span::{CompactStr, SPAN}; -use std::sync::Arc; +use oxc_traverse::{traverse_mut, Traverse, TraverseCtx}; use super::replace_global_defines::{DotDefine, ReplaceGlobalDefines}; @@ -100,12 +103,18 @@ impl<'a> From<&InjectImport> for DotDefineState<'a> { } } +#[must_use] +pub struct InjectGlobalVariablesReturn { + pub symbols: SymbolTable, + pub scopes: ScopeTree, +} + /// Injects import statements for global variables. /// /// References: /// /// * -pub struct InjectGlobalVariables<'a, 'b> { +pub struct InjectGlobalVariables<'a> { ast: AstBuilder<'a>, config: InjectGlobalVariablesConfig, @@ -116,36 +125,32 @@ pub struct InjectGlobalVariables<'a, 'b> { /// Identifiers for which dot define replaced a member expression. replaced_dot_defines: Vec<(/* identifier of member expression */ CompactStr, /* local */ CompactStr)>, - - symbols: &'b mut SymbolTable, // will be used to keep symbols in sync - scopes: &'b mut ScopeTree, } -impl<'a, 'b> VisitMut<'a> for InjectGlobalVariables<'a, 'b> { - fn visit_expression(&mut self, expr: &mut Expression<'a>) { - self.replace_dot_defines(expr); - walk_mut::walk_expression(self, expr); +impl<'a> Traverse<'a> for InjectGlobalVariables<'a> { + fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { + self.replace_dot_defines(expr, ctx); } } -impl<'a, 'b> InjectGlobalVariables<'a, 'b> { - pub fn new( - allocator: &'a Allocator, - symbols: &'b mut SymbolTable, - scopes: &'b mut ScopeTree, - config: InjectGlobalVariablesConfig, - ) -> Self { +impl<'a> InjectGlobalVariables<'a> { + pub fn new(allocator: &'a Allocator, config: InjectGlobalVariablesConfig) -> Self { Self { ast: AstBuilder::new(allocator), config, dot_defines: vec![], replaced_dot_defines: vec![], - symbols, - scopes, } } - pub fn build(&mut self, program: &mut Program<'a>) { + pub fn build( + &mut self, + symbols: SymbolTable, + scopes: ScopeTree, + program: &mut Program<'a>, + ) -> InjectGlobalVariablesReturn { + let mut symbols = symbols; + let mut scopes = scopes; // Step 1: slow path where visiting the AST is required to replace dot defines. let dot_defines = self .config @@ -157,7 +162,7 @@ impl<'a, 'b> InjectGlobalVariables<'a, 'b> { if !dot_defines.is_empty() { self.dot_defines = dot_defines; - self.visit_program(program); + (symbols, scopes) = traverse_mut(self, self.ast.allocator, program, symbols, scopes); } // Step 2: find all the injects that are referenced. @@ -172,17 +177,19 @@ impl<'a, 'b> InjectGlobalVariables<'a, 'b> { } else if self.replaced_dot_defines.iter().any(|d| d.0 == i.specifier.local()) { false } else { - self.scopes.root_unresolved_references().contains_key(i.specifier.local()) + scopes.root_unresolved_references().contains_key(i.specifier.local()) } }) .cloned() .collect::>(); if injects.is_empty() { - return; + return InjectGlobalVariablesReturn { symbols, scopes }; } self.inject_imports(&injects, program); + + InjectGlobalVariablesReturn { symbols, scopes } } fn inject_imports(&self, injects: &[InjectImport], program: &mut Program<'a>) { @@ -227,10 +234,10 @@ impl<'a, 'b> InjectGlobalVariables<'a, 'b> { } } - fn replace_dot_defines(&mut self, expr: &mut Expression<'a>) { + fn replace_dot_defines(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { if let Expression::StaticMemberExpression(member) = expr { for DotDefineState { dot_define, value_atom } in &mut self.dot_defines { - if ReplaceGlobalDefines::is_dot_define(self.symbols, dot_define, member) { + if ReplaceGlobalDefines::is_dot_define(ctx.symbols(), dot_define, member) { // If this is first replacement made for this dot define, // create `Atom` for replacement, and record in `replaced_dot_defines` if value_atom.is_none() { diff --git a/crates/oxc_minifier/src/plugins/replace_global_defines.rs b/crates/oxc_minifier/src/plugins/replace_global_defines.rs index 59d9bf1d02c030..7868806d84a1fe 100644 --- a/crates/oxc_minifier/src/plugins/replace_global_defines.rs +++ b/crates/oxc_minifier/src/plugins/replace_global_defines.rs @@ -1,12 +1,13 @@ use std::{cmp::Ordering, sync::Arc}; use oxc_allocator::Allocator; -use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, VisitMut}; +use oxc_ast::ast::*; use oxc_diagnostics::OxcDiagnostic; use oxc_parser::Parser; -use oxc_semantic::{IsGlobalReference, SymbolTable}; +use oxc_semantic::{IsGlobalReference, ScopeTree, SymbolTable}; use oxc_span::{CompactStr, SourceType}; use oxc_syntax::identifier::is_identifier_name; +use oxc_traverse::{traverse_mut, Traverse, TraverseCtx}; /// Configuration for [ReplaceGlobalDefines]. /// @@ -162,6 +163,12 @@ impl ReplaceGlobalDefinesConfig { } } +#[must_use] +pub struct ReplaceGlobalDefinesReturn { + pub symbols: SymbolTable, + pub scopes: ScopeTree, +} + /// Replace Global Defines. /// /// References: @@ -169,46 +176,44 @@ impl ReplaceGlobalDefinesConfig { /// * /// * /// * -pub struct ReplaceGlobalDefines<'a, 'b> { - ast: AstBuilder<'a>, - symbols: &'b mut SymbolTable, +pub struct ReplaceGlobalDefines<'a> { + allocator: &'a Allocator, config: ReplaceGlobalDefinesConfig, } -impl<'a, 'b> VisitMut<'a> for ReplaceGlobalDefines<'a, 'b> { - fn visit_expression(&mut self, expr: &mut Expression<'a>) { - self.replace_identifier_defines(expr); - self.replace_dot_defines(expr); - walk_mut::walk_expression(self, expr); +impl<'a> Traverse<'a> for ReplaceGlobalDefines<'a> { + fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { + self.replace_identifier_defines(expr, ctx); + self.replace_dot_defines(expr, ctx); } } -impl<'a, 'b> ReplaceGlobalDefines<'a, 'b> { - pub fn new( - allocator: &'a Allocator, - symbols: &'b mut SymbolTable, - config: ReplaceGlobalDefinesConfig, - ) -> Self { - Self { ast: AstBuilder::new(allocator), symbols, config } +impl<'a> ReplaceGlobalDefines<'a> { + pub fn new(allocator: &'a Allocator, config: ReplaceGlobalDefinesConfig) -> Self { + Self { allocator, config } } - pub fn build(&mut self, program: &mut Program<'a>) { - self.visit_program(program); + pub fn build( + &mut self, + symbols: SymbolTable, + scopes: ScopeTree, + program: &mut Program<'a>, + ) -> ReplaceGlobalDefinesReturn { + let (symbols, scopes) = traverse_mut(self, self.allocator, program, symbols, scopes); + ReplaceGlobalDefinesReturn { symbols, scopes } } // Construct a new expression because we don't have ast clone right now. fn parse_value(&self, source_text: &str) -> Expression<'a> { // Allocate the string lazily because replacement happens rarely. - let source_text = self.ast.allocator.alloc(source_text.to_string()); + let source_text = self.allocator.alloc(source_text.to_string()); // Unwrapping here, it should already be checked by [ReplaceGlobalDefinesConfig::new]. - Parser::new(self.ast.allocator, source_text, SourceType::default()) - .parse_expression() - .unwrap() + Parser::new(self.allocator, source_text, SourceType::default()).parse_expression().unwrap() } - fn replace_identifier_defines(&self, expr: &mut Expression<'a>) { + fn replace_identifier_defines(&self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { let Expression::Identifier(ident) = expr else { return }; - if !ident.is_global_reference(self.symbols) { + if !ident.is_global_reference(ctx.symbols()) { return; } for (key, value) in &self.config.0.identifier { @@ -220,12 +225,12 @@ impl<'a, 'b> ReplaceGlobalDefines<'a, 'b> { } } - fn replace_dot_defines(&mut self, expr: &mut Expression<'a>) { + fn replace_dot_defines(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { let Expression::StaticMemberExpression(member) = expr else { return; }; for dot_define in &self.config.0.dot { - if Self::is_dot_define(self.symbols, dot_define, member) { + if Self::is_dot_define(ctx.symbols(), dot_define, member) { let value = self.parse_value(&dot_define.value); *expr = value; return; diff --git a/crates/oxc_minifier/tests/plugins/inject_global_variables.rs b/crates/oxc_minifier/tests/plugins/inject_global_variables.rs index b359622b9acece..6793093b45f323 100644 --- a/crates/oxc_minifier/tests/plugins/inject_global_variables.rs +++ b/crates/oxc_minifier/tests/plugins/inject_global_variables.rs @@ -16,11 +16,11 @@ pub(crate) fn test(source_text: &str, expected: &str, config: InjectGlobalVariab let allocator = Allocator::default(); let ret = Parser::new(&allocator, source_text, source_type).parse(); let program = allocator.alloc(ret.program); - let (mut symbols, mut scopes) = SemanticBuilder::new(source_text) + let (symbols, scopes) = SemanticBuilder::new(source_text) .build(program) .semantic .into_symbol_table_and_scope_tree(); - InjectGlobalVariables::new(&allocator, &mut symbols, &mut scopes, config).build(program); + let _ = InjectGlobalVariables::new(&allocator, config).build(symbols, scopes, program); let result = CodeGenerator::new() .with_options(CodegenOptions { single_quote: true, ..CodegenOptions::default() }) .build(program) diff --git a/crates/oxc_minifier/tests/plugins/replace_global_defines.rs b/crates/oxc_minifier/tests/plugins/replace_global_defines.rs index fc1ed274c7f474..1755855e197c98 100644 --- a/crates/oxc_minifier/tests/plugins/replace_global_defines.rs +++ b/crates/oxc_minifier/tests/plugins/replace_global_defines.rs @@ -12,11 +12,11 @@ pub(crate) fn test(source_text: &str, expected: &str, config: ReplaceGlobalDefin let allocator = Allocator::default(); let ret = Parser::new(&allocator, source_text, source_type).parse(); let program = allocator.alloc(ret.program); - let (mut symbols, _scopes) = SemanticBuilder::new(source_text) + let (symbols, scopes) = SemanticBuilder::new(source_text) .build(program) .semantic .into_symbol_table_and_scope_tree(); - ReplaceGlobalDefines::new(&allocator, &mut symbols, config).build(program); + let _ = ReplaceGlobalDefines::new(&allocator, config).build(symbols, scopes, program); let result = CodeGenerator::new() .with_options(CodegenOptions { single_quote: true, ..CodegenOptions::default() }) .build(program)