diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index fe7f1fe8b..d1283bd2d 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -139,15 +139,7 @@ pub(crate) fn input_to_cell(input: &str, sheet: usize, at: CellRef, wb: &mut gri if is_formula { let body = input.strip_prefix('=').unwrap_or(input); // Excel closes missing parentheses for you. - let mut text = body.to_string(); - let mut parsed = gridcraft_formula::parse(&text); - for _ in 0..8 { - if parsed.is_ok() { - break; - } - text.push(')'); - parsed = gridcraft_formula::parse(&text); - } + let parsed = gridcraft_formula::parse_closing_parens(body, gridcraft_formula::parse); let expr = parsed.map_err(|e| EngineError::Other(format!("There's a problem with this formula: {e}")))?; let expr = wb.name_call_case(expr); check_arg_counts(&expr)?; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2ac290654..637b4a9dc 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -241,6 +241,21 @@ fn formulas_with_the_wrong_argument_count_are_refused_on_entry() { assert_eq!(v(&s, "C1"), Value::text("=DATEDIF(1,2)")); } +#[test] +fn an_incomplete_formula_is_refused_with_an_error_about_what_was_typed() { + // #2030: `=1+` said "unexpected `)`", although no `)` was typed: entry closes missing + // parentheses and reported the error of its last retry. + let mut s = s(); + for (input, msg) in [("=1+", "unexpected end of formula"), ("=SUM(1,+", "unexpected end of formula")] { + let e = s.execute("cell.set", json!({"cell": "A1", "input": input})).unwrap_err().to_string(); + assert!(e.contains("problem with this formula") && e.contains(msg), "{input}: {e}"); + assert!(!e.contains('`'), "{input}: {e}"); + } + // Missing closing parentheses are still added. + s.execute("cell.set", json!({"cell": "A1", "input": "=SUM(1,(2+3"})).unwrap(); + assert_eq!(v(&s, "A1"), Value::Number(6.0)); +} + #[test] fn undo_redo() { let mut s = s(); diff --git a/crates/formula/src/lib.rs b/crates/formula/src/lib.rs index 32ac0e17c..0861b6763 100644 --- a/crates/formula/src/lib.rs +++ b/crates/formula/src/lib.rs @@ -12,7 +12,7 @@ pub mod printer; pub use ast::{Anchor, BinOp, Expr, RefKind, Reference, SheetSel, StructItem, StructRef, TrimMode, UnOp}; pub use input::{FormulaLocale, parse_input, print_input}; -pub use parser::{MAX_FORMULA_LEN, ParseError, normalize_function_name, parse}; +pub use parser::{MAX_FORMULA_LEN, ParseError, normalize_function_name, parse, parse_closing_parens}; pub use printer::{print, print_r1c1, quote_sheet}; /// Whether two LET/LAMBDA names are the same: case-insensitive in any script, as in Excel diff --git a/crates/formula/src/parser.rs b/crates/formula/src/parser.rs index 555717be9..a100d8486 100644 --- a/crates/formula/src/parser.rs +++ b/crates/formula/src/parser.rs @@ -28,6 +28,27 @@ pub fn parse(src: &str) -> Result { parse_syntax(src, crate::FormulaLocale::En) } +/// Most closing parentheses [`parse_closing_parens`] adds. +const MAX_CLOSED_PARENS: usize = 8; + +/// Parses formula text as typed into a cell with `parse`, closing up to eight missing `)` at the +/// end as Excel does (`SUM(1,2` enters `SUM(1,2)`). When no number of `)` makes it parse, the +/// error describes the text as typed: `1+` is "unexpected end of formula", not "unexpected `)`". +pub fn parse_closing_parens(src: &str, parse: impl Fn(&str) -> Result) -> Result { + let typed = match parse(src) { + Ok(parsed) => return Ok(parsed), + Err(e) => e, + }; + let mut text = src.to_string(); + for _ in 0..MAX_CLOSED_PARENS { + text.push(')'); + if let Ok(parsed) = parse(&text) { + return Ok(parsed); + } + } + Err(typed) +} + pub(crate) fn parse_syntax(src: &str, locale: crate::FormulaLocale) -> Result { let body = src.strip_prefix('=').unwrap_or(src); if body.len() > MAX_FORMULA_LEN && body.chars().count() > MAX_FORMULA_LEN { diff --git a/crates/formula/src/tests.rs b/crates/formula/src/tests.rs index 8c7d85ea7..e898f5657 100644 --- a/crates/formula/src/tests.rs +++ b/crates/formula/src/tests.rs @@ -470,3 +470,16 @@ fn walk_is_preorder_and_map_is_postorder_left_to_right() { assert_eq!(mapped, ["A1", "B1", "A1+B1", "(A1+B1)", "C1", "D1", "SUM(C1,D1)", "(A1+B1)+SUM(C1,D1)", "E1", "(A1+B1)+SUM(C1,D1)-E1"]); assert_eq!(print(&out), "(A1+B1)+SUM(C1,D1)-E1"); } + +#[test] +fn missing_closing_parentheses_are_closed_and_errors_describe_the_typed_text() { + // #2030: `1+` reported "unexpected `)`", the error of the last retry with `)` appended. + let entry = |src: &str| parse_closing_parens(src, parse); + assert_eq!(entry("SUM(1,2").map(|e| print(&e)), Ok("SUM(1,2)".to_string())); + assert_eq!(entry("((1+2").map(|e| print(&e)), Ok("((1+2))".to_string())); + assert_eq!(entry("1+2").map(|e| print(&e)), Ok("1+2".to_string())); + for src in ["1+", "--", "SUM(1,+", "1+*2", &format!("{}1", "(".repeat(9))] { + assert_eq!(entry(src).unwrap_err(), parse(src).unwrap_err(), "{src}"); + } + assert_eq!(entry("1+").unwrap_err().msg, "unexpected end of formula"); +} diff --git a/crates/ui-egui/src/formula_locale.rs b/crates/ui-egui/src/formula_locale.rs index 6e833b95d..aa0d60188 100644 --- a/crates/ui-egui/src/formula_locale.rs +++ b/crates/ui-egui/src/formula_locale.rs @@ -27,15 +27,7 @@ pub(crate) fn canonical( { let known = |name: &str| wb.name(name, sheet).is_some() || wb.table(name).is_some(); // Match the engine's existing convenience for missing closing parentheses. - let mut body = body.to_string(); - let mut expr = formula::parse_input(&body, locale, known); - for _ in 0..8 { - if expr.is_ok() { - break; - } - body.push(')'); - expr = formula::parse_input(&body, locale, known); - } + let expr = formula::parse_closing_parens(body, |text| formula::parse_input(text, locale, known)); return Ok(format!("={}", formula::print(&expr?))); } // Decimal-comma number entry follows the same preference, without rewriting text or dates.