Merge pull request #157 from nihilist-narwhal/fix-blocked-spill-dependents

Recalc: formulas over a blocked spill see #SPILL!; no spilling in tables

crates/calc/src/recalc.rs

@@ -391,6 +391,27 @@ struct PassHost<'a> {
     depth: usize,
 }
 
+/// Whether an array result at `k` can't spill: it is in a table, or its area runs off the sheet,
+/// overlaps merged cells, holds other cells, or another formula's spill (unless that formula is
+/// recalculated in this pass, and so may spill elsewhere now).
+fn spill_blocked(host: &PassHost<'_>, (sheet, anchor): Key, a: &Array) -> bool {
+    let Some(sh) = host.wb.sheet(sheet) else { return false };
+    let end =
+        CellRef::new(anchor.row.saturating_add((a.rows as u32).saturating_sub(1)), anchor.col.saturating_add((a.cols as u32).saturating_sub(1)));
+    let range = RangeRef::new(anchor, end);
+    let own = sh.spill_ranges.get(&anchor);
+    let spilled_over = |c: CellRef| {
+        !own.is_some_and(|o| o.contains(c))
+            && sh.spill_ranges.iter().any(|(o, r)| *o != anchor && r.contains(c) && !host.exprs.contains_key(&(sheet, *o)))
+    };
+    end.row >= gridcraft_core::MAX_ROWS
+        || end.col >= gridcraft_core::MAX_COLS
+        || sh.merges.iter().any(|m| m.intersects(&range))
+        || sh.tables.iter().any(|t| t.range.contains(anchor))
+        || sh.cells.iter_range(range).any(|(c, cell)| c != anchor && (!cell.value.is_empty() || cell.formula.is_some()))
+        || sh.spill.range(range.start..=range.end).any(|(c, _)| range.contains(*c) && spilled_over(*c))
+}
+
 impl PassHost<'_> {
     fn compute(&mut self, k: Key) -> Value {
         if let Some(v) = self.results.get(&k) {
@@ -434,6 +455,14 @@ impl PassHost<'_> {
             Value::Empty => Value::Number(0.0),
             v => v,
         };
+        // An array that can't spill is #SPILL! for the formulas evaluated after it in this pass too.
+        let v = match self.spills.get(&k) {
+            Some(a) if spill_blocked(self, k, a) => {
+                self.spills.remove(&k);
+                Value::Error(CellError::Spill)
+            }
+            _ => v,
+        };
         self.results.insert(k, v.clone());
         v
     }
@@ -799,7 +828,8 @@ impl Calc {
                     let blocked = end.row >= gridcraft_core::MAX_ROWS
                         || end.col >= gridcraft_core::MAX_COLS
                         || range.iter().any(|c| c != k.1 && (sheet.cells.has(c) || sheet.spill.contains_key(&c)))
-                        || sheet.merges.iter().any(|m| m.intersects(&range));
+                        || sheet.merges.iter().any(|m| m.intersects(&range))
+                        || sheet.tables.iter().any(|t| t.range.contains(k.1));
                     if blocked {
                         if let Some(cell) = sheet.cells.get_mut(k.1) {
                             cell.value = Value::Error(CellError::Spill);

crates/calc/src/tests.rs

@@ -139,6 +139,72 @@ fn operators() {
     assert_eq!(t.num("A7"), 0.0);
 }
 
+#[test]
+fn blocked_spill_reads_as_spill_error() {
+    // Calculated in the same pass, a formula over a blocked spill read the array that couldn't
+    // spill.
+    let mut t = T::new();
+    let s = t.wb.sheet_mut(0).unwrap();
+    for a in ["A1", "B1", "C1", "A2", "C2", "A3", "B3", "C3"] {
+        s.cells.set(c(a), Cell::value(Value::Number(1.0)));
+    }
+    s.cells.set(c("B2"), Cell::formula(Formula::new("=SEQUENCE(2,2,9)")));
+    s.cells.set(c("E1"), Cell::formula(Formula::new("=SUM(A1:C3)")));
+    t.calc.recalc_all(&mut t.wb);
+    assert_eq!(t.get("B2"), Value::Error(CellError::Spill));
+    assert_eq!(t.get("E1"), Value::Error(CellError::Spill));
+    assert_eq!(t.num("C3"), 1.0);
+}
+
+#[test]
+fn blocked_spill_read_through_offset() {
+    // The reader's reference is only known while evaluating, so it isn't in the dependency graph.
+    let mut t = T::new();
+    t.set("E4", "5");
+    t.set("E8", "=SUM(OFFSET(E2,0,0,2,2))");
+    t.set("E3", "=SEQUENCE(2,2,3)");
+    t.calc.recalc_all(&mut t.wb);
+    assert_eq!(t.get("E3"), Value::Error(CellError::Spill));
+    assert_eq!(t.get("E8"), Value::Error(CellError::Spill));
+}
+
+#[test]
+fn spill_into_the_old_area_of_a_blocked_neighbour() {
+    // C7's array becomes blocked by C8, whose array then takes over C7's old area.
+    let mut t = T::new();
+    t.set("C7", "=SEQUENCE(2,2)");
+    t.set("C8", "=SEQUENCE(2,2)");
+    assert_eq!(t.get("C7"), Value::Error(CellError::Spill));
+    assert_eq!([t.num("C8"), t.num("D8"), t.num("C9"), t.num("D9")], [1.0, 2.0, 3.0, 4.0]);
+}
+
+#[test]
+fn arrays_do_not_spill_in_tables() {
+    // Excel: spilled array formulas aren't supported in tables.
+    let mut t = T::new();
+    t.wb.sheet_mut(0).unwrap().tables.push(gridcraft_model::Table {
+        id: 1,
+        name: "Table1".into(),
+        range: gridcraft_core::RangeRef::parse("B3:C6").unwrap(),
+        header_row: true,
+        totals_row: false,
+        columns: vec![],
+        style: String::new(),
+        banded_rows: false,
+        banded_cols: false,
+        first_col: false,
+        last_col: false,
+        filter_button: false,
+    });
+    t.set("C4", "=SEQUENCE(2)");
+    t.set("E1", "=SEQUENCE(4)");
+    t.set("H1", "=SUM(C4:C5)");
+    assert_eq!(t.get("C4"), Value::Error(CellError::Spill));
+    assert_eq!(t.get("C5"), Value::Empty);
+    assert_eq!(t.get("H1"), Value::Error(CellError::Spill));
+    assert_eq!(t.num("E4"), 4.0);
+}
+
 #[test]
 fn ifs_and_lazy() {
     let mut t = T::new();