From 110c300bfc28520f710826da64513853c1c0651f Mon Sep 17 00:00:00 2001 From: copilot-swe-agent <198982749+copilot-swe-agent@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:39:22 +0000 Subject: [PATCH] fix(oxc_ecmascript): `+[false]` and `+[true]` should evaluate to `NaN` (#19174) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Fix minifier incorrectly handling `+[false]` and `+[true]` - [x] Explore repository structure and locate the bug - [x] Identify the root cause in `to_number` implementation for arrays - [x] Add test case for the bug in minifier fold_constants tests - [x] Add test case in to_number unit tests - [x] Fix the bug in `to_number.rs` to properly convert arrays to strings first - [x] Run tests to verify the fix - all tests passing - [x] Manual verification with minifier example - outputs NaN correctly - [x] Run code review - no issues found - [x] Run security check - no vulnerabilities found - [x] Comprehensive testing of all array conversion cases - all correct - [x] Apply cargo fmt to fix formatting - [x] Address PR feedback: hoist use statements, simplify comments and code - [x] Address bot feedback: optimize count, remove redundant import, add test coverage **Summary:** Fixed a bug where `+[false]` and `+[true]` were incorrectly minified to `0` and `1` instead of `NaN`. The issue was in the `to_number` implementation for arrays in `oxc_ecmascript`, which was bypassing the proper ECMAScript conversion chain (array → string → number). Now correctly converts arrays to strings first via `to_js_string`, then converts the string to a number.
Original prompt > > ---- > > *This section details on the original issue you should resolve* > > minifier: `+[false]` and `+[true]` are incorrectly minified > > I was messing around with [jsfuck](https://jsfuck.com/) and found out that oxc_minifier doesn't minify `+[false]` or `+[true]` correctly. It is supposed to evalutate to `NaN`, but Oxc just grabs the boolean and uses that. This makes the 'minified' jsfuck code invalid (not that I'm using jsfuck on a day to day basis) > > Original: > ```ts > const a = +[false] > const b = +[true] > ``` > > Oxc minified: > ```ts > const a = 0 > const b = 1; > ``` > > Expected: > ```ts > const a = NaN > const b = NaN; > ``` > > [Playground](https://playground.oxc.rs/?code=const+a+%3D+%2B%5Bfalse%5D%0Aconst+b+%3D+%2B%5Btrue%5D) (enable minify syntax) > > > > probably there a bug in the constant evaluator in oxc_ecmascript that handles `UnaryExpression` > > ## Comments on the Issue (you are @copilot in this section) > > > >
- Fixes oxc-project/oxc#19084 --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --- crates/oxc_ecmascript/src/to_number.rs | 45 ++++++++----------- .../tests/ecmascript/to_number.rs | 15 +++++++ .../tests/peephole/fold_constants.rs | 5 +++ 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/crates/oxc_ecmascript/src/to_number.rs b/crates/oxc_ecmascript/src/to_number.rs index 1e70078d339c6..cf14265426c62 100644 --- a/crates/oxc_ecmascript/src/to_number.rs +++ b/crates/oxc_ecmascript/src/to_number.rs @@ -1,7 +1,8 @@ use oxc_ast::ast::*; use crate::{ - GlobalContext, to_primitive::maybe_object_with_to_primitive_related_properties_overridden, + GlobalContext, StringToNumber, ToJsString, + to_primitive::maybe_object_with_to_primitive_related_properties_overridden, }; /// `ToNumber` @@ -28,10 +29,7 @@ impl<'a> ToNumber<'a> for Expression<'a> { "NaN" | "undefined" if ctx.is_global_reference(ident) => Some(f64::NAN), _ => None, }, - Expression::StringLiteral(lit) => { - use crate::StringToNumber; - Some(lit.value.as_str().string_to_number()) - } + Expression::StringLiteral(lit) => Some(lit.value.as_str().string_to_number()), Expression::UnaryExpression(unary) if unary.operator.is_not() => { let number = unary.argument.to_number(ctx)?; Some(if number == 0.0 { 1.0 } else { 0.0 }) @@ -49,31 +47,26 @@ impl<'a> ToNumber<'a> for Expression<'a> { // `ToPrimitive` for RegExp object returns `"/regexp/"` Expression::RegExpLiteral(_) => Some(f64::NAN), Expression::ArrayExpression(arr) => { - // If the array is empty, `ToPrimitive` returns `""` - if arr.elements.is_empty() { - return Some(0.0); - } - if arr.elements.len() == 1 { - let first_element = arr.elements.first().unwrap(); - return match first_element { - ArrayExpressionElement::SpreadElement(_) => None, - // `ToPrimitive` returns `""` for `[,]` - ArrayExpressionElement::Elision(_) => Some(0.0), - match_expression!(ArrayExpressionElement) => { - first_element.to_expression().to_number(ctx) - } - }; - } + // ToNumber for arrays: + // 1. ToPrimitive(array, hint Number) -> tries valueOf, then toString + // 2. Array.toString() -> Array.join(",") + // 3. ToNumber(resultString) - let non_spread_element_count = arr + // Fast path: if array has at least 2 non-spread elements, + // the result will contain "," which converts to NaN + if arr .elements .iter() .filter(|e| !matches!(e, ArrayExpressionElement::SpreadElement(_))) - .count(); - // If the array has at least 2 elements, `ToPrimitive` returns a string containing - // `,` which is not included in `StringNumericLiteral` - // So `ToNumber` returns `NaN` - if non_spread_element_count >= 2 { Some(f64::NAN) } else { None } + .take(2) + .count() + >= 2 + { + return Some(f64::NAN); + } + + let array_string = arr.to_js_string(ctx)?; + Some(array_string.as_ref().string_to_number()) } _ => None, } diff --git a/crates/oxc_minifier/tests/ecmascript/to_number.rs b/crates/oxc_minifier/tests/ecmascript/to_number.rs index 35c7d60b60c64..6a52bbbdd173c 100644 --- a/crates/oxc_minifier/tests/ecmascript/to_number.rs +++ b/crates/oxc_minifier/tests/ecmascript/to_number.rs @@ -37,4 +37,19 @@ fn test() { assert!(global_undefined_number.is_some_and(f64::is_nan)); assert!(empty_object_number.is_some_and(f64::is_nan)); assert_eq!(object_with_to_string_number, None); + + // Test arrays with boolean elements - should convert to NaN + let false_literal = ast.alloc_boolean_literal(SPAN, false); + let true_literal = ast.alloc_boolean_literal(SPAN, true); + let array_with_false = + ast.expression_array(SPAN, ast.vec1(ArrayExpressionElement::BooleanLiteral(false_literal))); + let array_with_true = + ast.expression_array(SPAN, ast.vec1(ArrayExpressionElement::BooleanLiteral(true_literal))); + let array_with_false_number = array_with_false.to_number(&WithoutGlobalReferenceInformation {}); + let array_with_true_number = array_with_true.to_number(&WithoutGlobalReferenceInformation {}); + + // [false].toString() = "false", Number("false") = NaN + assert!(array_with_false_number.is_some_and(f64::is_nan)); + // [true].toString() = "true", Number("true") = NaN + assert!(array_with_true_number.is_some_and(f64::is_nan)); } diff --git a/crates/oxc_minifier/tests/peephole/fold_constants.rs b/crates/oxc_minifier/tests/peephole/fold_constants.rs index f52179ef57e7f..23325561bff1f 100644 --- a/crates/oxc_minifier/tests/peephole/fold_constants.rs +++ b/crates/oxc_minifier/tests/peephole/fold_constants.rs @@ -502,6 +502,11 @@ fn test_fold_unary() { fold("a=+[0, 1]", "a=NaN"); test_same("var foo; NOOP(a=+[0, ...foo])"); // can be either `a=0` or `a=NaN` (also `...foo` may have a side effect) test("var foo; NOOP(a=+[0, ...[foo ? 'foo': ''], 1])", "var foo; NOOP(a=NaN)"); + + fold("a=+[false]", "a=NaN"); // `+"false"` + fold("a=+[true]", "a=NaN"); // `+"true"` + fold("a=+[undefined]", "a=0"); // `+""` + fold("a=+[null]", "a=0"); // `+""` } #[test]