Merge pull request #155 from nihilist-narwhal/fix-range-blank-cells

Ranges keep their size past the used range; spilled blanks show 0

crates/calc/src/eval.rs

@@ -13,6 +13,10 @@ use gridcraft_model::Workbook;
 
 /// Largest array a reference may expand to.
 pub const MAX_CELLS: u64 = 16_000_000;
+/// References up to this many cells are read at their full size; larger ones (`A:A`, ranges
+/// reserved for data to come such as `$A$2:$A$50000`) stop at the sheet's used range, so that
+/// formulas over them stay fast.
+const FULL_SIZE_CELLS: u64 = 16_384;
 const MAX_DEPTH: usize = 400;
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
@@ -138,7 +142,10 @@ impl<'h> Evaluator<'h> {
         if a.range.is_single() {
             return self.host.cell_value(a.sheet, a.range.start);
         }
-        let range = self.trim(a);
+        // A reference keeps its size, so the array has its shape and its blanks count
+        // (`=A1:A10`, `SUMPRODUCT(--(A1:A10=""))`).
+        let cells = u64::from(a.range.height()) * u64::from(a.range.width());
+        let range = if cells > FULL_SIZE_CELLS { self.trim(a) } else { a.range };
         let (h, w) = (range.height() as usize, range.width() as usize);
         if (h as u64) * (w as u64) > MAX_CELLS {
             return Value::Error(CellError::Num);

crates/calc/src/recalc.rs

@@ -414,10 +414,21 @@ impl PassHost<'_> {
         self.depth -= 1;
         self.in_progress.remove(&k);
         self.pending.remove(&k);
+        // A blank cell shows as 0 in a formula's result, inside an array as well (`=A1:A3`,
+        // FILTER or SORT of a range with blanks). GROUPBY and PIVOTBY lay out blank cells
+        // of their own, which stay blank.
+        let keep_blanks = matches!(&*expr, Expr::Call(n, _) if matches!(n.as_str(), "GROUPBY" | "PIVOTBY"));
         let v = match v {
-            Value::Array(a) if a.rows == 1 && a.cols == 1 => a.data.first().cloned().unwrap_or_default(),
             Value::Array(a) => {
-                self.spills.insert(k, a.clone());
+                let a = if keep_blanks || !a.data.iter().any(|x| matches!(x, Value::Empty)) {
+                    a
+                } else {
+                    let data = a.data.iter().map(|x| if matches!(x, Value::Empty) { Value::Number(0.0) } else { x.clone() }).collect();
+                    Array::new(a.rows, a.cols, data).map(std::sync::Arc::new).unwrap_or(a)
+                };
+                if a.rows > 1 || a.cols > 1 {
+                    self.spills.insert(k, a.clone());
+                }
                 a.data.first().cloned().unwrap_or_default()
             }
             Value::Empty => Value::Number(0.0),

crates/calc/src/tests.rs

@@ -192,6 +192,40 @@ fn spills() {
     assert_eq!(t.num("D2"), 40.0);
 }
 
+#[test]
+fn references_keep_their_size_past_the_used_range() {
+    let mut t = T::new();
+    t.set("A1", "5");
+    t.set("A2", "6");
+    t.set("C1", "=ROWS(A1:A10*1)");
+    t.set("C2", "=SUMPRODUCT(--(A1:A10=\"\"))");
+    t.set("C3", "=COUNTBLANK(A1:A10)");
+    t.set("C4", "=COUNTIF(A1:A10,\"\")");
+    t.set("C5", "=COLUMNS(TRANSPOSE(A1:A10))");
+    for (at, want) in [("C1", 10.0), ("C2", 8.0), ("C3", 8.0), ("C4", 8.0), ("C5", 10.0)] {
+        assert_eq!(t.num(at), want, "{at}");
+    }
+    // Whole columns are still read up to the used range.
+    t.set("D1", "=ROWS(FILTER(A:A,A:A<>\"\"))");
+    assert_eq!(t.num("D1"), 2.0);
+}
+
+#[test]
+fn spilled_blank_cells_show_zero() {
+    let mut t = T::new();
+    t.set("A1", "5");
+    t.set("A3", "7");
+    t.set("C1", "=A1:A4");
+    assert_eq!(t.get("C2"), Value::Number(0.0));
+    assert_eq!(t.get("C4"), Value::Number(0.0));
+    t.set("D1", "=COUNT(C1:C4)");
+    assert_eq!(t.num("D1"), 4.0);
+    t.set("E1", "=A2:A3");
+    assert_eq!(t.get("E1"), Value::Number(0.0));
+    t.set("F1", "=SORT(A1:A3)");
+    assert_eq!(t.get("F3"), Value::Number(0.0));
+}
+
 #[test]
 fn let_lambda() {
     let mut t = T::new();