Merge pull request #398 from nihilist-narwhal/fix-relative-names

Defined names: relative references follow the cell that uses the name

crates/calc/src/eval.rs

@@ -249,6 +249,13 @@ impl<'h> Evaluator<'h> {
             let scope = def.scope.unwrap_or(self.sheet);
             return match gridcraft_formula::parse(&text) {
                 Ok(expr) => {
+                    // Relative references in a name are relative to A1: used here, they move by
+                    // this cell's offset from it.
+                    let expr = if gridcraft_formula::adjust::has_relative_refs(&expr) {
+                        gridcraft_formula::adjust::shift_relative_wrapping(expr, self.at.row as i64, self.at.col as i64)
+                    } else {
+                        expr
+                    };
                     let saved = self.sheet;
                     // Unqualified references in a name refer to the scope sheet.
                     self.sheet = scope;

crates/calc/src/tests.rs

@@ -261,6 +261,32 @@ fn references_functions() {
     assert_eq!(t.num("B6"), 105.0);
 }
 
+#[test]
+fn relative_names() {
+    let mut t = T::new();
+    // As files store them: relative to A1. `Left` is the cell to the left, `Above3` the three
+    // cells above, `Mixed` column A of the same row.
+    for (name, formula) in [("Left", "Sheet1!XFD1"), ("Above3", "Sheet1!A1048574:A1048576"), ("Mixed", "Sheet1!$A1")] {
+        t.wb.names.push(gridcraft_model::DefinedName {
+            name: name.into(),
+            scope: None,
+            formula: formula.into(),
+            comment: String::new(),
+            hidden: false,
+        });
+    }
+    t.calc.rebuild(&t.wb);
+    for r in 1..=5 {
+        t.set(&format!("A{r}"), &(r * 10).to_string());
+    }
+    t.set("B2", "=Left");
+    assert_eq!(t.num("B2"), 20.0);
+    t.set("B4", "=Left+Mixed");
+    assert_eq!(t.num("B4"), 80.0);
+    t.set("A6", "=SUM(Above3)");
+    assert_eq!(t.num("A6"), 120.0);
+}
+
 #[test]
 fn spills() {
     let mut t = T::new();

crates/engine/src/cmd/formulas.rs

@@ -253,7 +253,15 @@ fn define_name(s: &mut Session, p: &Json) -> Result<Json> {
         Some(sh) => Some(d.wb.sheet_index(sh).ok_or_else(|| bad("formulas.defineName", "no such sheet"))?),
     };
     let refers = match str_param(p, "refersTo") {
-        Some(r) => r.trim_start_matches('=').to_string(),
+        // 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('=')) {
+            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)))
+            }
+            _ => r.trim_start_matches('=').to_string(),
+        },
         None => {
             let sh = d.wb.active().ok_or(EngineError::NoDocument)?;
             let r = d.selection.current();
@@ -564,3 +572,27 @@ fn calc_options(s: &mut Session, p: &Json) -> Result<Json> {
         Ok(json!({"mode": format!("{:?}", cx.wb.calc.mode)}))
     })
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn names_with_relative_references() {
+        let mut s = Session::new();
+        s.new_workbook();
+        s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2], [3]]})).unwrap();
+        // Defined from B2, `=Sheet1!A2` is "the cell to the left".
+        s.execute("selection.set", json!({"range": "B2"})).unwrap();
+        let r = s.execute("formulas.defineName", json!({"name": "LeftCell", "refersTo": "=Sheet1!A2"})).unwrap();
+        assert_eq!(r["refersTo"], "=Sheet1!XFD1");
+        s.execute("cell.set", json!({"cell": "B1", "input": "=LeftCell"})).unwrap();
+        s.execute("cell.set", json!({"cell": "B3", "input": "=LeftCell*10"})).unwrap();
+        let v = |s: &mut Session, a: &str| s.execute("cell.get", json!({"cell": a})).unwrap()["value"].clone();
+        assert_eq!(v(&mut s, "B1"), json!(1.0));
+        assert_eq!(v(&mut s, "B3"), json!(30.0));
+        // Absolute references are kept as they are.
+        let r = s.execute("formulas.defineName", json!({"name": "First", "refersTo": "=Sheet1!$A$1"})).unwrap();
+        assert_eq!(r["refersTo"], "=Sheet1!$A$1");
+    }
+}

crates/formula/src/adjust.rs

@@ -16,6 +16,35 @@ pub fn shift_relative(e: Expr, dr: i64, dc: i64) -> Expr {
     })
 }
 
+/// Defined names: relative parts move by `(dr, dc)`, wrapping around the sheet's edges. Files
+/// store a name's relative references as seen from A1 (a name for "the cell to the left" defined
+/// in B1 is `Sheet1!XFD1`), and a use in another cell moves them by that cell's offset from A1.
+pub fn shift_relative_wrapping(e: Expr, dr: i64, dc: i64) -> Expr {
+    let wrap = |v: u32, abs: bool, d: i64, max: u32| if abs { v } else { (v as i64 + d).rem_euclid(max as i64) as u32 };
+    let anchor = |a: Anchor| Anchor { row: wrap(a.row, a.row_abs, dr, MAX_ROWS), col: wrap(a.col, a.col_abs, dc, MAX_COLS), ..a };
+    e.map(&mut |x| match x {
+        Expr::Ref(r) => {
+            let kind = match r.kind {
+                RefKind::Cell(a) => RefKind::Cell(anchor(a)),
+                RefKind::Range(a, b) => RefKind::Range(anchor(a), anchor(b)),
+                RefKind::Rows(r0, a0, r1, a1) => RefKind::Rows(wrap(r0, a0, dr, MAX_ROWS), a0, wrap(r1, a1, dr, MAX_ROWS), a1),
+                RefKind::Cols(c0, a0, c1, a1) => RefKind::Cols(wrap(c0, a0, dc, MAX_COLS), a0, wrap(c1, a1, dc, MAX_COLS), a1),
+            };
+            Expr::Ref(Reference { sheet: r.sheet, kind })
+        }
+        other => other,
+    })
+}
+
+/// Whether the formula has a relative (no `$`) row or column anywhere.
+pub fn has_relative_refs(e: &Expr) -> bool {
+    e.references().iter().any(|r| match r.kind {
+        RefKind::Cell(a) => !a.row_abs || !a.col_abs,
+        RefKind::Range(a, b) => !a.row_abs || !a.col_abs || !b.row_abs || !b.col_abs,
+        RefKind::Rows(_, a0, _, a1) | RefKind::Cols(_, a0, _, a1) => !a0 || !a1,
+    })
+}
+
 fn shift_anchor(a: &Anchor, dr: i64, dc: i64) -> Option<Anchor> {
     let row = if a.row_abs { a.row as i64 } else { a.row as i64 + dr };
     let col = if a.col_abs { a.col as i64 } else { a.col as i64 + dc };