diff --git a/crates/expr/src/rewrite.rs b/crates/expr/src/rewrite.rs index 34b52439b..69762d21e 100644 --- a/crates/expr/src/rewrite.rs +++ b/crates/expr/src/rewrite.rs @@ -334,7 +334,7 @@ fn raw(item: &Item) -> Piece { Item::Tok(t) => Piece { ws: t.ws.clone(), body: t.text.clone() }, Item::Group { open, inner, close } => { let mut body = open.text.clone(); - body.push_str(&rewrite_seq(inner)); + body.push_str(&rewrite_seq(inner, open.text == "{")); if let Some(c) = close { body.push_str(&c.ws); body.push_str(&c.text); @@ -378,25 +378,7 @@ fn operand(items: &[Item], i: usize) -> Option<(Piece, usize)> { }; return Some((Piece { ws: t.ws.clone(), body }, j)); } - let mut p = raw(first); - let mut j = i + 1; - // `function name(args) { body }` is one atom. - if first.tok().is_some_and(|t| t.is_kw(&["function"])) { - if items.get(j).is_some_and(|x| x.is("*")) { - push(&mut p, &items[j]); - j += 1; - } - if items.get(j).and_then(Item::tok).is_some_and(|t| t.kind == Kind::Ident) { - push(&mut p, &items[j]); - j += 1; - } - for g in ["(", "{"] { - if items.get(j).and_then(Item::group) == Some(g) { - push(&mut p, &items[j]); - j += 1; - } - } - } + let (mut p, mut j) = if is_function(first) { function_atom(items, i) } else { (raw(first), i + 1) }; // Postfix: member access, calls, indexing, tagged templates, `x++`. while let Some(it) = items.get(j) { let member = it.is(".") || it.is("?."); @@ -428,6 +410,59 @@ fn operand(items: &[Item], i: usize) -> Option<(Piece, usize)> { Some((p, j)) } +fn is_function(item: &Item) -> bool { + item.tok().is_some_and(|t| t.is_kw(&["function"])) +} + +/// `function [*] [name] (args) { body }` from the `function` keyword at `i`, as one atom. +fn function_atom(items: &[Item], i: usize) -> (Piece, usize) { + let Some(first) = items.get(i) else { + // Unreachable for callers (they pass a `function` item); still advance so no loop stalls. + return (Piece { ws: String::new(), body: String::new() }, i.saturating_add(1)); + }; + let mut p = raw(first); + let mut j = i + 1; + if items.get(j).is_some_and(|x| x.is("*")) { + push(&mut p, &items[j]); + j += 1; + } + if items.get(j).and_then(Item::tok).is_some_and(|t| t.kind == Kind::Ident) { + push(&mut p, &items[j]); + j += 1; + } + for g in ["(", "{"] { + if items.get(j).and_then(Item::group) == Some(g) { + push(&mut p, &items[j]); + j += 1; + } + } + (p, j) +} + +/// Is the `function` keyword at `i` at the start of a statement, so a function declaration +/// (ECMAScript: an expression statement cannot start with `function`)? A declaration ends at its +/// `}`: whatever follows is a new statement, never an operand of it. `stmts` says whether +/// `items` is a statement list (the whole expression or a `{ … }` block) rather than the inside +/// of `( … )` or `[ … ]`. +fn at_statement_start(items: &[Item], i: usize, stmts: bool) -> bool { + // `async function`: the statement starts at `async` (no line break may come between them). + let is_async = |k: usize| items.get(k).and_then(Item::tok).is_some_and(|a| a.is_kw(&["async"])); + let at = match i.checked_sub(1) { + Some(k) if is_async(k) && !items.get(i).is_some_and(Item::nl) => k, + _ => i, + }; + let Some(prev) = at.checked_sub(1).and_then(|k| items.get(k)) else { + return stmts; + }; + match prev { + // A block, an `if (…)`/`while (…)` header or a value the line break ends. + Item::Group { .. } => true, + // Known limit: after a label, `case …:` or `default:` a declaration is still read as an + // expression (that `:` cannot be told from an object literal's or a conditional's here). + Item::Tok(t) => t.is(";") || t.is_kw(&["else", "do"]) || ends_value(t), + } +} + fn push(p: &mut Piece, item: &Item) { let r = raw(item); p.body.push_str(&r.ws); @@ -510,7 +545,9 @@ fn assignment_end(items: &[Item], i: usize) -> usize { j } -fn rewrite_seq(items: &[Item]) -> String { +/// Rewrite a sequence of items. `stmts`: the sequence is a statement list (the whole expression +/// or the inside of `{ … }`), not the inside of `( … )` or `[ … ]`. +fn rewrite_seq(items: &[Item], stmts: bool) -> String { let mut out = String::new(); let mut i = 0; while i < items.len() { @@ -521,12 +558,20 @@ fn rewrite_seq(items: &[Item]) -> String { i += 2; continue; } + // A function declaration ends at its `}` (#3212): `function f() {}\n-1` is two statements, + // so the next line must not become an operand (`__sub(function f() {}, 1)` is NaN). + if is_function(it) && at_statement_start(items, i, stmts) { + let (p, j) = function_atom(items, i); + push_piece(&mut out, p); + i = j; + continue; + } if let Some((p, j)) = chain(items, i) { // Compound assignment: `x += v` → `x = __add(x, (v))` (the target is evaluated twice, // which only matters for targets with side effects). if let Some(op) = items.get(j).and_then(Item::tok).filter(|t| t.kind == Kind::Punct && matches!(t.text.as_str(), "+=" | "-=" | "*=" | "/=")) { let end = assignment_end(items, j + 1); - let rhs = rewrite_seq(&items[j + 1..end]); + let rhs = rewrite_seq(&items[j + 1..end], false); let h = helper(&op.text[..1]).unwrap_or("__add"); let target = compound_assignment_target(items, i, j); out.push_str(&p.ws); @@ -555,7 +600,7 @@ fn raw_ref(item: &Item) -> String { out.push_str(&raw_ref(item)); } } else { - out.push_str(&rewrite_seq(inner)); + out.push_str(&rewrite_seq(inner, open.text == "{")); } if let Some(close) = close { out.push_str(&close.text); @@ -794,7 +839,7 @@ pub fn check_nesting(src: &str) -> Result<(), String> { if too_nested(&toks) { return Err(too_deep()); } - let mut out = rewrite_seq(&build(toks)); + let mut out = rewrite_seq(&build(toks), true); out.push_str(&tail); if over_rewrite_limit(&out) { return Err(too_large()); @@ -825,7 +870,7 @@ pub fn rewrite(src: &str) -> String { if too_nested(&toks) { return throw_too_deep(); } - let mut out = rewrite_seq(&build(toks)); + let mut out = rewrite_seq(&build(toks), true); out.push_str(&tail); if over_rewrite_limit(&out) { return throw_too_large(); @@ -897,6 +942,26 @@ mod tests { assert_eq!(n(&rewrite("2 ** 3 * 2")), n("__mul(2 ** 3, 2)")); } + #[test] + fn a_function_declaration_is_not_an_operand_of_the_next_statement() { + // #3212: a declaration ends at its `}` (an expression statement cannot start with + // `function`), so a following `-`, `+` or `[` starts a new statement. + assert_eq!(n(&rewrite("function f(x){ return x; }\n-50 + 100")), n("function f(x){ return x; }\n__add(-50, 100)")); + assert_eq!(n(&rewrite("function f(){}\n+a - b")), n("function f(){}\n__sub(+a, b)")); + assert_eq!(n(&rewrite("function f(){}\n[1, 2] - [1, 1]")), n("function f(){}\n__sub([1, 2], [1, 1])")); + assert_eq!(n(&rewrite("function f(){}\n(a) - b")), n("function f(){}\n__sub((a), b)")); + assert_eq!(n(&rewrite("function* g(){} -x")), n("function* g(){} __neg(x)")); + assert_eq!(n(&rewrite("async function f(){}\n-x")), n("async function f(){}\n__neg(x)")); + assert_eq!(n(&rewrite("var a = 1;function f(){}\n-x")), n("var a = 1;function f(){}\n__neg(x)")); + assert_eq!(n(&rewrite("if (a) { function f(){}\n-b }")), n("if (a) { function f(){}\n__neg(b) }")); + // Function expressions stay operands, whatever follows on the next line. + assert_eq!(n(&rewrite("x = function(a){ return a } - 1")), n("x = __sub(function(a){ return a }, 1)")); + assert_eq!(n(&rewrite("var g = function(){}\n- 1")), n("var g = __sub(function(){},\n 1)")); + assert_eq!(n(&rewrite("(function f(){} - 1)")), n("(__sub(function f(){}, 1))")); + assert_eq!(n(&rewrite("[function(){} - 1]")), n("[__sub(function(){}, 1)]")); + assert_eq!(n(&rewrite("o = {k: function(){} - 1}")), n("o = {k: __sub(function(){}, 1)}")); + } + #[test] fn source_expansion_is_bounded_before_rewriting() { let src = "x += 1;".repeat(super::MAX_REWRITTEN_BYTES / 6 + 1); diff --git a/crates/expr/src/tests.rs b/crates/expr/src/tests.rs index 361f12b87..4329d9c21 100644 --- a/crates/expr/src/tests.rs +++ b/crates/expr/src/tests.rs @@ -193,6 +193,22 @@ fn functions_and_let_are_reusable_across_evaluations() { assert_close!(f.num(e, 2.0), 9.0); } +#[test] +fn a_function_declaration_ends_at_its_brace_so_a_leading_minus_starts_a_new_statement() { + // #3212: the array-math rewrite took `function … {…}\n-50 + 100` for one expression, + // `(function … {…}) - 50 + 100`, which is NaN (and left the function undeclared). + let f = fx(); + assert_close!(f.num("function same3212(x){ return x; }\n-50 + 100", 0.0), 50.0); + // The declaration really declared the function (globals do not outlive one evaluation). + assert_close!(f.num("function same3212(x){ return x; }\n-50 + 100;\nsame3212(7)", 0.0), 7.0); + assert_close!(f.num("function twice3212(x){ return x * 2; }\n-twice3212(10) + 100", 0.0), 80.0); + assert_close!(f.num("function seven3212(){ return 7; }\n+seven3212()", 0.0), 7.0); + assert_close!(f.num("function one3212(){ return 1; } -one3212() * 3", 0.0), -3.0); + assert_close!(f.num("if (time >= 0) { function neg3212(x){ return -x; }\n-neg3212(4) }", 0.0), 4.0); + // A function expression stays an operand: `function - 1` is NaN in JavaScript too. + assert_close!(f.num("var g3212 = function(){ return 1; }\n- 1;\ntypeof g3212 == 'number' ? 5 : 6", 0.0), 5.0); +} + #[test] fn an_expression_cannot_rebind_a_prelude_name_for_the_next_one() { let f = fx();