Skip to content

Commit

Permalink
refactor(minifier): do not use AstBuilder::*_from_* methods
Browse files Browse the repository at this point in the history
  • Loading branch information
overlookmotel committed Nov 1, 2024
1 parent 0f8a61d commit 28293bd
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 85 deletions.
21 changes: 6 additions & 15 deletions crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,26 +215,21 @@ impl<'a, 'b> PeepholeRemoveDeadCode {
.declarations
.splice(0..0, ctx.ast.move_vec(&mut var_init.declarations));
} else {
var_decl = Some(ctx.ast.move_variable_declaration(var_init));
var_decl =
Some(ctx.ast.alloc(ctx.ast.move_variable_declaration(var_init)));
}
}
Some(var_decl.map_or_else(
|| ctx.ast.statement_empty(SPAN),
|var_decl| {
ctx.ast
.statement_declaration(ctx.ast.declaration_from_variable(var_decl))
},
Statement::VariableDeclaration,
))
}
None => {
let mut keep_var = KeepVar::new(ctx.ast);
keep_var.visit_statement(&for_stmt.body);
Some(keep_var.get_variable_declaration().map_or_else(
|| ctx.ast.statement_empty(SPAN),
|var_decl| {
ctx.ast
.statement_declaration(ctx.ast.declaration_from_variable(var_decl))
},
Statement::VariableDeclaration,
))
}
_ => None,
Expand Down Expand Up @@ -315,9 +310,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode {

return Some(ctx.ast.statement_expression(
template_lit.span,
ctx.ast.expression_from_sequence(
ctx.ast.sequence_expression(template_lit.span, expressions),
),
ctx.ast.expression_sequence(template_lit.span, expressions),
));
}
Expression::FunctionExpression(function_expr) if function_expr.id.is_none() => {
Expand Down Expand Up @@ -390,9 +383,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode {

Some(ctx.ast.statement_expression(
array_expr.span,
ctx.ast.expression_from_sequence(
ctx.ast.sequence_expression(array_expr.span, transformed_elements),
),
ctx.ast.expression_sequence(array_expr.span, transformed_elements),
))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,27 +46,20 @@ impl PeepholeReplaceKnownMethods {
let Expression::StaticMemberExpression(member) = &call_expr.callee else { return };
if let Expression::StringLiteral(string_lit) = &member.object {
let replacement = match member.property.name.as_str() {
"toLowerCase" | "toUpperCase" | "trim" => {
let transformed_value =
match member.property.name.as_str() {
"toLowerCase" => Some(ctx.ast.string_literal(
call_expr.span,
string_lit.value.cow_to_lowercase(),
)),
"toUpperCase" => Some(ctx.ast.string_literal(
call_expr.span,
string_lit.value.cow_to_uppercase(),
)),
"trim" => Some(
ctx.ast.string_literal(call_expr.span, string_lit.value.trim()),
),
_ => None,
};

transformed_value.map(|transformed_value| {
ctx.ast.expression_from_string_literal(transformed_value)
})
}
"toLowerCase" | "toUpperCase" | "trim" => match member.property.name.as_str() {
"toLowerCase" => Some(ctx.ast.expression_string_literal(
call_expr.span,
string_lit.value.cow_to_lowercase(),
)),
"toUpperCase" => Some(ctx.ast.expression_string_literal(
call_expr.span,
string_lit.value.cow_to_uppercase(),
)),
"trim" => Some(
ctx.ast.expression_string_literal(call_expr.span, string_lit.value.trim()),
),
_ => None,
},
"indexOf" | "lastIndexOf" => Self::try_fold_string_index_of(
call_expr.span,
call_expr,
Expand Down Expand Up @@ -131,12 +124,12 @@ impl PeepholeReplaceKnownMethods {
};

#[expect(clippy::cast_precision_loss)]
return Some(ctx.ast.expression_from_numeric_literal(ctx.ast.numeric_literal(
return Some(ctx.ast.expression_numeric_literal(
span,
result as f64,
result.to_string(),
NumberBase::Decimal,
)));
));
}

fn try_fold_string_substring_or_slice<'a>(
Expand Down Expand Up @@ -181,8 +174,9 @@ impl PeepholeReplaceKnownMethods {
}
};

return Some(ctx.ast.expression_from_string_literal(
ctx.ast.string_literal(span, string_lit.value.as_str().substring(start_idx, end_idx)),
return Some(ctx.ast.expression_string_literal(
span,
string_lit.value.as_str().substring(start_idx, end_idx),
));
}

Expand Down Expand Up @@ -216,7 +210,7 @@ impl PeepholeReplaceKnownMethods {
.char_at(char_at_index)
.map_or(String::new(), |v| v.to_string());

return Some(ctx.ast.expression_from_string_literal(ctx.ast.string_literal(span, result)));
return Some(ctx.ast.expression_string_literal(span, result));
}

fn try_fold_string_char_code_at<'a>(
Expand All @@ -240,12 +234,12 @@ impl PeepholeReplaceKnownMethods {
let result = string_lit.value.as_str().char_code_at(char_at_index)?;

#[expect(clippy::cast_lossless)]
Some(ctx.ast.expression_from_numeric_literal(ctx.ast.numeric_literal(
Some(ctx.ast.expression_numeric_literal(
span,
result as f64,
result.to_string(),
NumberBase::Decimal,
)))
))
}
fn try_fold_string_replace_or_string_replace_all<'a>(
span: Span,
Expand Down Expand Up @@ -287,7 +281,7 @@ impl PeepholeReplaceKnownMethods {
_ => unreachable!(),
};

Some(ctx.ast.expression_from_string_literal(ctx.ast.string_literal(span, result)))
Some(ctx.ast.expression_string_literal(span, result))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,7 @@ impl<'a> Traverse<'a> for PeepholeSubstituteAlternateSyntax {
}
Expression::TemplateLiteral(_) => {
if let Some(val) = expr.to_js_string() {
let new_expr = ctx.ast.string_literal(expr.span(), val);
*expr = ctx.ast.expression_from_string_literal(new_expr);
*expr = ctx.ast.expression_string_literal(expr.span(), val);
self.changed = true;
}
}
Expand Down Expand Up @@ -273,15 +272,11 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
let Some((_void_exp, id_ref)) = pair else {
return;
};
let argument = ctx.ast.expression_from_identifier_reference(id_ref);
let left = ctx.ast.unary_expression(SPAN, UnaryOperator::Typeof, argument);
let right = ctx.ast.string_literal(SPAN, "u");
let binary_expr = ctx.ast.binary_expression(
expr.span,
ctx.ast.expression_from_unary(left),
BinaryOperator::GreaterThan,
ctx.ast.expression_from_string_literal(right),
);
let argument = Expression::Identifier(ctx.alloc(id_ref));
let left = ctx.ast.expression_unary(SPAN, UnaryOperator::Typeof, argument);
let right = ctx.ast.expression_string_literal(SPAN, "u");
let binary_expr =
ctx.ast.binary_expression(expr.span, left, BinaryOperator::GreaterThan, right);
*expr = binary_expr;
self.changed = true;
}
Expand Down Expand Up @@ -406,8 +401,7 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
// `new Array(literal)` -> `[literal]`
else if arg.is_literal() || matches!(arg, Expression::ArrayExpression(_)) {
let mut elements = ctx.ast.vec();
let element =
ctx.ast.array_expression_element_expression(ctx.ast.move_expression(arg));
let element = ArrayExpressionElement::from(ctx.ast.move_expression(arg));
elements.push(element);
Some(Self::array_literal(elements, ctx))
}
Expand All @@ -421,12 +415,11 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
} else {
// `new Array(1, 2, 3)` -> `[1, 2, 3]`
let elements = ctx.ast.vec_from_iter(
new_expr.arguments.iter_mut().filter_map(|arg| arg.as_expression_mut()).map(
|arg| {
ctx.ast
.array_expression_element_expression(ctx.ast.move_expression(arg))
},
),
new_expr
.arguments
.iter_mut()
.filter_map(|arg| arg.as_expression_mut())
.map(|arg| ArrayExpressionElement::from(ctx.ast.move_expression(arg))),
);
Some(Self::array_literal(elements, ctx))
}
Expand Down Expand Up @@ -465,8 +458,7 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
// `Array(literal)` -> `[literal]`
else if arg.is_literal() || matches!(arg, Expression::ArrayExpression(_)) {
let mut elements = ctx.ast.vec();
let element =
ctx.ast.array_expression_element_expression(ctx.ast.move_expression(arg));
let element = ArrayExpressionElement::from(ctx.ast.move_expression(arg));
elements.push(element);
Some(Self::array_literal(elements, ctx))
} else {
Expand All @@ -475,12 +467,11 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
} else {
// `Array(1, 2, 3)` -> `[1, 2, 3]`
let elements = ctx.ast.vec_from_iter(
call_expr.arguments.iter_mut().filter_map(|arg| arg.as_expression_mut()).map(
|arg| {
ctx.ast
.array_expression_element_expression(ctx.ast.move_expression(arg))
},
),
call_expr
.arguments
.iter_mut()
.filter_map(|arg| arg.as_expression_mut())
.map(|arg| ArrayExpressionElement::from(ctx.ast.move_expression(arg))),
);
Some(Self::array_literal(elements, ctx))
}
Expand Down Expand Up @@ -510,17 +501,17 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
}
}

Some(ctx.ast.expression_from_unary(ctx.ast.unary_expression(
Some(ctx.ast.expression_unary(
call_expr.span,
UnaryOperator::LogicalNot,
ctx.ast.expression_from_unary(ctx.ast.unary_expression(
ctx.ast.expression_unary(
call_expr.span,
UnaryOperator::LogicalNot,
ctx.ast.move_expression(
call_expr.arguments.get_mut(0).and_then(|arg| arg.as_expression_mut())?,
),
)),
)))
),
))
} else if call_expr.callee.is_global_reference_name("String", ctx.symbols()) {
// `String(a)` -> `'' + (a)`
let arg = call_expr.arguments.get_mut(0).and_then(|arg| arg.as_expression_mut())?;
Expand All @@ -533,7 +524,7 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {

Some(ctx.ast.expression_binary(
call_expr.span,
ctx.ast.expression_from_string_literal(ctx.ast.string_literal(SPAN, "")),
ctx.ast.expression_string_literal(SPAN, ""),
BinaryOperator::Addition,
ctx.ast.move_expression(arg),
))
Expand Down
4 changes: 1 addition & 3 deletions crates/oxc_minifier/src/ast_passes/statement_fusion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,7 @@ impl<'a> StatementFusion {
init.as_expression_mut().unwrap()
} else {
for_stmt.init =
Some(ctx.ast.for_statement_init_expression(
ctx.ast.expression_sequence(SPAN, exprs),
));
Some(ForStatementInit::from(ctx.ast.expression_sequence(SPAN, exprs)));
return;
}
}
Expand Down
10 changes: 4 additions & 6 deletions crates/oxc_minifier/src/keep_var.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use oxc_allocator::Box as ArenaBox;
use oxc_ast::{ast::*, AstBuilder, Visit, NONE};
use oxc_ecmascript::BoundNames;
use oxc_span::{Atom, Span, SPAN};
Expand Down Expand Up @@ -60,7 +61,7 @@ impl<'a> KeepVar<'a> {
self.all_hoisted
}

pub fn get_variable_declaration(self) -> Option<VariableDeclaration<'a>> {
pub fn get_variable_declaration(self) -> Option<ArenaBox<'a, VariableDeclaration<'a>>> {
if self.vars.is_empty() {
return None;
}
Expand All @@ -72,14 +73,11 @@ impl<'a> KeepVar<'a> {
self.ast.variable_declarator(span, kind, id, None, false)
}));

let decl = self.ast.variable_declaration(SPAN, kind, decls, false);
let decl = self.ast.alloc_variable_declaration(SPAN, kind, decls, false);
Some(decl)
}

pub fn get_variable_declaration_statement(self) -> Option<Statement<'a>> {
let stmt = self.ast.statement_declaration(
self.ast.declaration_from_variable(self.get_variable_declaration()?),
);
Some(stmt)
self.get_variable_declaration().map(Statement::VariableDeclaration)
}
}

0 comments on commit 28293bd

Please sign in to comment.