Skip to content

Commit

Permalink
fix(minifier): fix remaining runtime bugs (oxc-project#6855)
Browse files Browse the repository at this point in the history
  • Loading branch information
Boshen committed Oct 24, 2024
1 parent a366fae commit a47c70e
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 127 deletions.
18 changes: 0 additions & 18 deletions crates/oxc_ast/src/ast_impl/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,24 +271,6 @@ impl<'a> Expression<'a> {
}
}

#[allow(missing_docs)]
pub fn is_immutable_value(&self) -> bool {
match self {
Self::BooleanLiteral(_)
| Self::NullLiteral(_)
| Self::NumericLiteral(_)
| Self::BigIntLiteral(_)
| Self::RegExpLiteral(_)
| Self::StringLiteral(_) => true,
Self::TemplateLiteral(lit) if lit.is_no_substitution_template() => true,
Self::UnaryExpression(unary_expr) => unary_expr.argument.is_immutable_value(),
Self::Identifier(ident) => {
matches!(ident.name.as_str(), "undefined" | "Infinity" | "NaN")
}
_ => false,
}
}

#[allow(missing_docs)]
pub fn is_require_call(&self) -> bool {
if let Self::CallExpression(call_expr) = self {
Expand Down
18 changes: 17 additions & 1 deletion crates/oxc_ecmascript/src/constant_evaluation/is_litral_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,22 @@ pub trait IsLiteralValue<'a, 'b> {
fn is_literal_value(&self, include_functions: bool) -> bool;
}

pub fn is_immutable_value(expr: &Expression<'_>) -> bool {
match expr {
Expression::BooleanLiteral(_)
| Expression::NullLiteral(_)
| Expression::NumericLiteral(_)
| Expression::BigIntLiteral(_)
| Expression::RegExpLiteral(_)
| Expression::StringLiteral(_) => true,
Expression::TemplateLiteral(lit) if lit.is_no_substitution_template() => true,
Expression::Identifier(ident) => {
matches!(ident.name.as_str(), "undefined" | "Infinity" | "NaN")
}
_ => false,
}
}

impl<'a, 'b> IsLiteralValue<'a, 'b> for Expression<'a> {
fn is_literal_value(&self, include_functions: bool) -> bool {
match self {
Expand All @@ -27,7 +43,7 @@ impl<'a, 'b> IsLiteralValue<'a, 'b> for Expression<'a> {
Self::ObjectExpression(expr) => {
expr.properties.iter().all(|property| property.is_literal_value(include_functions))
}
_ => self.is_immutable_value(),
_ => is_immutable_value(self),
}
}
}
Expand Down
26 changes: 7 additions & 19 deletions crates/oxc_ecmascript/src/constant_evaluation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,19 +222,7 @@ pub trait ConstantEvaluation<'a> {
let rval = self.eval_to_number(right)?;
let val = match e.operator {
BinaryOperator::Subtraction => lval - rval,
BinaryOperator::Division => {
if rval.is_zero() {
if lval.is_zero() || lval.is_nan() || lval.is_infinite() {
f64::NAN
} else if lval.is_sign_positive() {
f64::INFINITY
} else {
f64::NEG_INFINITY
}
} else {
lval / rval
}
}
BinaryOperator::Division => lval / rval,
BinaryOperator::Remainder => {
if rval.is_zero() {
f64::NAN
Expand Down Expand Up @@ -332,17 +320,17 @@ pub trait ConstantEvaluation<'a> {
fn eval_unary_expression(&self, expr: &UnaryExpression<'a>) -> Option<ConstantValue<'a>> {
match expr.operator {
UnaryOperator::Typeof => {
if !expr.argument.is_literal_value(true) {
return None;
}
let s = match &expr.argument {
Expression::ObjectExpression(_) | Expression::ArrayExpression(_)
if expr.argument.is_literal_value(true) =>
{
"object"
}
Expression::FunctionExpression(_) => "function",
Expression::StringLiteral(_) => "string",
Expression::NumericLiteral(_) => "number",
Expression::BooleanLiteral(_) => "boolean",
Expression::NullLiteral(_)
| Expression::ObjectExpression(_)
| Expression::ArrayExpression(_) => "object",
Expression::NullLiteral(_) => "object",
Expression::UnaryExpression(e) if e.operator == UnaryOperator::Void => {
"undefined"
}
Expand Down
42 changes: 16 additions & 26 deletions crates/oxc_ecmascript/src/string_to_number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,26 @@ pub trait StringToNumber {
/// <https://tc39.es/ecma262/#sec-stringtonumber>
impl StringToNumber for &str {
fn string_to_number(&self) -> f64 {
let s = self.trim_matches(is_trimmable_whitespace);

let s = *self;
match s {
"" => return 0.0,
"-Infinity" => return f64::NEG_INFINITY,
"Infinity" | "+Infinity" => return f64::INFINITY,
// Make sure that no further variants of "infinity" are parsed.
"inf" | "-inf" | "+inf" => return f64::NAN,
_ => {}
_ => {
// Make sure that no further variants of "infinity" are parsed by `f64::parse`.
// Note that alphabetical characters are not case-sensitive.
// <https://doc.rust-lang.org/std/primitive.f64.html#method.from_str>
let mut bytes = s.trim_start_matches(['-', '+']).bytes();
if bytes
.next()
.filter(|c| c.to_ascii_lowercase() == b'i')
.and_then(|_| bytes.next().filter(|c| c.to_ascii_lowercase() == b'n'))
.and_then(|_| bytes.next().filter(|c| c.to_ascii_lowercase() == b'f'))
.is_some()
{
return f64::NAN;
}
}
}

let mut bytes = s.bytes();
Expand Down Expand Up @@ -52,24 +63,3 @@ impl StringToNumber for &str {
s.parse::<f64>().unwrap_or(f64::NAN)
}
}

// <https://github.com/boa-dev/boa/blob/94d08fe4e68791ceca3c4b7d94ccc5f3588feeb3/core/string/src/lib.rs#L55>
/// Helper function to check if a `char` is trimmable.
pub(crate) const fn is_trimmable_whitespace(c: char) -> bool {
// The rust implementation of `trim` does not regard the same characters whitespace as ecma standard does
//
// Rust uses \p{White_Space} by default, which also includes:
// `\u{0085}' (next line)
// And does not include:
// '\u{FEFF}' (zero width non-breaking space)
// Explicit whitespace: https://tc39.es/ecma262/#sec-white-space
matches!(
c,
'\u{0009}' | '\u{000B}' | '\u{000C}' | '\u{0020}' | '\u{00A0}' | '\u{FEFF}' |
// Unicode Space_Separator category
'\u{1680}' | '\u{2000}'
..='\u{200A}' | '\u{202F}' | '\u{205F}' | '\u{3000}' |
// Line terminators: https://tc39.es/ecma262/#sec-line-terminators
'\u{000A}' | '\u{000D}' | '\u{2028}' | '\u{2029}'
)
}
8 changes: 6 additions & 2 deletions crates/oxc_linter/src/rules/eslint/prefer_numeric_literals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,11 @@ impl Rule for PreferNumericLiterals {
}

fn is_string_type(arg: &Argument) -> bool {
matches!(arg, Argument::StringLiteral(_) | Argument::TemplateLiteral(_))
match arg {
Argument::StringLiteral(_) => true,
Argument::TemplateLiteral(t) => t.is_no_substitution_template(),
_ => false,
}
}

fn is_parse_int_call(
Expand All @@ -123,7 +127,7 @@ fn check_arguments<'a>(call_expr: &CallExpression<'a>, ctx: &LintContext<'a>) {
}

let string_arg = &call_expr.arguments[0];
if !is_string_type(string_arg) || !string_arg.to_expression().is_immutable_value() {
if !is_string_type(string_arg) {
return;
}

Expand Down
6 changes: 5 additions & 1 deletion crates/oxc_linter/src/rules/import/no_dynamic_require.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ impl Rule for NoDynamicRequire {
}

fn is_static_value(expr: &Expression) -> bool {
expr.is_string_literal() && expr.is_immutable_value()
match expr {
Expression::StringLiteral(_) => true,
Expression::TemplateLiteral(t) => t.is_no_substitution_template(),
_ => false,
}
}

#[test]
Expand Down
15 changes: 14 additions & 1 deletion crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,20 @@ mod test {

test("x = Infinity / Infinity", "x = NaN");
test("x = Infinity % Infinity", "x = NaN");
test("x = Infinity / 0", "x = NaN");
test("x = Infinity / 0", "x = Infinity");
test("x = Infinity % 0", "x = NaN");
}

#[test]
fn test_to_number() {
test("x = +''", "x = 0");
test("x = +'+Infinity'", "x = Infinity");
test("x = +'-Infinity'", "x = -Infinity");

for op in ["", "+", "-"] {
for s in ["inf", "infinity", "INFINITY", "InFiNiTy"] {
test(&format!("x = +'{op}{s}'"), "x = NaN");
}
}
}
}
42 changes: 26 additions & 16 deletions crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,15 +204,11 @@ impl<'a, 'b> PeepholeRemoveDeadCode {
) -> Option<Statement<'a>> {
let test_boolean = for_stmt.test.as_ref().and_then(|test| ctx.get_boolean_value(test));
match test_boolean {
Some(false) => {
// Remove the entire `for` statement.
// Check vars in statement
let mut keep_var = KeepVar::new(ctx.ast);
keep_var.visit_statement(&for_stmt.body);

let mut var_decl = keep_var.get_variable_declaration();

if let Some(ForStatementInit::VariableDeclaration(var_init)) = &mut for_stmt.init {
Some(false) => match &mut for_stmt.init {
Some(ForStatementInit::VariableDeclaration(var_init)) => {
let mut keep_var = KeepVar::new(ctx.ast);
keep_var.visit_statement(&for_stmt.body);
let mut var_decl = keep_var.get_variable_declaration();
if var_init.kind.is_var() {
if let Some(var_decl) = &mut var_decl {
var_decl
Expand All @@ -222,14 +218,27 @@ impl<'a, 'b> PeepholeRemoveDeadCode {
var_decl = Some(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))
},
))
}

var_decl
.map(|var_decl| {
ctx.ast.statement_declaration(ctx.ast.declaration_from_variable(var_decl))
})
.or_else(|| Some(ctx.ast.statement_empty(SPAN)))
}
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))
},
))
}
_ => None,
},
Some(true) => {
// Remove the test expression.
for_stmt.test = None;
Expand Down Expand Up @@ -508,6 +517,7 @@ mod test {
fold("for (var se = [1, 2]; false;);", "var se = [1, 2];");
fold("for (var se = [1, 2]; false;) { var a = 0; }", "var se = [1, 2], a;");

fold("for (foo = bar; false;) {}", "for (foo = bar; false;);");
// fold("l1:for(;false;) { }", "");
}

Expand Down
44 changes: 1 addition & 43 deletions tasks/coverage/snapshots/runtime.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ commit: 06454619

runtime Summary:
AST Parsed : 18444/18444 (100.00%)
Positive Passed: 18273/18444 (99.07%)
Positive Passed: 18287/18444 (99.15%)
tasks/coverage/test262/test/annexB/language/function-code/if-decl-else-decl-a-func-existing-block-fn-no-init.js
minify error: Test262Error: Expected SameValue(«function f(){}», «undefined») to be true

Expand Down Expand Up @@ -318,9 +318,6 @@ minify error: SyntaxError: Identifier 'f' has already been declared
tasks/coverage/test262/test/annexB/language/global-code/if-stmt-else-decl-global-skip-early-err.js
minify error: SyntaxError: Identifier 'f' has already been declared

tasks/coverage/test262/test/language/expressions/addition/S11.6.1_A4_T5.js
minify error: Test262Error: #1.1: -0 + -0 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/array/spread-obj-getter-init.js
transform error: Test262Error: Expected SameValuetrue», «false») to be true

Expand All @@ -330,12 +327,6 @@ codegen error: Test262Error: descriptor value should be ; object value should be
tasks/coverage/test262/test/language/expressions/call/spread-obj-getter-init.js
transform error: Test262Error: Expected SameValuetrue», «false») to be true

tasks/coverage/test262/test/language/expressions/conditional/in-branch-1.js
minify error: Test262Error: Expected SameValue0», «1») to be true

tasks/coverage/test262/test/language/expressions/division/S11.5.2_A4_T8.js
minify error: Test262Error: #1.2: -0 / 1 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/does-not-equals/bigint-and-object.js
minify error: Test262Error: The result of (0n != {valueOf: function() {return 0n;}}) is false Expected SameValuetrue», «false») to be true

Expand All @@ -345,9 +336,6 @@ codegen error: Test262Error: p1 constructor is %Promise% Expected SameValue(«fu
tasks/coverage/test262/test/language/expressions/dynamic-import/eval-self-once-module.js
codegen error: Test262Error: global property was defined and incremented only once Expected SameValue2», «1») to be true

tasks/coverage/test262/test/language/expressions/dynamic-import/import-attributes/2nd-param-in.js
minify error: TypeError: Cannot read properties of undefined (reading 'then')

tasks/coverage/test262/test/language/expressions/dynamic-import/imported-self-update.js
codegen error: Test262Error: updated value, direct binding Expected SameValue0», «1») to be true

Expand Down Expand Up @@ -381,24 +369,6 @@ minify error: Test262Error: The result of (0n == {valueOf: function() {return 0n
tasks/coverage/test262/test/language/expressions/exponentiation/bigint-negative-exponent-throws.js
transform error: Test262Error: (-1n) ** -1n throws RangeError Expected a RangeError but got a TypeError

tasks/coverage/test262/test/language/expressions/logical-and/S11.11.1_A3_T2.js
minify error: Test262Error: #1.2: (-0 && -1) === -0

tasks/coverage/test262/test/language/expressions/logical-and/S11.11.1_A4_T2.js
minify error: Test262Error: #1.2: (-1 && -0) === -0

tasks/coverage/test262/test/language/expressions/logical-or/S11.11.2_A3_T2.js
minify error: Test262Error: #1.2: (0 || -0) === -0

tasks/coverage/test262/test/language/expressions/modulus/S11.5.3_A4_T2.js
minify error: Test262Error: #2.2: -1 % -1 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/modulus/S11.5.3_A4_T6.js
minify error: Test262Error: #3.2: -0 % 1 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/multiplication/S11.5.1_A4_T2.js
minify error: Test262Error: #6.2: 0 * -0 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/new/spread-obj-getter-init.js
transform error: Test262Error: Expected SameValuetrue», «false») to be true

Expand All @@ -414,21 +384,9 @@ transform error: TypeError: Cannot read properties of undefined (reading 'enumer
tasks/coverage/test262/test/language/expressions/object/object-spread-proxy-ownkeys-returned-keys-order.js
transform error: TypeError: Cannot read properties of undefined (reading 'enumerable')

tasks/coverage/test262/test/language/expressions/subtraction/S11.6.2_A4_T5.js
minify error: Test262Error: #3.2: -0 - 0 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/super/call-spread-obj-getter-init.js
transform error: Test262Error: Expected SameValuetrue», «false») to be true

tasks/coverage/test262/test/language/expressions/unary-plus/S11.4.6_A3_T3.js
minify error: Test262Error: #4: +"INFINITY" === Not-a-Number. Actual: Infinity

tasks/coverage/test262/test/language/expressions/unary-plus/S9.3_A4.2_T2.js
minify error: Test262Error: #3.2: +(-0) === -0. Actual: +0

tasks/coverage/test262/test/language/expressions/unary-plus/bigint-throws.js
minify error: Test262Error: +0n throws TypeError Expected a TypeError to be thrown but no exception was thrown at all

tasks/coverage/test262/test/language/global-code/decl-func.js
codegen error: Test262Error: descriptor should not be configurable

Expand Down

0 comments on commit a47c70e

Please sign in to comment.