From c6bbf94f4c2e8d1ffa566e500fdbeeca71a6a443 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Tue, 10 Sep 2024 07:18:54 +0000 Subject: [PATCH] feat(minifier): constant fold unary expression (#5669) --- crates/oxc_ast/src/ast_impl/js.rs | 11 +++++-- crates/oxc_linter/src/ast_util.rs | 2 +- .../array_callback_return/return_checker.rs | 5 ++- .../src/rules/eslint/no_magic_numbers.rs | 10 +++--- crates/oxc_linter/src/utils/react.rs | 2 +- .../src/ast_passes/fold_constants.rs | 19 ++++++++---- .../ast_passes/substitute_alternate_syntax.rs | 16 ++++------ crates/oxc_minifier/src/node_util/mod.rs | 16 +++++++++- .../tests/ast_passes/fold_constants.rs | 31 +++++++++---------- .../ast_passes/substitute_alternate_syntax.rs | 3 ++ 10 files changed, 70 insertions(+), 45 deletions(-) diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs index e487e9773..2f16cc4ba 100644 --- a/crates/oxc_ast/src/ast_impl/js.rs +++ b/crates/oxc_ast/src/ast_impl/js.rs @@ -132,12 +132,16 @@ impl<'a> Expression<'a> { } } + pub fn is_number(&self) -> bool { + matches!(self, Self::NumericLiteral(_)) + } + /// Determines whether the given expr is a `0` pub fn is_number_0(&self) -> bool { matches!(self, Self::NumericLiteral(lit) if lit.value == 0.0) } - pub fn is_number(&self, val: f64) -> bool { + pub fn is_number_value(&self, val: f64) -> bool { matches!(self, Self::NumericLiteral(lit) if (lit.value - val).abs() < f64::EPSILON) } @@ -260,7 +264,10 @@ impl<'a> Expression<'a> { /// Returns literal's value converted to the Boolean type /// returns `true` when node is truthy, `false` when node is falsy, `None` when it cannot be determined. - pub fn get_boolean_value(&self) -> Option { + /// + /// 1. If argument is a Boolean, return argument. + /// 2. If argument is one of undefined, null, +0𝔽, -0𝔽, NaN, 0ℤ, or the empty String, return false. + pub fn to_boolean(&self) -> Option { match self { Self::BooleanLiteral(lit) => Some(lit.value), Self::NullLiteral(_) => Some(false), diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index 10b555169..d3e9cd26c 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -29,7 +29,7 @@ pub fn is_static_boolean<'a>(expr: &Expression<'a>, ctx: &LintContext<'a>) -> bo fn is_logical_identity(op: LogicalOperator, expr: &Expression) -> bool { match expr { expr if expr.is_literal() => { - let boolean_value = expr.get_boolean_value(); + let boolean_value = expr.to_boolean(); (op == LogicalOperator::Or && boolean_value == Some(true)) || (op == LogicalOperator::And && boolean_value == Some(false)) } diff --git a/crates/oxc_linter/src/rules/eslint/array_callback_return/return_checker.rs b/crates/oxc_linter/src/rules/eslint/array_callback_return/return_checker.rs index 7b6647bb1..e4dbbbf40 100644 --- a/crates/oxc_linter/src/rules/eslint/array_callback_return/return_checker.rs +++ b/crates/oxc_linter/src/rules/eslint/array_callback_return/return_checker.rs @@ -133,14 +133,13 @@ pub fn check_statement(statement: &Statement) -> StatementReturnStatus { let right = stmt.alternate.as_ref().map_or(StatementReturnStatus::NotReturn, check_statement); - test.get_boolean_value() - .map_or_else(|| left.join(right), |val| if val { left } else { right }) + test.to_boolean().map_or_else(|| left.join(right), |val| if val { left } else { right }) } Statement::WhileStatement(stmt) => { let test = &stmt.test; let inner_return = check_statement(&stmt.body); - if test.get_boolean_value() == Some(true) { + if test.to_boolean() == Some(true) { inner_return } else { inner_return.join(StatementReturnStatus::NotReturn) diff --git a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs index f36eddd44..b940a43f5 100644 --- a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs @@ -388,7 +388,7 @@ fn is_array_index<'a>(ast_kind: &AstKind<'a>, parent_kind: &AstKind<'a>) -> bool AstKind::MemberExpression(expression) => { if let MemberExpression::ComputedMemberExpression(computed_expression) = expression { - return computed_expression.expression.is_number(numeric.value) + return computed_expression.expression.is_number_value(numeric.value) && numeric.value >= 0.0 && numeric.value.fract() == 0.0 && numeric.value < f64::from(u32::MAX); @@ -635,7 +635,7 @@ fn test() { ( " type Others = [['a'], ['b']]; - + type Foo = { [K in keyof Others[0]]: Others[K]; }; @@ -703,7 +703,7 @@ fn test() { type Other = { [0]: 3; }; - + type Foo = { [K in keyof Other]: `${K & number}`; }; @@ -881,7 +881,7 @@ fn test() { ( " type Others = [['a'], ['b']]; - + type Foo = { [K in keyof Others[0]]: Others[K]; }; @@ -893,7 +893,7 @@ fn test() { type Other = { [0]: 3; }; - + type Foo = { [K in keyof Other]: `${K & number}`; }; diff --git a/crates/oxc_linter/src/utils/react.rs b/crates/oxc_linter/src/utils/react.rs index dbb47a8fb..7cf537185 100644 --- a/crates/oxc_linter/src/utils/react.rs +++ b/crates/oxc_linter/src/utils/react.rs @@ -81,7 +81,7 @@ pub fn is_hidden_from_screen_reader<'a>( Some(JSXAttributeValue::StringLiteral(s)) if s.value == "true" => true, Some(JSXAttributeValue::ExpressionContainer(container)) => { if let Some(expr) = container.expression.as_expression() { - expr.get_boolean_value().unwrap_or(false) + expr.to_boolean().unwrap_or(false) } else { false } diff --git a/crates/oxc_minifier/src/ast_passes/fold_constants.rs b/crates/oxc_minifier/src/ast_passes/fold_constants.rs index d21869b61..537cf1b25 100644 --- a/crates/oxc_minifier/src/ast_passes/fold_constants.rs +++ b/crates/oxc_minifier/src/ast_passes/fold_constants.rs @@ -49,6 +49,7 @@ impl<'a> FoldConstants<'a> { } // [optimizeSubtree](https://github.com/google/closure-compiler/blob/75335a5138dde05030747abfd3c852cd34ea7429/src/com/google/javascript/jscomp/PeepholeFoldConstants.java#L72) + // TODO: tryReduceOperandsForOp pub fn fold_expression(&self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { if let Some(folded_expr) = match expr { Expression::BinaryExpression(e) => self.try_fold_binary_operator(e, ctx), @@ -166,6 +167,17 @@ impl<'a> FoldConstants<'a> { ) -> Option> { match expr.operator { UnaryOperator::Typeof => self.try_fold_type_of(expr, ctx), + UnaryOperator::LogicalNot => { + expr.argument.to_boolean().map(|b| self.ast.expression_boolean_literal(SPAN, !b)) + } + // `-NaN` -> `NaN` + UnaryOperator::UnaryNegation if expr.argument.is_nan() => { + Some(ctx.ast.move_expression(&mut expr.argument)) + } + // `+1` -> `1` + UnaryOperator::UnaryPlus if expr.argument.is_number() => { + Some(ctx.ast.move_expression(&mut expr.argument)) + } _ => None, } } @@ -191,12 +203,7 @@ impl<'a> FoldConstants<'a> { | Expression::ArrayExpression(_) => "object", Expression::UnaryExpression(e) if e.operator == UnaryOperator::Void => "undefined", Expression::BigIntLiteral(_) => "bigint", - Expression::Identifier(ident) - if ident.name == "undefined" - && ctx.symbols().is_global_identifier_reference(ident) => - { - "undefined" - } + Expression::Identifier(ident) if ctx.is_identifier_undefined(ident) => "undefined", _ => return None, }; Some(self.ast.expression_string_literal(SPAN, s)) diff --git a/crates/oxc_minifier/src/ast_passes/substitute_alternate_syntax.rs b/crates/oxc_minifier/src/ast_passes/substitute_alternate_syntax.rs index a7a267adc..1b086fdb1 100644 --- a/crates/oxc_minifier/src/ast_passes/substitute_alternate_syntax.rs +++ b/crates/oxc_minifier/src/ast_passes/substitute_alternate_syntax.rs @@ -6,7 +6,7 @@ use oxc_syntax::{ }; use oxc_traverse::{Traverse, TraverseCtx}; -use crate::{CompressOptions, CompressorPass}; +use crate::{node_util::NodeUtil, CompressOptions, CompressorPass}; /// A peephole optimization that minimizes code by simplifying conditional /// expressions, replacing IFs with HOOKs, replacing object constructors @@ -67,8 +67,8 @@ impl<'a> Traverse<'a> for SubstituteAlternateSyntax<'a> { self.in_define_export = false; } - fn enter_expression(&mut self, expr: &mut Expression<'a>, _ctx: &mut TraverseCtx<'a>) { - if !self.compress_undefined(expr) { + fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { + if !self.compress_undefined(expr, ctx) { self.compress_boolean(expr); } } @@ -90,15 +90,11 @@ impl<'a> SubstituteAlternateSyntax<'a> { /* Utilities */ /// Transforms `undefined` => `void 0` - fn compress_undefined(&self, expr: &mut Expression<'a>) -> bool { - let Expression::Identifier(ident) = expr else { return false }; - if ident.name == "undefined" { - // if let Some(reference_id) = ident.reference_id.get() { - // && self.semantic.symbols().is_global_reference(reference_id) + fn compress_undefined(&self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) -> bool { + if ctx.is_expression_undefined(expr) { *expr = self.ast.void_0(); return true; - // } - } + }; false } diff --git a/crates/oxc_minifier/src/node_util/mod.rs b/crates/oxc_minifier/src/node_util/mod.rs index f67997b8e..15f8f7c81 100644 --- a/crates/oxc_minifier/src/node_util/mod.rs +++ b/crates/oxc_minifier/src/node_util/mod.rs @@ -26,6 +26,20 @@ pub trait NodeUtil { #[allow(unused)] fn scopes(&self) -> &ScopeTree; + fn is_expression_undefined(&self, expr: &Expression) -> bool { + if let Expression::Identifier(ident) = expr { + return self.is_identifier_undefined(ident); + }; + false + } + + fn is_identifier_undefined(&self, ident: &IdentifierReference) -> bool { + if ident.name == "undefined" && self.symbols().is_global_identifier_reference(ident) { + return true; + } + false + } + /// port from [closure compiler](https://github.com/google/closure-compiler/blob/a4c880032fba961f7a6c06ef99daa3641810bfdd/src/com/google/javascript/jscomp/AbstractPeepholeOptimization.java#L104-L114) /// Returns the number value of the node if it has one and it cannot have side effects. fn get_side_free_number_value(&self, expr: &Expression) -> Option { @@ -101,7 +115,7 @@ pub trait NodeUtil { Expression::Identifier(ident) => match ident.name.as_str() { "NaN" => Some(false), "Infinity" => Some(true), - "undefined" if self.symbols().is_global_identifier_reference(ident) => Some(false), + "undefined" if self.is_identifier_undefined(ident) => Some(false), _ => None, }, Expression::AssignmentExpression(assign_expr) => { diff --git a/crates/oxc_minifier/tests/ast_passes/fold_constants.rs b/crates/oxc_minifier/tests/ast_passes/fold_constants.rs index d491779b1..b57d59928 100644 --- a/crates/oxc_minifier/tests/ast_passes/fold_constants.rs +++ b/crates/oxc_minifier/tests/ast_passes/fold_constants.rs @@ -475,7 +475,7 @@ fn test_nan_comparison() { } #[test] -fn fold_typeof() { +fn js_typeof() { test("x = typeof 1", "x = \"number\""); test("x = typeof 'foo'", "x = \"string\""); test("x = typeof true", "x = \"boolean\""); @@ -494,7 +494,6 @@ fn fold_typeof() { } #[test] -#[ignore] fn unary_ops() { // TODO: need to port // These cases are handled by PeepholeRemoveDeadCode in closure-compiler. @@ -507,29 +506,29 @@ fn unary_ops() { test("a=!10", "a=false"); test("a=!false", "a=true"); test_same("a=!foo()"); - test("a=-0", "a=-0.0"); - test("a=-(0)", "a=-0.0"); + // test("a=-0", "a=-0.0"); + // test("a=-(0)", "a=-0.0"); test_same("a=-Infinity"); test("a=-NaN", "a=NaN"); test_same("a=-foo()"); - test("a=~~0", "a=0"); - test("a=~~10", "a=10"); - test("a=~-7", "a=6"); + // test("a=~~0", "a=0"); + // test("a=~~10", "a=10"); + // test("a=~-7", "a=6"); - test("a=+true", "a=1"); + // test("a=+true", "a=1"); test("a=+10", "a=10"); - test("a=+false", "a=0"); + // test("a=+false", "a=0"); test_same("a=+foo()"); test_same("a=+f"); - test("a=+(f?true:false)", "a=+(f?1:0)"); + // test("a=+(f?true:false)", "a=+(f?1:0)"); test("a=+0", "a=0"); - test("a=+Infinity", "a=Infinity"); - test("a=+NaN", "a=NaN"); - test("a=+-7", "a=-7"); - test("a=+.5", "a=.5"); + // test("a=+Infinity", "a=Infinity"); + // test("a=+NaN", "a=NaN"); + // test("a=+-7", "a=-7"); + // test("a=+.5", "a=.5"); - test("a=~0xffffffff", "a=0"); - test("a=~~0xffffffff", "a=-1"); + // test("a=~0xffffffff", "a=0"); + // test("a=~~0xffffffff", "a=-1"); // test_same("a=~.5", PeepholeFoldConstants.FRACTIONAL_BITWISE_OPERAND); } diff --git a/crates/oxc_minifier/tests/ast_passes/substitute_alternate_syntax.rs b/crates/oxc_minifier/tests/ast_passes/substitute_alternate_syntax.rs index e7fdaacf1..3def6cb1c 100644 --- a/crates/oxc_minifier/tests/ast_passes/substitute_alternate_syntax.rs +++ b/crates/oxc_minifier/tests/ast_passes/substitute_alternate_syntax.rs @@ -66,4 +66,7 @@ fn undefined() { test("for (undefined in {}) {}", "for(undefined in {}){}"); test("undefined++", "undefined++"); test("undefined += undefined", "undefined+=void 0"); + + // shadowd + test_same("(function(undefined) { let x = typeof undefined; })()"); }