Names: a name over several ranges reads its top-level commas as unions (fixes #728)

crates/calc/src/eval.rs

@@ -247,7 +247,7 @@ impl<'h> Evaluator<'h> {
         if let Some(def) = wb.name(bare, scope_sheet) {
             let text = def.formula.clone();
             let scope = def.scope.unwrap_or(self.sheet);
-            return match gridcraft_formula::parse(&text) {
+            return match gridcraft_formula::parse_name(&text) {
                 Ok(expr) => {
                     // Relative references in a name are relative to A1: used here, they move by
                     // this cell's offset from it.

crates/calc/src/tests.rs

@@ -515,6 +515,24 @@ fn names_and_sheets() {
     assert_eq!(t.num("A1"), 5.0);
 }
 
+#[test]
+fn name_referring_to_several_ranges() {
+    let mut t = T::new();
+    // As Excel writes it in `<definedName>`: a union with no parentheses around it.
+    let union = "Sheet1!$A$1:$A$2,Sheet1!$C$1:$C$2";
+    t.wb.names.push(gridcraft_model::DefinedName { name: "u".into(), scope: None, formula: union.into(), comment: String::new(), hidden: false });
+    for (at, v) in [("A1", "1"), ("A2", "2"), ("C1", "5"), ("C2", "7")] {
+        t.set(at, v);
+    }
+    t.set("E1", "=SUM(u)");
+    t.set("E2", "=AREAS(u)");
+    t.set("E3", "=MIN(u)");
+    t.set("E4", "=INDEX(u,2,1,2)");
+    assert_eq!((t.num("E1"), t.num("E2"), t.num("E3"), t.num("E4")), (15.0, 2.0, 1.0, 7.0));
+    t.set("C2", "70");
+    assert_eq!(t.num("E1"), 78.0);
+}
+
 #[test]
 fn legacy_array_formulas_fill_their_range() {
     let mut t = T::new();

crates/engine/src/cmd/edit.rs

@@ -1206,7 +1206,7 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: &
         }
     }
     for n in wb.names.iter_mut() {
-        if let Ok(expr) = gridcraft_formula::parse(&n.formula) {
+        if let Ok(expr) = gridcraft_formula::parse_name(&n.formula) {
             // Names are workbook-level: unqualified refs don't occur; qualified ones adjust.
             let new = gridcraft_formula::adjust::adjust(expr, "\u{0}", target, e);
             n.formula = gridcraft_formula::print(&new);

crates/engine/src/cmd/formulas.rs

@@ -265,7 +265,7 @@ fn define_name(s: &mut Session, p: &Json) -> Result<Json> {
     let refers = match str_param(p, "refersTo") {
         // As in Excel, relative references are relative to the active cell; names keep them
         // relative to A1, as files do.
-        Some(r) => match gridcraft_formula::parse(r.trim_start_matches('=')) {
+        Some(r) => match gridcraft_formula::parse_name(r.trim_start_matches('=')) {
             Ok(e) if gridcraft_formula::adjust::has_relative_refs(&e) => {
                 let at = d.selection.active;
                 gridcraft_formula::print(&gridcraft_formula::adjust::shift_relative_wrapping(e, -(at.row as i64), -(at.col as i64)))
@@ -280,7 +280,7 @@ fn define_name(s: &mut Session, p: &Json) -> Result<Json> {
             format!("{}!{}", gridcraft_formula::quote_sheet(&sh.name), body)
         }
     };
-    if gridcraft_formula::parse(&refers).is_err() {
+    if gridcraft_formula::parse_name(&refers).is_err() {
         return Err(EngineError::Other("There's a problem with this formula.".into()));
     }
     let comment = str_param(p, "comment").unwrap_or("").to_string();
@@ -355,7 +355,7 @@ fn rename_name_in_formulas(wb: &mut gridcraft_model::Workbook, old: &str, old_sc
     }
     for n in wb.names.iter_mut() {
         if n.scope.is_none_or(applies)
-            && let Some(e) = gridcraft_formula::parse(&n.formula).ok().and_then(rename)
+            && let Some(e) = gridcraft_formula::parse_name(&n.formula).ok().and_then(rename)
         {
             n.formula = gridcraft_formula::print(&e);
         }
@@ -610,4 +610,27 @@ mod tests {
         let r = s.execute("formulas.defineName", json!({"name": "First", "refersTo": "=Sheet1!$A$1"})).unwrap();
         assert_eq!(r["refersTo"], "=Sheet1!$A$1");
     }
+
+    #[test]
+    fn names_referring_to_several_ranges() {
+        let mut s = Session::new();
+        s.new_workbook();
+        s.execute("range.setValues", json!({"range": "A1:C2", "values": [[1, 0, 5], [2, 0, 7]]})).unwrap();
+        // Excel writes and accepts a union in a name without parentheses.
+        let r = s.execute("formulas.defineName", json!({"name": "u", "refersTo": "=Sheet1!$A$1:$A$2,Sheet1!$C$1:$C$2"})).unwrap();
+        assert_eq!(r["refersTo"], "=Sheet1!$A$1:$A$2,Sheet1!$C$1:$C$2");
+        for (cell, input) in [("E1", "=SUM(u)"), ("E2", "=AREAS(u)"), ("E3", "=MIN(u)"), ("E4", "=INDEX(u,2,1,2)")] {
+            s.execute("cell.set", json!({"cell": cell, "input": input})).unwrap();
+        }
+        let v = |s: &mut Session, a: &str| s.execute("cell.get", json!({"cell": a})).unwrap()["value"].clone();
+        assert_eq!(v(&mut s, "E1"), json!(15.0));
+        assert_eq!(v(&mut s, "E2"), json!(2.0));
+        assert_eq!(v(&mut s, "E3"), json!(1.0));
+        assert_eq!(v(&mut s, "E4"), json!(7.0));
+        // Renaming the sheet follows every area, and the definition keeps Excel's form.
+        s.execute("sheet.rename", json!({"name": "Data"})).unwrap();
+        let name = s.doc().unwrap().wb.names.iter().find(|n| n.name == "u").map(|n| n.formula.clone());
+        assert_eq!(name.as_deref(), Some("Data!$A$1:$A$2,Data!$C$1:$C$2"));
+        assert_eq!(v(&mut s, "E1"), json!(15.0));
+    }
 }

crates/engine/src/cmd/sheet.rs

@@ -704,7 +704,7 @@ fn delete_sheet(s: &mut Session, p: &Json) -> Result<Json> {
                 *sc -= 1;
             }
             // Names referring to the sheet become #REF!.
-            if let Ok(e) = gridcraft_formula::parse(&n.formula) {
+            if let Ok(e) = gridcraft_formula::parse_name(&n.formula) {
                 let ne = gridcraft_formula::adjust::delete_sheet(e.clone(), &name, &order);
                 if ne != e {
                     n.formula = gridcraft_formula::print(&ne);
@@ -766,7 +766,7 @@ fn rename_sheet(s: &mut Session, p: &Json) -> Result<Json> {
             }
         }
         for n in cx.wb.names.iter_mut() {
-            if let Ok(e) = gridcraft_formula::parse(&n.formula) {
+            if let Ok(e) = gridcraft_formula::parse_name(&n.formula) {
                 n.formula = gridcraft_formula::print(&gridcraft_formula::adjust::rename_sheet(e, &old, &name));
             }
         }
@@ -805,7 +805,7 @@ fn move_sheet(s: &mut Session, p: &Json) -> Result<Json> {
                 .iter()
                 .filter(|nm| nm.scope == Some(i))
                 .map(|nm| {
-                    let formula = gridcraft_formula::parse(&nm.formula)
+                    let formula = gridcraft_formula::parse_name(&nm.formula)
                         .map(|e| gridcraft_formula::print(&gridcraft_formula::adjust::rename_sheet(e, &base, &sh.name)))
                         .unwrap_or_else(|_| nm.formula.clone());
                     gridcraft_model::DefinedName { scope: Some(at), formula, ..nm.clone() }

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::{ParseError, normalize_function_name, parse};
+pub use parser::{ParseError, normalize_function_name, parse, parse_name};
 pub use printer::{print, print_r1c1, quote_sheet};
 
 #[cfg(test)]

crates/formula/src/parser.rs

@@ -23,12 +23,22 @@ pub fn parse(src: &str) -> Result<Expr, ParseError> {
     parse_syntax(src, crate::FormulaLocale::En)
 }
 
+/// Parses a defined name's formula. Unlike a cell formula, a comma at the top level is the
+/// union operator: Excel stores and shows a name over several ranges as `Sheet1!$A$1,Sheet1!$C$1`.
+pub fn parse_name(src: &str) -> Result<Expr, ParseError> {
+    parse_top(src, crate::FormulaLocale::En, true)
+}
+
 pub(crate) fn parse_syntax(src: &str, locale: crate::FormulaLocale) -> Result<Expr, ParseError> {
+    parse_top(src, locale, false)
+}
+
+fn parse_top(src: &str, locale: crate::FormulaLocale, allow_union: bool) -> Result<Expr, ParseError> {
     let body = src.strip_prefix('=').unwrap_or(src);
     let toks = tokenize_locale(body, locale).map_err(|e| ParseError { msg: e.msg, pos: e.pos })?;
     let toks = significant_spaces(toks);
     let mut p = Parser { toks, i: 0, depth: 0, input: locale == crate::FormulaLocale::Es };
-    let e = p.expr(0, false)?;
+    let e = p.expr(0, allow_union)?;
     match p.peek() {
         Tok::Eof => Ok(e),
         t => Err(p.err(format!("unexpected {}", describe(t)))),

crates/model/src/workbook.rs

@@ -228,7 +228,7 @@ impl Workbook {
             }
         }
         for n in self.names.iter_mut() {
-            if let Ok(e) = gridcraft_formula::parse(&n.formula) {
+            if let Ok(e) = gridcraft_formula::parse_name(&n.formula) {
                 let ne = spell_calls(e.clone(), &lambdas);
                 if ne != e {
                     n.formula = gridcraft_formula::print(&ne);

crates/xlsx/src/array_formula.rs

@@ -57,7 +57,7 @@ impl Scan<'_> {
                     None => (sheet, n.as_str()),
                 };
                 if let Some(def) = self.wb.name(bare, scope) {
-                    return match gridcraft_formula::parse(def.formula.trim_start_matches('=')) {
+                    return match gridcraft_formula::parse_name(def.formula.trim_start_matches('=')) {
                         Ok(expr) => self.expr(&expr, def.scope.unwrap_or(scope), array_context, depth + 1),
                         Err(_) => Shape::array(),
                     };