From 25d4bf9aadcd2faf527d7d60cfeeca08e10c0979 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Mon, 13 Jan 2025 13:53:53 +0000 Subject: [PATCH] fix(minifier): remove usage of empty spans (#8462) --- .../src/ast_passes/peephole_fold_constants.rs | 4 +- .../peephole_minimize_conditions.rs | 40 +++++++------- .../ast_passes/peephole_remove_dead_code.rs | 37 +++++++------ .../peephole_substitute_alternate_syntax.rs | 53 ++++++++----------- .../src/ast_passes/statement_fusion.rs | 16 ++++-- crates/oxc_minifier/src/keep_var.rs | 3 +- 6 files changed, 78 insertions(+), 75 deletions(-) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs b/crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs index 5a0b1501d3c5c..9bbd193adb95c 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs @@ -3,7 +3,7 @@ use oxc_ecmascript::{ constant_evaluation::{ConstantEvaluation, ConstantValue, ValueType}, side_effects::MayHaveSideEffects, }; -use oxc_span::{GetSpan, SPAN}; +use oxc_span::GetSpan; use oxc_syntax::{ number::NumberBase, operator::{BinaryOperator, LogicalOperator}, @@ -192,7 +192,7 @@ impl<'a, 'b> PeepholeFoldConstants { ctx.ast.move_expression(&mut logical_expr.left), ctx.ast.move_expression(&mut logical_expr.right), ]); - ctx.ast.expression_sequence(SPAN, expressions) + ctx.ast.expression_sequence(logical_expr.span, expressions) } else { // nullish condition => this expression evaluates to the right side. ctx.ast.move_expression(&mut logical_expr.right) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs b/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs index ddecb66869dda..53738d92b7267 100644 --- 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_allocator::Vec; use oxc_ast::{ast::*, NONE}; use oxc_ecmascript::constant_evaluation::{ConstantEvaluation, ValueType}; -use oxc_span::{cmp::ContentEq, GetSpan, SPAN}; +use oxc_span::{cmp::ContentEq, GetSpan}; use oxc_syntax::es_target::ESTarget; use oxc_traverse::{traverse_mut_with_ctx, Ancestor, ReusableTraverseCtx, Traverse, TraverseCtx}; @@ -433,9 +433,9 @@ impl<'a> PeepholeMinimizeConditions { if let Expression::ConditionalExpression(consequent) = &mut expr.consequent { if consequent.alternate.content_eq(&expr.alternate) { return Some(ctx.ast.expression_conditional( - SPAN, + expr.span, ctx.ast.expression_logical( - SPAN, + expr.test.span(), ctx.ast.move_expression(&mut expr.test), LogicalOperator::And, ctx.ast.move_expression(&mut consequent.test), @@ -450,9 +450,9 @@ impl<'a> PeepholeMinimizeConditions { if let Expression::ConditionalExpression(alternate) = &mut expr.alternate { if alternate.consequent.content_eq(&expr.consequent) { return Some(ctx.ast.expression_conditional( - SPAN, + expr.span, ctx.ast.expression_logical( - SPAN, + expr.test.span(), ctx.ast.move_expression(&mut expr.test), LogicalOperator::Or, ctx.ast.move_expression(&mut alternate.test), @@ -469,10 +469,10 @@ impl<'a> PeepholeMinimizeConditions { && alternate.expressions[1].content_eq(&expr.consequent) { return Some(ctx.ast.expression_sequence( - SPAN, + expr.span, ctx.ast.vec_from_array([ ctx.ast.expression_logical( - SPAN, + expr.test.span(), ctx.ast.move_expression(&mut expr.test), LogicalOperator::Or, ctx.ast.move_expression(&mut alternate.expressions[0]), @@ -489,10 +489,10 @@ impl<'a> PeepholeMinimizeConditions { && consequent.expressions[1].content_eq(&expr.alternate) { return Some(ctx.ast.expression_sequence( - SPAN, + expr.span, ctx.ast.vec_from_array([ ctx.ast.expression_logical( - SPAN, + expr.test.span(), ctx.ast.move_expression(&mut expr.test), LogicalOperator::And, ctx.ast.move_expression(&mut consequent.expressions[0]), @@ -509,9 +509,9 @@ impl<'a> PeepholeMinimizeConditions { && logical_expr.right.content_eq(&expr.alternate) { return Some(ctx.ast.expression_logical( - SPAN, + expr.span, ctx.ast.expression_logical( - SPAN, + expr.test.span(), ctx.ast.move_expression(&mut expr.test), LogicalOperator::And, ctx.ast.move_expression(&mut logical_expr.left), @@ -528,9 +528,9 @@ impl<'a> PeepholeMinimizeConditions { && logical_expr.right.content_eq(&expr.consequent) { return Some(ctx.ast.expression_logical( - SPAN, + expr.span, ctx.ast.expression_logical( - SPAN, + expr.test.span(), ctx.ast.move_expression(&mut expr.test), LogicalOperator::Or, ctx.ast.move_expression(&mut logical_expr.left), @@ -579,9 +579,9 @@ impl<'a> PeepholeMinimizeConditions { }; let mut args = std::mem::replace(&mut consequent.arguments, ctx.ast.vec()); args[0] = ctx.ast.argument_spread_element( - SPAN, + expr.span, ctx.ast.expression_conditional( - SPAN, + expr.test.span(), ctx.ast.move_expression(&mut expr.test), consequent_first_arg, alternate_first_arg, @@ -602,7 +602,7 @@ impl<'a> PeepholeMinimizeConditions { ctx.ast.move_expression(alternate.arguments[0].to_expression_mut()); let mut args = std::mem::replace(&mut consequent.arguments, ctx.ast.vec()); args[0] = Argument::from(ctx.ast.expression_conditional( - SPAN, + expr.test.span(), ctx.ast.move_expression(&mut expr.test), consequent_first_arg, alternate_first_arg, @@ -626,9 +626,9 @@ impl<'a> PeepholeMinimizeConditions { return Some(ctx.ast.expression_logical( expr.span, ctx.ast.expression_unary( - SPAN, + ident.span(), UnaryOperator::LogicalNot, - ctx.ast.expression_unary(SPAN, UnaryOperator::LogicalNot, ident), + ctx.ast.expression_unary(ident.span(), UnaryOperator::LogicalNot, ident), ), LogicalOperator::Or, ctx.ast.move_expression(&mut expr.alternate), @@ -661,9 +661,9 @@ impl<'a> PeepholeMinimizeConditions { return Some(ctx.ast.expression_logical( expr.span, ctx.ast.expression_unary( - SPAN, + expr.span, UnaryOperator::LogicalNot, - ctx.ast.expression_unary(SPAN, UnaryOperator::LogicalNot, ident), + ctx.ast.expression_unary(ident.span(), UnaryOperator::LogicalNot, ident), ), LogicalOperator::And, ctx.ast.move_expression(&mut expr.consequent), 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 index 65f984aa6af6a..79bcb8be7c878 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs @@ -4,7 +4,7 @@ use oxc_ecmascript::{ constant_evaluation::{ConstantEvaluation, IsLiteralValue}, side_effects::MayHaveSideEffects, }; -use oxc_span::SPAN; +use oxc_span::GetSpan; use oxc_traverse::{traverse_mut_with_ctx, Ancestor, ReusableTraverseCtx, Traverse, TraverseCtx}; use crate::{ctx::Ctx, keep_var::KeepVar, CompressorPass}; @@ -225,7 +225,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode { ctx.ast.move_statement(&mut if_stmt.consequent) } else { if_stmt.alternate.as_mut().map_or_else( - || ctx.ast.statement_empty(SPAN), + || ctx.ast.statement_empty(if_stmt.span), |alternate| ctx.ast.move_statement(alternate), ) }); @@ -256,7 +256,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode { } } Some(var_decl.map_or_else( - || ctx.ast.statement_empty(SPAN), + || ctx.ast.statement_empty(for_stmt.span), Statement::VariableDeclaration, )) } @@ -264,7 +264,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode { 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), + || ctx.ast.statement_empty(for_stmt.span), Statement::VariableDeclaration, )) } @@ -301,7 +301,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode { let mut var = KeepVar::new(ctx.ast); var.visit_statement(&s.body); let var_decl = var.get_variable_declaration_statement(); - var_decl.unwrap_or(ctx.ast.statement_empty(SPAN)).into() + var_decl.unwrap_or_else(|| ctx.ast.statement_empty(s.span)).into() } fn try_fold_expression_stmt( @@ -320,7 +320,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode { stmt.expression .is_literal_value(false) - .then(|| Some(ctx.ast.statement_empty(SPAN))) + .then(|| Some(ctx.ast.statement_empty(stmt.span))) .unwrap_or_else(|| match &mut stmt.expression { Expression::ArrayExpression(expr) => Self::try_fold_array_expression(expr, ctx), Expression::ObjectExpression(object_expr) => { @@ -332,7 +332,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode { } let mut expressions = ctx.ast.move_vec(&mut template_lit.expressions); if expressions.len() == 0 { - return Some(ctx.ast.statement_empty(SPAN)); + return Some(ctx.ast.statement_empty(stmt.span)); } else if expressions.len() == 1 { return Some( ctx.ast.statement_expression( @@ -347,15 +347,15 @@ impl<'a, 'b> PeepholeRemoveDeadCode { )) } Expression::FunctionExpression(function_expr) if function_expr.id.is_none() => { - Some(ctx.ast.statement_empty(SPAN)) + Some(ctx.ast.statement_empty(stmt.span)) } - Expression::ArrowFunctionExpression(_) => Some(ctx.ast.statement_empty(SPAN)), + Expression::ArrowFunctionExpression(_) => Some(ctx.ast.statement_empty(stmt.span)), // `typeof x` -> `` Expression::UnaryExpression(unary_expr) if unary_expr.operator.is_typeof() && unary_expr.argument.is_identifier_reference() => { - Some(ctx.ast.statement_empty(SPAN)) + Some(ctx.ast.statement_empty(stmt.span)) } // `typeof x.y` -> `x.y`, `void x` -> `x` // `+0n` -> `Uncaught TypeError: Cannot convert a BigInt value to a number` @@ -382,7 +382,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode { if finalizer.body.is_empty() { Some(ctx.ast.statement_empty(s.span)) } else { - let mut block = ctx.ast.block_statement(SPAN, ctx.ast.vec()); + let mut block = ctx.ast.block_statement(finalizer.span, ctx.ast.vec()); std::mem::swap(&mut **finalizer, &mut block); Some(Statement::BlockStatement(ctx.ast.alloc(block))) } @@ -430,7 +430,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode { if pending_spread_elements.len() > 0 { // flush pending spread elements transformed_elements.push(ctx.ast.expression_array( - SPAN, + el_expr.span(), pending_spread_elements, None, )); @@ -444,14 +444,14 @@ impl<'a, 'b> PeepholeRemoveDeadCode { if pending_spread_elements.len() > 0 { transformed_elements.push(ctx.ast.expression_array( - SPAN, + array_expr.span, pending_spread_elements, None, )); } if transformed_elements.is_empty() { - return Some(ctx.ast.statement_empty(SPAN)); + return Some(ctx.ast.statement_empty(array_expr.span)); } else if transformed_elements.len() == 1 { return Some( ctx.ast.statement_expression(array_expr.span, transformed_elements.pop().unwrap()), @@ -535,10 +535,13 @@ impl<'a, 'b> PeepholeRemoveDeadCode { } if should_keep_as_sequence_expr && new_exprs.len() == 1 { - new_exprs.insert( - 0, - ctx.ast.expression_numeric_literal(SPAN, 1.0, None, NumberBase::Decimal), + let number = ctx.ast.expression_numeric_literal( + sequence_expr.span, + 1.0, + None, + NumberBase::Decimal, ); + new_exprs.insert(0, number); } if new_exprs.len() == 1 { diff --git a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs index bae92232c412e..865aaf96baa3e 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs @@ -6,7 +6,7 @@ use oxc_ecmascript::{ ToInt32, ToJsString, ToNumber, }; use oxc_span::cmp::ContentEq; -use oxc_span::{GetSpan, SPAN}; +use oxc_span::GetSpan; use oxc_syntax::{ es_target::ESTarget, identifier::is_identifier_name, @@ -282,7 +282,7 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { }; // XOR: We should use `!neg` when it is not in binary expression. let num = ctx.ast.expression_numeric_literal( - SPAN, + lit.span, if lit.value ^ no_unary { 0.0 } else { 1.0 }, None, NumberBase::Decimal, @@ -290,7 +290,7 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { Some(if no_unary { num } else { - ctx.ast.expression_unary(SPAN, UnaryOperator::LogicalNot, num) + ctx.ast.expression_unary(lit.span, UnaryOperator::LogicalNot, num) }) } @@ -308,9 +308,8 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { if let Statement::ReturnStatement(ret_stmt) = body { let return_stmt_arg = ret_stmt.argument.as_mut().map(|arg| ctx.ast.move_expression(arg)); - - if let Some(return_stmt_arg) = return_stmt_arg { - *body = ctx.ast.statement_expression(SPAN, return_stmt_arg); + if let Some(arg) = return_stmt_arg { + *body = ctx.ast.statement_expression(arg.span(), arg); arrow_expr.expression = true; self.changed = true; } @@ -744,9 +743,9 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { // The `_` will not be placed to the target code. let target = std::mem::replace( target, - ctx.ast.simple_assignment_target_identifier_reference(SPAN, "_"), + ctx.ast.simple_assignment_target_identifier_reference(target.span(), "_"), ); - Some(ctx.ast.expression_update(SPAN, UpdateOperator::Decrement, true, target)) + Some(ctx.ast.expression_update(expr.span, UpdateOperator::Decrement, true, target)) } Expression::UnaryExpression(un) if matches!(un.operator, UnaryOperator::UnaryNegation) => @@ -756,9 +755,9 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { // The `_` will not be placed to the target code. let target = std::mem::replace( target, - ctx.ast.simple_assignment_target_identifier_reference(SPAN, "_"), + ctx.ast.simple_assignment_target_identifier_reference(target.span(), "_"), ); - ctx.ast.expression_update(SPAN, UpdateOperator::Increment, true, target) + ctx.ast.expression_update(expr.span, UpdateOperator::Increment, true, target) }) } _ => None, @@ -793,11 +792,11 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { } Some(ctx.ast.expression_assignment( - SPAN, + expr.span, consequent.operator, ctx.ast.move_assignment_target(&mut alternate.left), ctx.ast.expression_conditional( - SPAN, + expr.span, ctx.ast.move_expression(&mut expr.test), ctx.ast.move_expression(&mut consequent.right), ctx.ast.move_expression(&mut alternate.right), @@ -869,7 +868,7 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { } Some(ctx.ast.expression_binary( span, - ctx.ast.expression_string_literal(SPAN, "", None), + ctx.ast.expression_string_literal(call_expr.span, "", None), BinaryOperator::Addition, ctx.ast.move_expression(arg), )) @@ -949,24 +948,18 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { if n.value.fract() == 0.0 { let n_int = n.value as usize; if (1..=6).contains(&n_int) { - return Some( - ctx.ast.expression_array( - span, - ctx.ast.vec_from_iter( - std::iter::from_fn(|| { - Some(ArrayExpressionElement::Elision( - ctx.ast.elision(SPAN), - )) - }) - .take(n_int), - ), - None, - ), - ); + let elisions = std::iter::from_fn(|| { + Some(ArrayExpressionElement::Elision(ctx.ast.elision(n.span))) + }) + .take(n_int); + return Some(ctx.ast.expression_array( + span, + ctx.ast.vec_from_iter(elisions), + None, + )); } } - - let callee = ctx.ast.expression_identifier_reference(SPAN, "Array"); + let callee = ctx.ast.expression_identifier_reference(n.span, "Array"); let args = ctx.ast.move_vec(args); Some(ctx.ast.expression_call(span, callee, NONE, args, false)) } @@ -979,7 +972,7 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { } // `new Array(x)` -> `Array(x)` else { - let callee = ctx.ast.expression_identifier_reference(SPAN, "Array"); + let callee = ctx.ast.expression_identifier_reference(span, "Array"); let args = ctx.ast.move_vec(args); Some(ctx.ast.expression_call(span, callee, NONE, args, false)) } diff --git a/crates/oxc_minifier/src/ast_passes/statement_fusion.rs b/crates/oxc_minifier/src/ast_passes/statement_fusion.rs index 01aed31d60c62..0c108052a68d0 100644 --- a/crates/oxc_minifier/src/ast_passes/statement_fusion.rs +++ b/crates/oxc_minifier/src/ast_passes/statement_fusion.rs @@ -1,7 +1,7 @@ use oxc_allocator::Vec; use oxc_ast::ast::*; use oxc_ecmascript::side_effects::MayHaveSideEffects; -use oxc_span::SPAN; +use oxc_span::GetSpan; use oxc_traverse::{traverse_mut_with_ctx, ReusableTraverseCtx, Traverse, TraverseCtx}; use crate::CompressorPass; @@ -114,7 +114,7 @@ impl<'a> StatementFusion { } else { exprs.push(ctx.ast.move_expression(&mut expr_stmt.expression)); } - *stmt = ctx.ast.statement_empty(SPAN); + *stmt = ctx.ast.statement_empty(expr_stmt.span); } else { break; } @@ -140,8 +140,12 @@ impl<'a> StatementFusion { if let Some(init) = for_stmt.init.as_mut() { init.as_expression_mut().unwrap() } else { + let span = Span::new( + exprs.first().map_or(0, |e| e.span().start), + exprs.last().map_or(0, |e| e.span().end), + ); for_stmt.init = - Some(ForStatementInit::from(ctx.ast.expression_sequence(SPAN, exprs))); + Some(ForStatementInit::from(ctx.ast.expression_sequence(span, exprs))); return; } } @@ -167,7 +171,11 @@ impl<'a> StatementFusion { } }; exprs.push(ctx.ast.move_expression(expr)); - *expr = ctx.ast.expression_sequence(SPAN, exprs); + let span = Span::new( + exprs.first().map_or(0, |e| e.span().start), + exprs.last().map_or(0, |e| e.span().end), + ); + *expr = ctx.ast.expression_sequence(span, exprs); } } diff --git a/crates/oxc_minifier/src/keep_var.rs b/crates/oxc_minifier/src/keep_var.rs index 828ddc1638427..8cb71089b423d 100644 --- a/crates/oxc_minifier/src/keep_var.rs +++ b/crates/oxc_minifier/src/keep_var.rs @@ -73,8 +73,7 @@ impl<'a> KeepVar<'a> { self.ast.variable_declarator(span, kind, id, None, false) })); - let decl = self.ast.alloc_variable_declaration(SPAN, kind, decls, false); - Some(decl) + Some(self.ast.alloc_variable_declaration(SPAN, kind, decls, false)) } pub fn get_variable_declaration_statement(self) -> Option> {