Merge PR #2032: Formula entry: an incomplete formula's error describes what was typed (fixes #2030)

crates/formula/src/tests.rs: kept both tests.

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)?;

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();

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

crates/formula/src/parser.rs

@@ -28,6 +28,27 @@ pub fn parse(src: &str) -> Result<Expr, ParseError> {
     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<T>(src: &str, parse: impl Fn(&str) -> Result<T, ParseError>) -> Result<T, ParseError> {
+    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<Expr, ParseError> {
     let body = src.strip_prefix('=').unwrap_or(src);
     if body.len() > MAX_FORMULA_LEN && body.chars().count() > MAX_FORMULA_LEN {

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");
+}

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.