diff --git a/crates/calc/src/recalc.rs b/crates/calc/src/recalc.rs index 160e0d60d..5a3f27cc0 100644 --- a/crates/calc/src/recalc.rs +++ b/crates/calc/src/recalc.rs @@ -128,9 +128,8 @@ impl Calc { for (si, sheet) in wb.sheets.iter().enumerate() { for (c, cell) in sheet.cells.iter() { if let Some(f) = &cell.formula - && let Some(e) = f.expr() + && let Some((areas, dynamic)) = f.with_expr(|e| precedents(wb, si, e)) { - let (areas, dynamic) = precedents(wb, si, &e); self.graph.insert((si, c), Node { areas, dynamic }); if cell.value == Value::Error(CellError::Circ) { circ.push((si, c)); @@ -209,14 +208,14 @@ impl Calc { pub fn cells_changed(&mut self, wb: &mut Workbook, changed: &[Key]) { let mut cleared: Vec<(usize, RangeRef)> = Vec::new(); for &k in changed { - let formula = wb.sheet(k.0).and_then(|s| s.cell(k.1)).and_then(|c| c.formula.clone()); + let precedent = + wb.sheet(k.0).and_then(|s| s.cell(k.1)).and_then(|c| c.formula.as_ref()).and_then(|f| f.with_expr(|e| precedents(wb, k.0, e))); let py = wb.sheet(k.0).and_then(|s| s.cell(k.1)).and_then(|c| c.py.as_ref()); let python = py.is_some(); // A Python cell whose result (if any) is not an array spills nothing. let spills = py.and_then(|p| p.result_value()).is_some_and(|v| matches!(v, Value::Array(_))); - match formula.and_then(|f| f.expr()) { - Some(e) => { - let (areas, dynamic) = precedents(wb, k.0, &e); + match precedent { + Some((areas, dynamic)) => { self.graph.insert(k, Node { areas, dynamic }); } // See `rebuild`: its spill is laid out again from its result when it is calculated. diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index d10c2f983..7e871dee0 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -2379,6 +2379,45 @@ fn loan_schedule_residue_keeps_excels_sign() { assert_eq!(t.get("P7"), Value::Text("".into())); } +/// `rebuild` and `cells_changed` read formulas through `Formula::with_expr`, which borrows the +/// cached tree or parses on the fly. The dependency graph must come out the same for formulas that +/// carry their tree and for formulas that only carry their text (as after loading a file). +#[test] +fn graph_built_from_text_only_formulas_matches_cached() { + fn run(cached: bool) -> Vec { + let mut t = T::new(); + t.set("A1", "2"); + for (at, f) in [("B1", "=A1*10"), ("C1", "=B1+A1"), ("D1", "=SUM(A1:C1)"), ("E1", "=NOPE(")] { + // E1 does not parse, so it has no tree either way and only covers the `None` branch. + let mut formula = Formula::new(f); + if !cached { + formula.expr = None; + } + t.wb.sheet_mut(0).unwrap().cells.set(c(at), Cell::formula(formula)); + } + t.calc.rebuild(&t.wb); + t.calc.recalc_all(&mut t.wb); + let mut out: Vec = ["B1", "C1", "D1", "E1"].iter().map(|a| t.get(a)).collect(); + // Editing the input must reach the dependents through the rebuilt graph. + t.set("A1", "5"); + out.extend(["B1", "C1", "D1"].iter().map(|a| t.get(a))); + // A text-only formula that is rewritten through `cells_changed` joins the graph too. + let mut formula = Formula::new("=A1+100"); + if !cached { + formula.expr = None; + } + t.wb.sheet_mut(0).unwrap().cells.set(c("F1"), Cell::formula(formula)); + t.calc.cells_changed(&mut t.wb, &[(0, c("F1"))]); + t.set("A1", "7"); + out.push(t.get("F1")); + out + } + let cached = run(true); + assert_eq!(cached, run(false)); + assert_eq!(cached[4], Value::Number(50.0)); + assert_eq!(cached[7], Value::Number(107.0)); +} + /// Makes `at` a Python cell showing `cached` (its value before this calculation), with `result` /// as its kept result, and returns the cell's key for `Calc::cells_changed`. fn plant_python(t: &mut T, at: &str, cached: Value, result: Value) -> (usize, CellRef) { diff --git a/crates/model/src/cell.rs b/crates/model/src/cell.rs index 58df4ebeb..0ca1dbe74 100644 --- a/crates/model/src/cell.rs +++ b/crates/model/src/cell.rs @@ -47,6 +47,16 @@ impl Formula { pub fn expr_arc(&self) -> Option> { self.expr.clone().or_else(|| gridcraft_formula::parse(&self.text).ok().map(Arc::new)) } + /// Runs `f` on the parsed expression, borrowing the cached tree or, after deserialization, + /// a freshly parsed one that is dropped afterwards. Unlike [`expr`](Self::expr) it never copies + /// the tree, and unlike [`expr_arc`](Self::expr_arc) it needs no reference count or `Arc` + /// allocation. `None` when the text does not parse. + pub fn with_expr(&self, f: impl FnOnce(&Expr) -> R) -> Option { + match &self.expr { + Some(e) => Some(f(e)), + None => gridcraft_formula::parse(&self.text).ok().map(|e| f(&e)), + } + } } /// A static picture stored as cell content, independent of floating drawing anchors.