diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 0f9cfb03d..525d2c300 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -9,7 +9,7 @@ use std::sync::Arc; use gridcraft_core::{Array, CellError, CellRef, MAX_COLS, MAX_ROWS, RangeRef, Value, compare}; use gridcraft_formula::{BinOp, Expr, RefKind, Reference, SheetSel, StructItem, StructRef, UnOp}; use gridcraft_functions::{Arg, Ctx}; -use gridcraft_model::Workbook; +use gridcraft_model::{ExternalBook, ExternalName, ExternalSheet, Workbook}; /// Largest array a reference may expand to. pub const MAX_CELLS: u64 = 16_000_000; @@ -18,6 +18,14 @@ pub const MAX_CELLS: u64 = 16_000_000; /// formulas over them stay fast. const FULL_SIZE_CELLS: u64 = 16_384; const MAX_DEPTH: usize = 400; +/// Areas in a linked workbook (`[2]Prices!A:A`) have sheet numbers from here up, so that they stay +/// references (`@`, INDEX, ROWS and the like see their position and size) while their cells are +/// read from the values saved with the link: see [`linked_sheet_number`]. 2^30 + 2^30 - 1 still +/// fits a 32-bit `usize` (wasm). +const LINKED_SHEETS: usize = 1 << 30; +/// Bits of a linked area's sheet number for the sheet within its workbook, and for the workbook. +const LINKED_SHEET_BITS: u32 = 16; +const LINKED_BOOK_BITS: u32 = 30 - LINKED_SHEET_BITS; #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub struct Area { @@ -58,6 +66,11 @@ pub trait Host { fn random(&mut self) -> f64; /// Called when a volatile or dynamic-reference function is used. fn note_volatile(&mut self) {} + /// Called when a formula uses a linked workbook's name whose definition can't be calculated + /// from the values saved with the link (no definition, `#REF!`, or sheets, names or tables + /// the link didn't save). Excel keeps showing the value it saved for such a formula, since + /// linked values only change when the link is updated; hosts that hold that value use it. + fn note_unknown_link(&mut self) {} /// The spill range of the dynamic array anchored at `anchor` (`None`: it doesn't spill). fn spill_range(&mut self, sheet: usize, anchor: CellRef) -> Option { self.workbook().sheet(sheet)?.spill_ranges.get(&anchor).copied() @@ -66,10 +79,10 @@ pub trait Host { fn range_values(&mut self, sheet: usize, range: RangeRef) -> Vec { range.iter().map(|c| self.cell_value(sheet, c)).collect() } - /// The sheet's used range (`Sheet::used_range`, which walks every stored row). Hosts that - /// keep the workbook unchanged while evaluating can remember it. + /// The sheet's used range ([`used_range_of`]). Hosts that keep the workbook unchanged while + /// evaluating can remember it. fn used_range(&mut self, sheet: usize) -> Option { - self.workbook().sheet(sheet).and_then(|s| s.used_range()) + used_range_of(self.workbook(), sheet) } /// Calls `f` on each stored value of a block, row-major (formula cells evaluated first), /// until `f` returns false. `false` when this host can't (callers use `range_values`). @@ -154,6 +167,107 @@ impl<'h> Evaluator<'h> { Ok(self.resolve_sheet(&r.sheet)?.into_iter().map(|sheet| Area { sheet, range }).collect()) } + /// A reference into a linked workbook (`'[2]Prices'!B2`, `[2]Prices!A:A`): an area that keeps + /// the position and size written, read from the values saved with the link, as Excel + /// calculates while the source is closed (see [`LINKED_SHEETS`]). `None` = not a reference + /// into a linked workbook; `#REF!` = the link didn't save that workbook or sheet, noted to the + /// host when the link has no saved workbook to look in (see [`Host::note_unknown_link`]). + fn linked_area(&mut self, r: &Reference) -> Option> { + let SheetSel::Named(name) = &r.sheet else { return None }; + let (book, sheet) = external_sheet(name)?; + let wb = self.host.workbook(); + let linked = book.checked_sub(1).and_then(|b| Some((b, wb.external_books.get(b)?))); + if linked.is_some_and(|(_, l)| l.opaque) { + self.host.note_unknown_link(); + return Some(Err(CellError::Ref)); + } + let found = linked.and_then(|(b, l)| Some((b, l.sheet_index(sheet)?))); + Some(found.and_then(|(b, s)| linked_sheet_number(b, s)).map(|sheet| Area { sheet, range: r.range() }).ok_or(CellError::Ref)) + } + + /// The saved sheet of a linked workbook that an area's sheet number stands for. + fn linked_sheet(&self, sheet: usize) -> Option<&ExternalSheet> { + let (book, s) = linked_parts(sheet)?; + self.host.workbook().external_books.get(book)?.sheets.get(s) + } + + /// The value of a cell, in this workbook or in a linked one's saved values. + fn cell(&mut self, sheet: usize, c: CellRef) -> Value { + if linked_parts(sheet).is_none() { + return self.host.cell_value(sheet, c); + } + match self.linked_sheet(sheet) { + Some(s) => linked_value(s, s.value(c)), + None => Value::Error(CellError::Ref), + } + } + + /// Values of a block, row-major, in this workbook or in a linked one's saved values. + pub(crate) fn block_values(&mut self, sheet: usize, range: RangeRef) -> Vec { + if linked_parts(sheet).is_none() { + return self.host.range_values(sheet, range); + } + let Some(s) = self.linked_sheet(sheet) else { return vec![Value::Error(CellError::Ref); range.count() as usize] }; + let w = range.width() as usize; + let mut out = vec![linked_value(s, Value::Empty); range.height() as usize * w]; + for (c, v) in s.cells_in(range) { + let i = (c.row - range.start.row) as usize * w + (c.col - range.start.col) as usize; + if let Some(slot) = out.get_mut(i) { + *slot = v.clone(); + } + } + out + } + + /// A defined name of a linked workbook (`[1]!Rates`, `[1]Prices!Rates`): what it refers to in + /// that workbook, read from the values saved with the link. A name the link doesn't define is + /// `#REF!`; one it defines but whose definition can't be read from the saved values is `#REF!` + /// noted to the host (see [`Host::note_unknown_link`]). Through a link that isn't to a + /// workbook (DDE, OLE) only a DDE item saved with its value is known (`[1]!Price`); anything + /// else is noted the same way. + fn external_name(&mut self, book: usize, sheet: &str, name: &str) -> Ev { + let wb = self.host.workbook(); + let Some(b) = book.checked_sub(1).and_then(|i| wb.external_books.get(i)) else { return err(CellError::Ref) }; + if b.opaque { + if sheet.is_empty() + && let Some(v) = b.item(name) + { + return Ev::V(v.clone()); + } + self.host.note_unknown_link(); + return err(CellError::Ref); + } + let Some(def) = b.name(name, (!sheet.is_empty()).then_some(sheet)) else { return err(CellError::Ref) }; + let scope = def.sheet.and_then(|i| b.sheets.get(i)).map(|s| format!("[{book}]{}", s.name)); + let parsed = gridcraft_formula::parse(&def.refers_to).ok().filter(|e| linked_definition_readable(b, def, e)); + let Some(expr) = parsed else { + self.host.note_unknown_link(); + return err(CellError::Ref); + }; + // Everything it names is in the linked workbook: its sheets and names (checked above). + let expr = expr.map(&mut |e| match e { + Expr::Ref(mut r) => { + r.sheet = match r.sheet { + SheetSel::Named(s) => SheetSel::Named(format!("[{book}]{s}")), + SheetSel::Current => match &scope { + Some(s) => SheetSel::Named(s.clone()), + None => return Expr::Error(CellError::Ref), + }, + SheetSel::Span(..) => return Expr::Error(CellError::Ref), + }; + Expr::Ref(r) + } + Expr::Name(n) => Expr::Name(match (n.split_once('!'), &scope) { + (Some(_), _) => format!("[{book}]{n}"), + (None, Some(s)) => format!("{s}!{n}"), + (None, None) => format!("[{book}]!{n}"), + }), + Expr::Struct(_) => Expr::Error(CellError::Ref), + e => e, + }); + self.eval(&expr) + } + /// Dereferences: one cell → its value; a block → an array (trimmed to the used range); /// several areas → `#VALUE!`. pub fn deref(&mut self, ev: Ev) -> Value { @@ -169,7 +283,7 @@ impl<'h> Evaluator<'h> { pub fn area_value(&mut self, a: &Area) -> Value { if a.range.is_single() { - return self.host.cell_value(a.sheet, a.range.start); + return self.cell(a.sheet, a.range.start); } // A reference keeps its size, so the array has its shape and its blanks count // (`=A1:A10`, `SUMPRODUCT(--(A1:A10=""))`). @@ -179,22 +293,26 @@ impl<'h> Evaluator<'h> { if (h as u64) * (w as u64) > MAX_CELLS { return Value::Error(CellError::Num); } - let data = self.host.range_values(a.sheet, range); + let data = self.block_values(a.sheet, range); Array::new(h, w, data).map(Value::from).unwrap_or(Value::Error(CellError::Value)) } /// Trims the bottom/right of an area to the sheet's used range (keeps the top-left so /// positions stay right). pub fn trim(&mut self, a: &Area) -> RangeRef { - if self.host.workbook().sheet(a.sheet).is_none() { + if linked_parts(a.sheet).is_none() && self.host.workbook().sheet(a.sheet).is_none() { return a.range; } + // Past the cells a refresh-error sheet saved, everything is `#REF!`, not blank: keep one + // row and column past them, so `[1]Data!A:A` reads an uncached cell and agrees with + // `[1]Data!A1:A2`. + let past = u32::from(self.linked_sheet(a.sheet).is_some_and(|s| s.refresh_error)); let used = self.host.used_range(a.sheet); let (mut r1, mut c1) = (a.range.end.row, a.range.end.col); match used { Some(u) => { - r1 = r1.min(u.end.row.max(a.range.start.row)); - c1 = c1.min(u.end.col.max(a.range.start.col)); + r1 = r1.min(u.end.row.saturating_add(past).max(a.range.start.row)); + c1 = c1.min(u.end.col.saturating_add(past).max(a.range.start.col)); } None => { r1 = a.range.start.row; @@ -221,6 +339,7 @@ impl<'h> Evaluator<'h> { Expr::Text(t) => Ev::V(Value::Text(t.clone())), Expr::Bool(b) => Ev::V(Value::Bool(*b)), Expr::Error(x) => err(*x), + Expr::LinkedRefError(_) => err(CellError::Ref), Expr::Missing => Ev::V(Value::Empty), Expr::Paren(x) => self.eval(x), Expr::Array(rows) => { @@ -237,7 +356,11 @@ impl<'h> Evaluator<'h> { } Expr::Ref(r) => match self.reference_areas(r) { Ok(a) => Ev::R(a), - Err(x) => err(x), + Err(x) => match self.linked_area(r) { + Some(Ok(a)) => Ev::R(vec![a]), + Some(Err(e)) => err(e), + None => err(x), + }, }, Expr::Name(n) => self.name(n), Expr::Struct(s) => self.structured(s), @@ -259,6 +382,9 @@ impl<'h> Evaluator<'h> { if let Some((_, v)) = self.env.iter().rev().find(|(k, _)| k.eq_ignore_ascii_case(n)) { return v.clone(); } + if let Some((book, sheet, bare)) = external_name(n) { + return self.external_name(book, sheet, bare); + } let wb = self.host.workbook(); // Sheet-qualified `Sheet1!Name`. let (scope_sheet, bare) = match n.split_once('!') { @@ -397,7 +523,7 @@ impl<'h> Evaluator<'h> { return match ev { Ev::R(areas) => match areas.as_slice() { [a] => match self.implicit_intersect(a) { - Some(c) => Ev::V(self.host.cell_value(a.sheet, c)), + Some(c) => Ev::V(self.cell(a.sheet, c)), None => err(CellError::Value), }, _ => err(CellError::Value), @@ -713,7 +839,12 @@ impl<'h> Evaluator<'h> { match self.eval(&args[0]) { Ev::R(a) => { if name == "SHEET" { - a.first().map_or(err(CellError::Ref), |a| Ev::V(Value::number(a.sheet as f64 + 1.0))) + // A sheet of a linked workbook has no number in this one. + match a.first() { + Some(a) if linked_parts(a.sheet).is_some() => err(CellError::NA), + Some(a) => Ev::V(Value::number(a.sheet as f64 + 1.0)), + None => err(CellError::Ref), + } } else { let mut s: Vec = a.iter().map(|x| x.sheet).collect(); s.dedup(); @@ -826,6 +957,17 @@ impl<'h> Evaluator<'h> { { criteria = Some(*area); } + // Range parameters of the *IF(S) family take only references, and not references into + // a linked workbook: Excel answers `#VALUE!` (or the cell's error) while the source + // workbook is closed. A written error (`#REF!` left by a deleted range, here or in a + // linked workbook: `[1]Data!#REF!`) is that error. + let linked = matches!(&ev, Ev::R(areas) if areas.iter().any(|x| linked_parts(x.sheet).is_some())); + if takes_reference(spec.name, i) && (linked || matches!(a, Expr::Error(_) | Expr::LinkedRefError(_))) { + return err(match self.deref(ev) { + Value::Error(e) => e, + _ => CellError::Value, + }); + } if resizes_sum_range(name, args.len()) { if i == 0 { criteria_shape = ev_shape(&ev); @@ -882,7 +1024,8 @@ impl<'h> Evaluator<'h> { /// Adds one area to `acc`; `Err` is the result the function ends with. fn aggregate_area(&mut self, kind: Agg, a: &Area, acc: &mut Acc) -> Result<(), Ev> { - if !a.range.is_single() { + // Linked workbooks' saved values are read through `area_value`. + if !a.range.is_single() && linked_parts(a.sheet).is_none() { let mut error = None; let walked = self.host.visit_range(a.sheet, a.range, &mut |v| match acc.add_cell(kind, v) { Ok(()) => true, @@ -1117,7 +1260,9 @@ impl<'h> Evaluator<'h> { return err(CellError::Value); } let base = match self.eval(&args[0]) { - Ev::R(a) if a.len() == 1 => a[0], + // OFFSET needs the source workbook open: a closed linked one hands it values, not a + // reference, which is `#VALUE!` as for the *IF(S) range parameters. + Ev::R(a) if a.len() == 1 && linked_parts(a[0].sheet).is_none() => a[0], Ev::V(Value::Error(e)) => return err(e), _ => return err(CellError::Value), }; @@ -1180,7 +1325,9 @@ impl<'h> Evaluator<'h> { match gridcraft_formula::parse(&text) { Ok(Expr::Ref(_)) if !a1_ok => err(CellError::Ref), Ok(e @ (Expr::Ref(_) | Expr::Name(_) | Expr::Struct(_))) => match self.eval(&e) { - r @ Ev::R(_) => r, + // INDIRECT reads another workbook only while it is open (Microsoft's INDIRECT + // documentation): `#REF!` for a linked one. + Ev::R(a) if a.iter().all(|x| linked_parts(x.sheet).is_none()) => Ev::R(a), _ => err(CellError::Ref), }, _ => err(CellError::Ref), @@ -1363,9 +1510,9 @@ impl<'h> Evaluator<'h> { "address" => Ev::V(Value::text(format!("${}${}", gridcraft_core::col_to_letters(c.col), c.row + 1))), "row" => Ev::V(Value::number(c.row as f64 + 1.0)), "col" => Ev::V(Value::number(c.col as f64 + 1.0)), - "contents" => Ev::V(self.host.cell_value(a.sheet, c)), + "contents" => Ev::V(self.cell(a.sheet, c)), "type" => { - let v = self.host.cell_value(a.sheet, c); + let v = self.cell(a.sheet, c); Ev::V(Value::text(match v { Value::Empty => "b", Value::Text(_) => "l", @@ -1443,7 +1590,7 @@ impl<'h> Evaluator<'h> { if hidden || nested { continue; } - let v = self.host.cell_value(a.sheet, c); + let v = self.cell(a.sheet, c); if skip_errors && v.is_error() { continue; } @@ -1731,6 +1878,89 @@ pub fn resized_sum_range(sum: RangeRef, height: u32, width: u32) -> RangeRef { RangeRef::new(sum.start, CellRef::new(row, col)) } +/// `[2]!Rates` → linked workbook 2, no sheet, `Rates`; `[2]Prices!Rates` → sheet `Prices`. +fn external_name(name: &str) -> Option<(usize, &str, &str)> { + let (book, rest) = name.strip_prefix('[')?.split_once(']')?; + let (sheet, bare) = rest.split_once('!')?; + if book.is_empty() || !book.bytes().all(|b| b.is_ascii_digit()) || bare.is_empty() { + return None; + } + Some((book.parse().ok()?, sheet, bare)) +} + +/// Whether argument `i` of `name` is a range parameter that takes only a reference. +fn takes_reference(name: &str, i: usize) -> bool { + match name { + "SUMIF" | "AVERAGEIF" => i == 0 || i == 2, + "COUNTIF" | "COUNTBLANK" => i == 0, + "COUNTIFS" => i.is_multiple_of(2), + "SUMIFS" | "AVERAGEIFS" | "MAXIFS" | "MINIFS" => i == 0 || i % 2 == 1, + _ => false, + } +} + +/// `[2]Prices` (the sheet part of `'[2]Prices'!B2`) → linked workbook 2, sheet `Prices`. +fn external_sheet(name: &str) -> Option<(usize, &str)> { + let (book, sheet) = name.strip_prefix('[')?.split_once(']')?; + if book.is_empty() || !book.bytes().all(|b| b.is_ascii_digit()) || sheet.is_empty() { + return None; + } + Some((book.parse().ok()?, sheet)) +} + +/// The sheet number of an [`Area`] in linked workbook `book` (index into +/// `Workbook::external_books`), sheet `sheet` (index into its `sheets`); `None` past the limits. +fn linked_sheet_number(book: usize, sheet: usize) -> Option { + (book < 1 << LINKED_BOOK_BITS && sheet < 1 << LINKED_SHEET_BITS).then_some(LINKED_SHEETS + (book << LINKED_SHEET_BITS) + sheet) +} + +/// `(book, sheet)` of an area's sheet number in a linked workbook (see [`linked_sheet_number`]); +/// `None` for a sheet of this workbook. +fn linked_parts(sheet: usize) -> Option<(usize, usize)> { + let k = sheet.checked_sub(LINKED_SHEETS)?; + Some((k >> LINKED_SHEET_BITS, k & ((1 << LINKED_SHEET_BITS) - 1))) +} + +/// The used range of an area's sheet (`Sheet::used_range`, which walks every stored row); for a +/// sheet of a linked workbook, the block its saved cells cover. +pub(crate) fn used_range_of(wb: &Workbook, sheet: usize) -> Option { + match linked_parts(sheet) { + Some((book, s)) => wb.external_books.get(book)?.sheets.get(s)?.extent(), + None => wb.sheet(sheet)?.used_range(), + } +} + +/// Whether a linked workbook's name `def` (parsed as `expr`) can be calculated from the values +/// saved with the link: no `#REF!` in it, and only the link's saved sheets and names. A sheet +/// range, a sheetless reference in a workbook-level name and a table can't be read either. +fn linked_definition_readable(b: &ExternalBook, def: &ExternalName, expr: &Expr) -> bool { + let scope = def.sheet.and_then(|i| b.sheets.get(i)).map(|s| s.name.as_str()); + let mut readable = true; + expr.walk(&mut |e| { + readable &= match e { + Expr::Error(CellError::Ref) | Expr::Struct(_) | Expr::LinkedRefError(_) => false, + Expr::Ref(r) => match &r.sheet { + SheetSel::Named(s) => b.sheet_index(s).is_some(), + SheetSel::Current => scope.is_some(), + SheetSel::Span(..) => false, + }, + Expr::Name(n) => match n.split_once('!') { + Some((s, bare)) => b.name(bare, Some(s.trim_matches('\''))).is_some(), + None => b.name(n, scope).is_some(), + }, + _ => true, + }; + }); + readable +} + +/// A value read from a linked sheet's saved cells. When the last update of the link failed for +/// the sheet (`refreshError`), Excel keeps the values it had and shows `#REF!` for anything it +/// hadn't saved. +fn linked_value(s: &ExternalSheet, v: Value) -> Value { + if v.is_empty() && s.refresh_error { Value::Error(CellError::Ref) } else { v } +} + /// Collects the static references of an expression with their sheets resolved; `dynamic` is /// set when the formula uses INDIRECT/OFFSET/names/tables whose targets can change. pub fn precedents(wb: &Workbook, sheet: usize, e: &Expr) -> (Vec, bool) { @@ -1760,7 +1990,10 @@ pub fn precedents(wb: &Workbook, sheet: usize, e: &Expr) -> (Vec, bool) { let (crit, sum) = (c.range(), s.range()); push(&mut out, s, resized_sum_range(sum, crit.height(), crit.width())); } - (_, Some(Expr::Number(_) | Expr::Text(_) | Expr::Bool(_) | Expr::Error(_) | Expr::Array(_) | Expr::Missing)) => {} + ( + _, + Some(Expr::Number(_) | Expr::Text(_) | Expr::Bool(_) | Expr::Error(_) | Expr::LinkedRefError(_) | Expr::Array(_) | Expr::Missing), + ) => {} _ => dynamic = true, }, Expr::Call(n, _) @@ -1789,6 +2022,92 @@ pub fn precedents(wb: &Workbook, sheet: usize, e: &Expr) -> (Vec, bool) { (out, dynamic) } +/// What a formula can read in this workbook besides its static references ([`precedents`]): +/// the targets of the local names it uses, through other names too, and the tables it refers +/// to. `names` lists those names. `None` when that isn't known without evaluating it: +/// INDIRECT, OFFSET, CELL, INFO, a SUMIF whose criteria range isn't a reference, or a name +/// with relative references. +pub struct DynamicReads { + pub areas: Vec, + pub names: Vec, +} + +pub fn dynamic_reads(wb: &Workbook, sheet: usize, e: &Expr) -> Option { + let mut out = DynamicReads { areas: Vec::new(), names: Vec::new() }; + collect_dynamic_reads(wb, sheet, e, &mut out, 0).then_some(out) +} + +fn collect_dynamic_reads(wb: &Workbook, sheet: usize, e: &Expr, out: &mut DynamicReads, depth: usize) -> bool { + if depth > 32 { + return false; + } + let mut known = true; + let tables = |out: &mut DynamicReads, si: usize, ti: usize| { + if let Some(t) = wb.sheet(si).and_then(|s| s.tables.get(ti)) { + if !out.names.iter().any(|n| n.eq_ignore_ascii_case(&t.name)) { + out.names.push(t.name.clone()); + } + out.areas.push(Area { sheet: si, range: t.range }); + } + }; + let name = |out: &mut DynamicReads, n: &str| -> bool { + let (scope_sheet, bare) = match n.split_once('!') { + Some((s, b)) => match wb.sheet_index(s.trim_matches('\'')) { + Some(i) => (i, b), + None => return true, + }, + None => (sheet, n), + }; + if out.names.iter().any(|x| x.eq_ignore_ascii_case(bare)) { + return true; + } + out.names.push(bare.to_string()); + if let Some(def) = wb.name(bare, scope_sheet) { + let scope = def.scope.unwrap_or(sheet); + return match gridcraft_formula::parse(&def.formula) { + Ok(expr) if !gridcraft_formula::adjust::has_relative_refs(&expr) => { + out.areas.extend(precedents(wb, scope, &expr).0); + collect_dynamic_reads(wb, scope, &expr, out, depth + 1) + } + Ok(_) => false, + Err(_) => true, + }; + } + if let Some((si, ti)) = wb.table(bare) { + tables(out, si, ti); + } + // Otherwise a LET or LAMBDA variable, or a name nothing defines. + true + }; + e.walk(&mut |x| match x { + Expr::Name(n) if external_name(n).is_none() => known &= name(out, n), + Expr::Struct(s) if s.table.is_empty() => { + for ti in 0..wb.sheet(sheet).map_or(0, |s| s.tables.len()) { + tables(out, sheet, ti); + } + } + Expr::Struct(s) => { + if let Some((si, ti)) = wb.table(&s.table) { + tables(out, si, ti); + } + } + Expr::Call(n, _) + if matches!(n.as_str(), "INDIRECT" | "OFFSET" | "CELL" | "INFO" | "NOW" | "TODAY" | "RAND" | "RANDBETWEEN" | "RANDARRAY") => + { + known = false + } + Expr::Call(n, args) if resizes_sum_range(n, args.len()) => { + if !matches!(args.first(), Some(Expr::Ref(_))) && matches!(args.get(2), Some(Expr::Ref(_))) { + known = false; + } + } + // A function defined by a name (a LAMBDA). + Expr::Call(n, _) if gridcraft_functions::lookup(n).is_none() && !is_special(n) => known &= name(out, n), + _ => {} + }); + known +} + fn is_special(n: &str) -> bool { matches!( n, diff --git a/crates/calc/src/recalc.rs b/crates/calc/src/recalc.rs index b391821d7..6bb849174 100644 --- a/crates/calc/src/recalc.rs +++ b/crates/calc/src/recalc.rs @@ -408,6 +408,14 @@ struct PassHost<'a> { tainted: HashSet, /// Whether the formula being evaluated has read a previous value. taint: bool, + /// Whether the formula being evaluated used something in a linked workbook that can't be + /// calculated from the link's saved values ([`Host::note_unknown_link`]). + unknown_link: bool, + /// After an edit, the formulas that read a changed cell, directly or through other formulas + /// (see [`Calc::readers`]). A formula over something uncalculable in a linked workbook keeps + /// the value it showed unless it is one of them; then it is calculated, the uncalculable part + /// being `#REF!`. `None` (loading, F9): every such formula keeps its value. + reached: Option<&'a HashSet>, } impl<'a> PassHost<'a> { @@ -431,6 +439,8 @@ impl<'a> PassHost<'a> { members: HashSet::default(), tainted: HashSet::default(), taint: false, + unknown_link: false, + reached: None, } } } @@ -483,10 +493,15 @@ impl PassHost<'_> { self.stack.push(k); } let expr = std::sync::Arc::clone(expr); + let outer_unknown = std::mem::replace(&mut self.unknown_link, false); let v = { let mut ev = Evaluator::new(self, k.0, k.1); ev.value(&expr) }; + // Excel keeps showing what it saved for a formula over a linked value it can't calculate, + // until the formula has to be calculated again. + let unknown = std::mem::replace(&mut self.unknown_link, outer_unknown); + let v = if unknown && !self.reached.is_some_and(|r| r.contains(&k)) { self.saved_value(k).unwrap_or(v) } else { v }; // A legacy array formula fills the range it was entered in. let legacy = self.wb.sheet(k.0).and_then(|s| s.cell(k.1)).and_then(|c| c.formula.as_ref()).and_then(|f| f.array); let v = match legacy { @@ -537,6 +552,23 @@ impl PassHost<'_> { v } + /// The value a formula showed before this pass, if any: what it was saved with, or last + /// calculated to. A spilled array, or a legacy array formula's range, comes back whole. A + /// blank formula cell has none (a typed formula, or one a structural edit made read other + /// cells): formulas show 0 for blank. + fn saved_value(&self, (sheet, at): Key) -> Option { + let sh = self.wb.sheet(sheet)?; + if sh.value(at).is_empty() { + return None; + } + let block = sh.spill_ranges.get(&at).copied().or_else(|| sh.cell(at)?.formula.as_ref()?.array); + if let Some(range) = block.filter(|r| !r.is_single()) { + let data = range.iter().map(|c| sh.value(c)).collect(); + return Array::new(range.height() as usize, range.width() as usize, data).map(|a| Value::Array(std::sync::Arc::new(a))); + } + Some(sh.value(at)) + } + /// A formula's previous value for iterative calculation: from the last iteration, else the /// value it shows. Blank, and the `#CIRC!` of a loop calculated with iteration off, are 0. fn previous(&self, k: Key) -> Value { @@ -628,6 +660,9 @@ impl Host for PassHost<'_> { fn workbook(&self) -> &Workbook { self.wb } + fn note_unknown_link(&mut self) { + self.unknown_link = true; + } fn spill_range(&mut self, sheet: usize, anchor: CellRef) -> Option { let k = (sheet, anchor); if !(self.pending.contains(&k) || self.results.contains_key(&k)) { @@ -765,7 +800,7 @@ impl Host for PassHost<'_> { } fn used_range(&mut self, sheet: usize) -> Option { let wb = self.wb; - *self.used.entry(sheet).or_insert_with(|| wb.sheet(sheet).and_then(|s| s.used_range())) + *self.used.entry(sheet).or_insert_with(|| crate::eval::used_range_of(wb, sheet)) } fn now_serial(&self) -> f64 { self.now @@ -811,7 +846,23 @@ impl Calc { /// Recalculates every formula (F9 / Ctrl+Alt+F9, and after loading). pub fn recalc_all(&mut self, wb: &mut Workbook) { + self.recalc_every_formula(wb, None); + } + + /// Recalculates every formula after a structural edit (rows, columns, sheets, sorting, names, + /// row heights…): as [`Calc::recalc_all`], except that a formula over something uncalculable + /// in a linked workbook is calculated, rather than keeping its value, when it reads one of the + /// `changed` cells (where they are after the edit) or uses one of the `changed_names`, + /// directly or through other formulas, names and tables, or uses INDIRECT or OFFSET (see + /// [`Calc::kept_value_readers`]). Edits that change nothing it reads keep its value. + pub fn recalc_all_after_edit(&mut self, wb: &mut Workbook, changed: &[Key], changed_names: &[String]) { + self.recalc_every_formula(wb, Some((changed, changed_names))); + } + + /// `changed`: the cells and names a structural edit changed, `None` for F9 and loading. + fn recalc_every_formula(&mut self, wb: &mut Workbook, changed: Option<(&[Key], &[String])>) { self.rebuild(wb); + let reached = changed.map(|(c, names)| self.kept_value_readers(wb, c, names)); // Spills are laid out again from the formulas where they are now (a structural edit moves // or deletes the formulas, not the values they spilled). for i in 0..wb.sheets.len() { @@ -823,7 +874,7 @@ impl Calc { } } let all: Vec = self.graph.nodes.keys().copied().collect(); - self.run(wb, all, true); + self.run(wb, all, reached.as_ref()); } /// Updates the graph for cells whose content changed and recalculates what depends on them. @@ -860,7 +911,7 @@ impl Calc { if wb.calc.mode == CalcMode::Manual { // Only the edited formulas themselves are evaluated. let own: Vec = changed.iter().copied().filter(|k| self.graph.nodes.contains_key(k)).collect(); - self.run(wb, own, false); + self.run(wb, own, None); return; } let mut seeds: Vec = changed.to_vec(); @@ -905,30 +956,114 @@ impl Calc { seeds.extend(r.iter().map(|c| (*si, c))); } let t0 = prof_now(); - let dirty = self.dirty_closure(wb, &seeds); + let reached = self.kept_value_readers(wb, &seeds, &[]); + let dirty = self.dirty_closure_from(&reached, &seeds); let t1 = prof_now(); let n = dirty.len(); - self.run(wb, dirty, false); + self.run(wb, dirty, Some(&reached)); if std::env::var_os("GRIDCRAFT_PROFILE").is_some() { eprintln!("recalc: {} seeds, closure {} cells {:.1} ms, run {:.1} ms", seeds.len(), n, t1 - t0, prof_now() - t1); } } fn dirty_closure(&self, _wb: &Workbook, seeds: &[Key]) -> Vec { + self.dirty_closure_from(&self.readers(seeds), seeds) + } + + /// The formulas that read one of `seeds`, directly or through other formulas (a seed itself + /// only when it reads another). + fn readers(&self, seeds: &[Key]) -> HashSet { let mut seen: HashSet = HashSet::default(); let mut queue: VecDeque = VecDeque::new(); + let mut deps = Vec::new(); for k in seeds { - if self.graph.nodes.contains_key(k) && seen.insert(*k) { - queue.push_back(*k); + deps.clear(); + self.graph.dependents(k.0, k.1, &mut deps); + for d in deps.drain(..) { + if seen.insert(d) { + queue.push_back(d); + } } - let mut deps = Vec::new(); + } + while let Some(k) = queue.pop_front() { + deps.clear(); self.graph.dependents(k.0, k.1, &mut deps); - for d in deps { + for d in deps.drain(..) { if seen.insert(d) { queue.push_back(d); } } } + seen + } + + /// The formulas whose saved value gives way to a calculation after an edit that changed + /// `seeds` and the names `names` (see `PassHost::reached`): the [`Calc::readers`] of the + /// seeds, and formulas that read one of them, or a changed name, through a local name or a + /// table (see [`crate::eval::dynamic_reads`]), with their own readers. A formula over + /// INDIRECT, OFFSET, CELL or INFO is calculated after any edit, a structural one that changed + /// no cell (an inserted row) too, as Excel recalculates those functions each time. + fn kept_value_readers(&self, wb: &Workbook, seeds: &[Key], names: &[String]) -> HashSet { + let mut seen = self.readers(seeds); + // Only formulas over a linked workbook keep a saved value. + if wb.external_books.is_empty() { + return seen; + } + let mut pending: Vec<(Key, Option)> = self + .graph + .dynamic + .iter() + .filter(|k| !seen.contains(*k)) + .filter_map(|&k| { + let expr = wb.sheet(k.0)?.cell(k.1)?.formula.as_ref()?.expr_arc()?; + Some((k, crate::eval::dynamic_reads(wb, k.0, &expr))) + }) + .collect(); + // Changed cells, and formulas that will be calculated, by (sheet, column, row). + let mut marks: BTreeSet<(usize, u32, u32)> = seeds.iter().chain(seen.iter()).map(|(s, c)| (*s, c.col, c.row)).collect(); + let hit = |marks: &BTreeSet<(usize, u32, u32)>, a: &Area| { + let (r, s) = (a.range, a.sheet); + if (r.width() as usize) <= 64 { + (r.start.col..=r.end.col).any(|col| marks.range((s, col, r.start.row)..=(s, col, r.end.row)).next().is_some()) + } else { + marks.range((s, r.start.col, 0)..=(s, r.end.col, u32::MAX)).any(|(_, col, row)| r.contains(CellRef::new(*row, *col))) + } + }; + loop { + let mut found = Vec::new(); + pending.retain(|(k, reads)| { + let reads_changed = match reads { + None => true, + Some(r) => r.names.iter().any(|n| names.iter().any(|c| c.eq_ignore_ascii_case(n))) || r.areas.iter().any(|a| hit(&marks, a)), + }; + if reads_changed { + found.push(*k); + } + !reads_changed + }); + if found.is_empty() { + return seen; + } + let more = self.readers(&found); + for k in found.into_iter().chain(more) { + if seen.insert(k) { + marks.insert((k.0, k.1.col, k.1.row)); + } + } + pending.retain(|(k, _)| !seen.contains(k)); + } + } + + /// What a change to `seeds` recalculates: the seeds' own formulas, their `readers`, and every + /// dynamic formula with what reads it. + fn dirty_closure_from(&self, readers: &HashSet, seeds: &[Key]) -> Vec { + let mut seen = readers.clone(); + let mut queue: VecDeque = VecDeque::new(); + for k in seeds { + if self.graph.nodes.contains_key(k) && seen.insert(*k) { + queue.push_back(*k); + } + } for k in &self.graph.dynamic { if seen.insert(*k) { queue.push_back(*k); @@ -947,8 +1082,9 @@ impl Calc { seen.into_iter().collect() } - /// Evaluates the dirty set and writes results (and spills) back. - fn run(&mut self, wb: &mut Workbook, mut dirty: Vec, full: bool) { + /// Evaluates the dirty set and writes results (and spills) back. `reached`: after an edit, the + /// formulas that read a changed cell (see `PassHost::reached`). + fn run(&mut self, wb: &mut Workbook, mut dirty: Vec, reached: Option<&HashSet>) { let mut dynamic_again = !self.graph.dynamic.is_empty(); for _round in 0..8 { if dirty.is_empty() { @@ -968,6 +1104,7 @@ impl Calc { let (results, spills, cycles) = { let mut host = PassHost::new(wb, &exprs, &mut rng, now); host.iterate = wb.calc.iterative; + host.reached = reached; for k in &dirty { host.compute(*k); } @@ -1062,9 +1199,6 @@ impl Calc { } } } - if full { - // Everything was already evaluated once; spill dependents need another pass. - } if std::env::var_os("GRIDCRAFT_PROFILE").is_some() { eprintln!(" write-back {:.1} ms", prof_now() - tw); } diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 196c57de7..43d6f68f5 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -94,6 +94,292 @@ fn chains_out_of_order() { assert_eq!(t.num("A5"), 5.0); } +#[test] +fn references_into_linked_workbooks_read_the_saved_values() { + use gridcraft_model::{ExternalBook, ExternalName, ExternalSheet}; + let mut t = T::new(); + let prices = ExternalSheet { + name: "P&L Prices".into(), + cells: vec![(c("A1"), Value::text("gas")), (c("B1"), Value::Number(4.5)), (c("A2"), Value::text("power")), (c("B2"), Value::Number(61.0))], + refresh_error: false, + }; + let rates = ExternalSheet { name: "Rates".into(), cells: vec![(c("C3"), Value::Number(2.0))], refresh_error: false }; + let failed = ExternalSheet { name: "Failed".into(), cells: vec![(c("A1"), Value::Number(9.0))], refresh_error: true }; + let names = vec![ + ExternalName { name: "Gas".into(), sheet: None, refers_to: "'P&L Prices'!$B$1".into() }, + ExternalName { name: "Rate".into(), sheet: Some(1), refers_to: "$C$3".into() }, + // Names inside a linked name are the linked workbook's too. + ExternalName { name: "GasTwice".into(), sheet: None, refers_to: "Gas*2".into() }, + ExternalName { name: "RatePlus".into(), sheet: Some(1), refers_to: "Rate+GasTwice".into() }, + // A sheet range or a sheetless reference in a workbook-level name can't be resolved + // from the saved values (and must not read this workbook's cells). + ExternalName { name: "Span".into(), sheet: None, refers_to: "Rates:Failed!$A$1".into() }, + ExternalName { name: "Loose".into(), sheet: None, refers_to: "$E$9".into() }, + ]; + t.wb.external_books = vec![ExternalBook::default(), ExternalBook { sheets: vec![prices, rates, failed], names, ..Default::default() }].into(); + t.wb.names.push(gridcraft_model::DefinedName { + name: "Price".into(), + scope: None, + formula: "'[2]P&L Prices'!$A$1:$B$65536".into(), + comment: String::new(), + hidden: false, + }); + t.set("A1", "='[2]P&L Prices'!B2*2"); + t.set("A2", "=VLOOKUP(\"gas\",Price,2,FALSE)"); + t.set("A3", "=SUM('[2]P&L Prices'!B1:B9)+[2]Rates!C3"); + t.set("A4", "=[2]Rates!A1"); + assert_eq!((t.num("A1"), t.num("A2"), t.num("A3"), t.num("A4")), (122.0, 4.5, 67.5, 0.0)); + // A sheet the link didn't save, or a book that isn't linked, is #REF! as before. + t.set("A5", "=[2]Other!A1"); + t.set("A6", "=[1]Rates!C3"); + t.set("A7", "='[3]Rates'!C3"); + for at in ["A5", "A6", "A7"] { + assert_eq!(t.get(at), Value::Error(CellError::Ref), "{at}"); + } + // A sheet whose last refresh failed keeps its saved cells; anything else in it is #REF!. + t.set("A8", "=[2]Failed!A1+[2]Failed!B1"); + t.set("A9", "=[2]Failed!A1"); + t.set("A10", "=SUM([2]Failed!A1:B1)"); + assert_eq!((t.get("A8"), t.num("A9"), t.get("A10")), (Value::Error(CellError::Ref), 9.0, Value::Error(CellError::Ref))); + // A reference Excel found deleted in a linked workbook is #REF!. + t.set("A14", "=[2]Rates!#REF!+1"); + assert_eq!(t.get("A14"), Value::Error(CellError::Ref)); + // A whole column, or more columns than a block can hold, reaches the same uncached cells. + t.set("A11", "=SUM([2]Failed!A:A)"); + t.set("A12", "=SUM([2]Failed!A1:A2)"); + t.set("A13", "=SUM([2]Failed!A:Z)"); + for at in ["A11", "A12", "A13"] { + assert_eq!(t.get(at), Value::Error(CellError::Ref), "{at}"); + } + // The linked workbook's own names: workbook-scoped and sheet-scoped. + t.set("B1", "=[2]!Gas*10"); + t.set("B2", "=[2]Rates!Rate+1"); + assert_eq!((t.num("B1"), t.num("B2")), (45.0, 3.0)); + t.set("B3", "=[2]!Missing"); + assert_eq!(t.get("B3"), Value::Error(CellError::Ref)); + t.set("E9", "100"); + t.set("B4", "=[2]!GasTwice"); + t.set("B5", "=[2]Rates!RatePlus"); + t.set("B6", "=[2]!Span"); + t.set("B7", "=[2]!Loose"); + assert_eq!((t.num("B4"), t.num("B5")), (9.0, 11.0)); + assert_eq!((t.get("B6"), t.get("B7")), (Value::Error(CellError::Ref), Value::Error(CellError::Ref))); + // SUMIF and friends need references; Excel can't read a closed workbook through them. + t.set("C1", "=SUMIF('[2]P&L Prices'!A1:A2,\"gas\",'[2]P&L Prices'!B1:B2)"); + t.set("C2", "=COUNTIF(#REF!,\">0\")"); + t.set("C3", "=SUMIF(Price,\"gas\")"); + assert_eq!( + (t.get("C1"), t.get("C2"), t.get("C3")), + (Value::Error(CellError::Value), Value::Error(CellError::Ref), Value::Error(CellError::Value)) + ); + // Linked sheets print as Excel writes them: quoted only when the sheet name needs it. + let f = t.wb.sheet(0).unwrap().cells.get(c("A3")).and_then(|x| x.formula.clone()).unwrap(); + assert_eq!(gridcraft_formula::print(f.expr.as_ref().unwrap()), "SUM('[2]P&L Prices'!B1:B9)+[2]Rates!C3"); +} + +#[test] +fn linked_names_that_cannot_be_calculated_keep_their_saved_value() { + use gridcraft_model::{ExternalBook, ExternalName, ExternalSheet}; + let mut t = T::new(); + let data = ExternalSheet { name: "Data".into(), cells: vec![(c("A1"), Value::Number(7.0))], refresh_error: false }; + let failed = ExternalSheet { name: "Failed".into(), cells: vec![], refresh_error: true }; + let name = |name: &str, refers_to: &str| ExternalName { name: name.into(), sheet: None, refers_to: refers_to.into() }; + let names = vec![ + name("Broken", "#REF!"), + // Saved without a definition. + name("Blank", ""), + name("Span", "Data:Failed!$A$1"), + name("Loose", "$E$9"), + name("Elsewhere", "Gone!$A$1"), + name("Inner", "Undefined*2"), + name("Good", "Data!$A$1"), + ]; + t.wb.external_books = vec![ExternalBook { sheets: vec![data, failed], names, ..Default::default() }].into(); + // Cells as read from a file: the formula and the value Excel saved for it. + let saved = [ + ("B1", "=[1]!Broken", Value::Number(20.0)), + ("B2", "=[1]!Blank", Value::text("KW")), + ("B3", "=[1]!Span", Value::Number(3.0)), + ("B4", "=[1]!Loose", Value::Number(4.0)), + ("B5", "=[1]!Elsewhere", Value::Number(5.0)), + ("B6", "=[1]!Inner+1", Value::Number(6.0)), + ("B7", "=[1]!Good*2", Value::Number(99.0)), + ("B8", "=[1]!Undefined", Value::Number(8.0)), + ("B9", "=[1]Failed!B1", Value::Number(9.0)), + ("B10", "=B1+1", Value::Number(0.0)), + ("B11", "=[1]!Broken", Value::Empty), + ]; + for (at, formula, value) in saved { + let mut cell = Cell::formula(Formula::new(formula)); + cell.value = value; + t.wb.sheet_mut(0).unwrap().cells.set(c(at), cell); + } + t.calc.recalc_all(&mut t.wb); + let expect = |t: &T| { + // Linked values only change when the link is updated, so Excel keeps showing these. + assert_eq!(t.num("B1"), 20.0); + assert_eq!(t.get("B2"), Value::text("KW")); + assert_eq!([t.num("B3"), t.num("B4"), t.num("B5"), t.num("B6")], [3.0, 4.0, 5.0, 6.0]); + // What the saved values can answer is calculated, and users of a kept value see it. + assert_eq!((t.num("B7"), t.num("B10")), (14.0, 21.0)); + // A name the link doesn't define, a refresh-error sheet's unsaved cell, and a formula + // with no saved value are still #REF!. + for at in ["B8", "B9", "B11"] { + assert_eq!(t.get(at), Value::Error(CellError::Ref), "{at}"); + } + }; + expect(&t); + // Recalculating again keeps them. + t.set("E9", "100"); + t.calc.recalc_all(&mut t.wb); + expect(&t); +} + +#[test] +fn a_kept_value_gives_way_when_what_the_formula_reads_here_changes() { + use gridcraft_model::{ExternalBook, ExternalName}; + let mut t = T::new(); + let names = vec![ExternalName { name: "Broken".into(), sheet: None, refers_to: "#REF!".into() }]; + t.wb.external_books = vec![ExternalBook { names, ..Default::default() }].into(); + let saved = [ + ("A1", "5", Value::Number(5.0)), + ("B1", "=IFERROR([1]!Broken,A1*3)", Value::Number(15.0)), + ("C1", "=IF(A1>0,A1*2,[1]!Broken)", Value::Number(10.0)), + ("D1", "=[1]!Broken+A1", Value::Number(25.0)), + ("E1", "=SUM([1]!Broken,A1)", Value::Number(25.0)), + ("F1", "=[1]!Broken", Value::Number(9.0)), + ("G1", "=F1*2", Value::Number(18.0)), + ]; + for (at, input, value) in saved { + let mut cell = if input.starts_with('=') { Cell::formula(Formula::new(input)) } else { Cell::value(value.clone()) }; + cell.value = value; + t.wb.sheet_mut(0).unwrap().cells.set(c(at), cell); + } + t.calc.recalc_all(&mut t.wb); + let nums = |t: &T| ["B1", "C1", "D1", "E1", "F1", "G1"].map(|a| t.get(a)); + let n = Value::Number; + // Loaded (or F9): what Excel saved stays. + assert_eq!(nums(&t), [n(15.0), n(10.0), n(25.0), n(25.0), n(9.0), n(18.0)]); + // An edit of a cell they read calculates them: the name is #REF!, as typed. + t.set("A1", "-1"); + let r = Value::Error(CellError::Ref); + assert_eq!(nums(&t), [n(-3.0), r.clone(), r.clone(), r, n(9.0), n(18.0)]); + // F9 again: formulas that read only the link keep what they show. + t.calc.recalc_all(&mut t.wb); + assert_eq!((t.get("F1"), t.get("G1")), (n(9.0), n(18.0))); +} + +#[test] +fn dde_and_ole_links_read_saved_items_and_keep_other_values() { + use gridcraft_model::ExternalBook; + let mut t = T::new(); + // [1] a DDE link with item ENE saved as 85.33; [2] an OLE link (or one whose part is missing). + let dde = ExternalBook { opaque: true, items: vec![("ENE".into(), Value::Number(85.33))], ..Default::default() }; + let ole = ExternalBook { opaque: true, ..Default::default() }; + t.wb.external_books = vec![dde, ole].into(); + let saved = [ + ("A1", "=[1]!ENE", Value::Number(1.0)), + ("A2", "=[1]!ene*2", Value::Number(2.0)), + ("A3", "=[1]!NoValues", Value::Number(3.0)), + ("A4", "=[2]!Item", Value::Number(4.0)), + ("A5", "=[2]Sheet!A1+1", Value::Number(5.0)), + ("A6", "=A1+A4", Value::Number(0.0)), + ]; + for (at, formula, value) in saved { + let mut cell = Cell::formula(Formula::new(formula)); + cell.value = value; + t.wb.sheet_mut(0).unwrap().cells.set(c(at), cell); + } + t.calc.recalc_all(&mut t.wb); + assert_eq!((t.num("A1"), t.num("A2")), (85.33, 170.66)); + assert_eq!([t.num("A3"), t.num("A4"), t.num("A5")], [3.0, 4.0, 5.0]); + assert!((t.num("A6") - 89.33).abs() < 1e-9, "{}", t.num("A6")); +} + +#[test] +fn if_family_range_arguments_with_a_deleted_linked_reference_are_ref() { + let mut t = T::new(); + t.set("A1", "1"); + for (at, f) in [("B1", "=SUMIF([1]S!#REF!,1)"), ("B2", "=COUNTIFS([1]S!#REF!,1)"), ("B3", "=SUMIF(#REF!,1)")] { + t.set(at, f); + assert_eq!(t.get(at), Value::Error(CellError::Ref), "{f}"); + } +} + +#[test] +fn references_into_linked_workbooks_keep_their_position_and_size() { + use gridcraft_model::{ExternalBook, ExternalSheet}; + let mut t = T::new(); + // Data!A1:B6: a header row, then items a..e priced 10..14. + let mut cells = vec![(c("A1"), Value::text("Item")), (c("B1"), Value::text("Price"))]; + for (i, item) in ["a", "b", "c", "d", "e"].into_iter().enumerate() { + cells.push((CellRef::new(i as u32 + 1, 0), Value::text(item))); + cells.push((CellRef::new(i as u32 + 1, 1), Value::Number(10.0 + i as f64))); + } + cells.sort_by_key(|(p, _)| *p); + t.wb.external_books = + vec![ExternalBook { sheets: vec![ExternalSheet { name: "Data".into(), cells, refresh_error: false }], ..Default::default() }].into(); + // `@` takes the cell on the formula's row (or column), as for this workbook's references. + for at in ["D2", "D3", "D4", "D5", "D6"] { + t.set(at, "=@[1]Data!B:B"); + } + assert_eq!([t.num("D2"), t.num("D3"), t.num("D4"), t.num("D5"), t.num("D6")], [10.0, 11.0, 12.0, 13.0, 14.0]); + t.set("F4", "=@[1]Data!B3:B6"); + t.set("F9", "=@[1]Data!B3:B6"); + t.set("B10", "=@[1]Data!A1:B1"); + assert_eq!((t.num("F4"), t.get("F9"), t.get("B10")), (12.0, Value::Error(CellError::Value), Value::text("Price"))); + // The reference keeps the size written, past the last saved cell. + t.set("G1", "=ROWS([1]Data!A:A)"); + t.set("G2", "=ROWS([1]Data!A1:A100000)"); + t.set("G3", "=COLUMNS([1]Data!1:1)"); + t.set("G4", "=INDEX([1]Data!B:B,4)"); + t.set("G5", "=INDEX([1]Data!A:A,10)"); + t.set("G6", "=SUM([1]Data!B:B)+VLOOKUP(\"c\",[1]Data!A:B,2,FALSE)"); + t.set("G7", "=ISREF([1]Data!B2)"); + assert_eq!( + (t.num("G1"), t.num("G2"), t.num("G3"), t.num("G4"), t.num("G5"), t.num("G6"), t.get("G7")), + (1_048_576.0, 100_000.0, 16_384.0, 12.0, 0.0, 72.0, Value::Bool(true)) + ); + // INDIRECT reads another workbook only while it is open. + t.set("H1", "=INDIRECT(\"[1]Data!B2\")"); + assert_eq!(t.get("H1"), Value::Error(CellError::Ref)); +} + +#[test] +fn offset_refuses_a_base_in_a_linked_workbook() { + use gridcraft_model::{ExternalBook, ExternalSheet}; + let mut t = T::new(); + // Curve!D8:D10 = 37 and Backup!D38 = 39: the saved link has every cell OFFSET would read, + // but Excel takes the ISERROR branch while the source workbook is closed (enron + // "West NatGas Prices 1031", PowerPrices!D9). + let curve = (8..=10).map(|r| (CellRef::new(r - 1, 3), Value::Number(37.0))).collect(); + let backup = vec![(c("D38"), Value::Number(39.0))]; + t.wb.external_books = vec![ExternalBook { + sheets: vec![ + ExternalSheet { name: "Curve".into(), cells: curve, refresh_error: false }, + ExternalSheet { name: "Backup".into(), cells: backup, refresh_error: false }, + ], + ..Default::default() + }] + .into(); + t.set("A1", "=OFFSET([1]Curve!$D$6,2,0,3,1)"); + t.set("A2", "=IF(ISERROR(AVERAGE(OFFSET([1]Curve!$D$6,2,0,3,1))),[1]Backup!$D$38,0)"); + t.set("A3", "=SUM(OFFSET(B1,0,0,2,1))"); + assert_eq!((t.get("A1"), t.num("A2"), t.num("A3")), (Value::Error(CellError::Value), 39.0, 0.0)); +} + +#[test] +fn if_family_ranges_inside_lambdas_stay_references() { + // MAP/BYROW hand their LAMBDA ranges of this workbook: COUNTIF and SUMIF take them. + let mut t = T::new(); + for (at, v) in [("E1", "1"), ("F1", "-2"), ("G1", "3"), ("E2", "-1"), ("F2", "5"), ("G2", "6")] { + t.set(at, v); + } + t.set("H1", "=BYROW(E1:G2,LAMBDA(r,COUNTIF(r,\">0\")))"); + t.set("I1", "=MAP(E1:E2,LAMBDA(x,SUMIF(x,\">0\")))"); + assert_eq!((t.num("H1"), t.num("H2"), t.num("I1"), t.num("I2")), (2.0, 2.0, 1.0, 0.0)); +} + #[test] fn xlookup_returns_a_reference() { let mut t = T::new(); diff --git a/crates/calc/src/trimrange.rs b/crates/calc/src/trimrange.rs index f95020e2e..27b0ba413 100644 --- a/crates/calc/src/trimrange.rs +++ b/crates/calc/src/trimrange.rs @@ -95,7 +95,7 @@ impl Evaluator<'_> { if (h as u64) * (w as u64) > MAX_CELLS { return Err(CellError::Num); } - let values = self.host.range_values(a.sheet, part); + let values = self.block_values(a.sheet, part); let ((r0, r1), (c0, c1)) = bounds(&values, w).ok_or(CellError::Calc)?; let (h_all, w_all) = (a.range.height() as usize, a.range.width() as usize); let (kr0, kr1) = keep(h_all, (r0, r1), rows); diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 383624b70..e9e036cf8 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -307,7 +307,7 @@ fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec, by_co && let Some(e) = f.expr() { let shifted = gridcraft_formula::adjust::shift_relative(e, dst.row as i64 - src.row as i64, dst.col as i64 - src.col as i64); - c.formula = Some(Arc::new(Formula::from_expr(shifted))); + c.formula = Some(Arc::new(f.rewritten(shifted))); } if let Some(picture) = sh.cell_pictures.get(&src) { pictures.push((dst, picture.clone())); @@ -951,6 +951,9 @@ fn validation(s: &mut Session, p: &Json) -> Result { "custom" => ValidationKind::Custom, _ => ValidationKind::Any, }; + for f in [str_param(p, "formula1"), str_param(p, "formula2")].into_iter().flatten() { + super::edit::refuse_linked_books(f)?; + } let dv = Validation { ranges: ranges.clone(), kind, diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index c69bac208..a21955103 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -149,6 +149,7 @@ pub(crate) fn input_to_cell(input: &str, sheet: usize, at: CellRef, wb: &mut gri parsed = gridcraft_formula::parse(&text); } let expr = parsed.map_err(|e| EngineError::Other(format!("There's a problem with this formula: {e}")))?; + check_linked_books(&expr, wb.external_books.len())?; let expr = wb.name_call_case(expr); check_arg_counts(&expr)?; let mut cell = Cell::formula(Formula::from_expr(expr)); @@ -191,6 +192,132 @@ pub(crate) fn input_to_cell(input: &str, sheet: usize, at: CellRef, wb: &mut gri Ok(Some(Cell { value: parsed.value, formula: None, style })) } +/// The workbook a sheet name (or the sheet part of a qualified name) is in: `Ok(None)` = this one, +/// `Ok(Some(n))` = `[n]Prices`, linked workbook `n` (`[0]` = this one). `Err` = a bracketed prefix +/// that names no workbook (`[-1]`, `[Book2.xlsx]`, `[ 1]`): a sheet name can't contain `[` or `]`, +/// and a formula names a linked workbook only by its index. +fn sheet_book(s: &str) -> std::result::Result, String> { + if !s.contains(['[', ']']) { + return Ok(None); + } + let bad = || s.find(']').and_then(|i| s.get(..=i)).unwrap_or(s).to_string(); + let Some((book, rest)) = s.strip_prefix('[').and_then(|x| x.split_once(']')) else { return Err(bad()) }; + if book.is_empty() || !book.bytes().all(|b| b.is_ascii_digit()) || rest.contains(['[', ']']) { + return Err(bad()); + } + book.parse().map(Some).map_err(|_| bad()) +} + +/// The sheets an expression names, as written: both ends of a sheet range (`'[1]Jan:Dec'`) and the +/// sheet part of qualified names (`[1]!Rates`, `[1]Prices!Rates`). `true` = the second sheet of a +/// range, which can't carry a workbook index (MS-XLSX `sheet-range`). +fn sheets_named(e: &gridcraft_formula::Expr) -> Vec<(&str, bool)> { + use gridcraft_formula::{Expr, SheetSel}; + match e { + Expr::Ref(gridcraft_formula::Reference { sheet, .. }) | Expr::LinkedRefError(sheet) => match sheet { + SheetSel::Named(a) => vec![(a.as_str(), false)], + SheetSel::Span(a, b) => vec![(a.as_str(), false), (b.as_str(), true)], + SheetSel::Current => vec![], + }, + Expr::Name(n) => n.rsplit_once('!').map(|(s, _)| vec![(s, false)]).unwrap_or_default(), + _ => vec![], + } +} + +/// The first workbook prefix in `expr` that isn't `[0]` or one of `links` linked workbooks +/// (`[1]` is the first), as written (`[5]`, `[Book2.xlsx]`). +fn first_bad_book(expr: &gridcraft_formula::Expr, links: usize) -> Option { + let mut bad = None; + expr.walk(&mut |e| { + for (s, range_end) in sheets_named(e) { + if bad.is_some() { + return; + } + bad = match sheet_book(s) { + Ok(None) => None, + Ok(Some(n)) if n <= links && !range_end => None, + Ok(Some(_)) => s.find(']').and_then(|i| s.get(..=i)).map(str::to_string), + Err(prefix) => Some(prefix), + }; + } + }); + bad +} + +/// A cell reference after a bare workbook index (`[1]!A1`, `SUM([1]!A1:B2)`, `[0]!A1`), as written. +/// Only a defined name can follow `[n]!`: the reference would be to a sheet called `[1]`, which no +/// sheet can be. +fn first_book_only_ref(expr: &gridcraft_formula::Expr) -> Option { + use gridcraft_formula::{Expr, SheetSel}; + let book_only = |s: &str| matches!(sheet_book(s), Ok(Some(_))) && s.ends_with(']'); + let mut bad = None; + expr.walk(&mut |e| { + let sheet = match e { + // `[1]!#REF!` too: an error, not a name. + Expr::Ref(gridcraft_formula::Reference { sheet: SheetSel::Named(a), .. }) | Expr::LinkedRefError(SheetSel::Named(a)) => Some(a.as_str()), + _ => None, + }; + if bad.is_none() + && let Some(s) = sheet.filter(|s| book_only(s)) + { + bad = Some(s.to_string()); + } + }); + bad +} + +/// Typed references into other workbooks. `[n]` must name one of the workbook's `links` external +/// links (`[1]` is the first): Excel refuses any other index on entry, and a file saved with one +/// can't be opened. `[0]` is this workbook, as in Excel (the parser enters `[0]Sheet1!A1` as +/// `Sheet1!A1`). Only a defined name can follow a bare `[n]!`. +pub(crate) fn check_linked_books(expr: &gridcraft_formula::Expr, links: usize) -> Result<()> { + if let Some(book) = first_bad_book(expr, links) { + return Err(EngineError::Other(format!("There's a problem with this formula: {book} isn't a linked workbook."))); + } + if let Some(book) = first_book_only_ref(expr) { + return Err(EngineError::Other(format!("There's a problem with this formula: only a defined name can follow {book}!."))); + } + Ok(()) +} + +/// Conditional formats and data validation can't refer to other workbooks: Excel refuses it in +/// those dialogs. Text that isn't a formula is left to the caller. +pub(crate) fn refuse_linked_books(text: &str) -> Result<()> { + match gridcraft_formula::parse(text.trim_start_matches('=')) { + Ok(e) if first_bad_book(&e, 0).is_some() || first_book_only_ref(&e).is_some() => { + Err(EngineError::Other("There's a problem with this formula: references to other workbooks can't be used here.".into())) + } + _ => Ok(()), + } +} + +/// Which of the source workbook's links (`[1]` first) the target holds at the same index: the same +/// linked workbook with the same saved sheets and names. A New Window of the workbook shares them. +fn shared_links(src: &gridcraft_model::Workbook, dst: &gridcraft_model::Workbook) -> Vec { + if std::sync::Arc::ptr_eq(&src.external_books, &dst.external_books) { + return vec![true; src.external_books.len()]; + } + src.external_books.iter().enumerate().map(|(i, b)| dst.external_books.get(i) == Some(b)).collect() +} + +/// A formula pasted from another workbook: its references into links the target doesn't share +/// (`shared`, from [`shared_links`]) are `#REF!`. `[1]` there would be a link it doesn't have, or a +/// different workbook. +fn drop_linked_books(expr: gridcraft_formula::Expr, shared: &[bool]) -> gridcraft_formula::Expr { + use gridcraft_formula::Expr; + if shared.iter().all(|&s| s) && first_bad_book(&expr, shared.len()).is_none() { + return expr; + } + expr.map(&mut |e| { + let unshared = sheets_named(&e).iter().any(|(s, _)| match sheet_book(s) { + Ok(None | Some(0)) => false, + Ok(Some(n)) => !shared.get(n - 1).copied().unwrap_or(false), + Err(_) => true, + }); + if unshared { Expr::Error(gridcraft_core::CellError::Ref) } else { e } + }) +} + /// Like Excel, a formula that calls a built-in function with too few or too many arguments is /// refused when it's entered, rather than stored to show `#VALUE!`. Functions the registry doesn't /// hold (special forms like LET, LAMBDA, OFFSET, INDIRECT, SUBTOTAL), defined LAMBDA names and @@ -349,7 +476,7 @@ fn range_fill(s: &mut Session, p: &Json) -> Result { && let Some(e) = f.expr() { let shifted = gridcraft_formula::adjust::shift_relative(e, at.row as i64 - origin.row as i64, at.col as i64 - origin.col as i64); - cell.formula = Some(std::sync::Arc::new(Formula::from_expr(shifted))); + cell.formula = Some(std::sync::Arc::new(f.rewritten(shifted))); } cell.style = cx.wb.sheet(sheet).and_then(|sh| sh.cell(at)).map(|c| c.style).unwrap_or(cell.style); cx.sheet_mut(sheet)?.set_cell(at, cell); @@ -936,6 +1063,7 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { let src_wb = clip.wb.clone(); let Some(src_sheet) = src_wb.sheet(clip.sheet) else { return Err(EngineError::Other("The copied sheet no longer exists.".into())) }; let src_sheet_name = src_sheet.name.clone(); + let shared = if same_doc { vec![] } else { shared_links(&src_wb, &s.doc()?.wb) }; edit(s, |cx| { // Styles from another workbook must be re-interned. let mut style_map = std::collections::HashMap::new(); @@ -981,6 +1109,8 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { let shift = |cell: &Cell| -> Option> { let f = cell.formula.as_ref()?; let e = f.expr()?; + // `[1]…` is the source workbook's first link: kept where this one shares it. + let e = if same_doc { e } else { drop_linked_books(e, &shared) }; if cut_move { // A moved formula still reads cells on its original sheet. Qualify // those references before the workbook-wide move adjustment below. @@ -995,7 +1125,7 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { } else { e }; - let mut moved = Formula::from_expr(e); + let mut moved = f.rewritten(e); // The array range moves with its formula. let (dr, dc) = (dest.row as i64 - sc.row as i64, dest.col as i64 - sc.col as i64); moved.array = f.array.and_then(|a| Some(RangeRef::new(a.start.offset(dr, dc)?, a.end.offset(dr, dc)?))); @@ -1003,7 +1133,7 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { } let moved = gridcraft_formula::adjust::shift_relative(e, dest.row as i64 - sc.row as i64, dest.col as i64 - sc.col as i64); - Some(std::sync::Arc::new(Formula::from_expr(moved))) + Some(std::sync::Arc::new(f.rewritten(moved))) }; if link { let sheet_prefix = if dest_sheet == clip.sheet && same_doc { @@ -1020,7 +1150,10 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { What::All | What::AllExceptBorders => { new_picture = src_picture.clone(); new.formula = shift(&sc_cell); - new.value = if new.formula.is_some() { Value::Empty } else { sc_cell.value.clone() }; + // A moved formula keeps the value it showed until it is + // calculated: one over a linked value that can't be + // calculated keeps showing it, as it would where it was. + new.value = if new.formula.is_some() && !cut_move { Value::Empty } else { sc_cell.value.clone() }; let st = map_style(sc_cell.style, &mut cx.wb); new.style = if what == What::AllExceptBorders { let borders = cx.wb.styles.get(old.style).borders; @@ -1032,7 +1165,7 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { What::Formulas | What::FormulasAndNumberFormats => { new_picture = src_picture.clone(); new.formula = shift(&sc_cell); - new.value = if new.formula.is_some() { Value::Empty } else { sc_cell.value.clone() }; + new.value = if new.formula.is_some() && !cut_move { Value::Empty } else { sc_cell.value.clone() }; if what == What::FormulasAndNumberFormats { let nf = src_wb.styles.get(sc_cell.style).num_fmt.clone(); new.style = cx.wb.styles.derive(old.style, |s| s.num_fmt = nf); @@ -1164,7 +1297,10 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { } else { gridcraft_formula::adjust::Edit::MoveToSheet { from: src, to_sheet: dest_name, to_row: at.row, to_col: at.col } }; - rewrite_all_formulas(&mut cx.wb, &src_sheet_name, &e); + let (forgotten, moved, changed) = rewrite_all_formulas(&mut cx.wb, &src_sheet_name, &e); + cx.changed.extend(forgotten); + cx.moved_names.extend(moved); + cx.changed_names.extend(changed); cx.structural = true; } let end = at.offset_clamped((tiles_r * h) as i64 - 1, (tiles_c * w) as i64 - 1); @@ -1177,9 +1313,40 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { Ok(json!({"pasted": s.doc()?.selection.a1()})) } +/// Whether a structural edit made a formula read other cells than it did, not the same cells +/// moved: a reference became `#REF!`, or a range grew or shrank. +pub(crate) fn reads_differently(old: &gridcraft_formula::Expr, new: &gridcraft_formula::Expr) -> bool { + fn reads(e: &gridcraft_formula::Expr) -> Vec> { + use gridcraft_formula::Expr; + let mut out = Vec::new(); + e.walk(&mut |x| match x { + Expr::Ref(r) => out.push(Some((r.range().height(), r.range().width()))), + Expr::Error(gridcraft_core::CellError::Ref) | Expr::LinkedRefError(_) => out.push(None), + _ => {} + }); + out + } + reads(old) != reads(new) +} + +/// A formula whose references a structural edit changed no longer shows its value: clearing it +/// means a formula over a linked name that can't be calculated is calculated again, rather than +/// keeping the value it had, and the edit counts it as changed for the formulas that read it +/// (see `Calc::recalc_all_after_edit`). +pub(crate) fn forget_value(cell: &mut Cell) { + cell.value = Value::Empty; +} + /// Applies a reference adjustment to every cell formula and defined name in the workbook. -pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: &str, e: &gridcraft_formula::adjust::Edit) { +/// Returns formulas whose values were cleared, names whose references only moved, and names whose +/// references changed (lowercase). +pub(crate) fn rewrite_all_formulas( + wb: &mut gridcraft_model::Workbook, + target: &str, + e: &gridcraft_formula::adjust::Edit, +) -> (Vec<(usize, CellRef)>, Vec, Vec) { use gridcraft_formula::adjust::Edit; + let mut forgotten = Vec::new(); // Rows and columns inserted or deleted on the target sheet move its legacy array ranges too // (a cut moves its arrays itself). let moves_arrays = matches!(e, Edit::Insert { .. } | Edit::Delete { .. } | Edit::InsertCells { .. } | Edit::DeleteCells { .. }); @@ -1193,8 +1360,8 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: & let on_target = moves_arrays && host.eq_ignore_ascii_case(target); let Some(sheet) = wb.sheet_mut(si) else { continue }; for (c, f) in keys { - let Some(expr) = f.expr() else { continue }; - let new = gridcraft_formula::adjust::adjust(expr, &host, target, e); + let Some(old) = f.expr_arc() else { continue }; + let new = gridcraft_formula::adjust::adjust((*old).clone(), &host, target, e); let text = gridcraft_formula::print(&new); let array = match f.array { Some(a) if on_target => super::sheet::map_range(a, e), @@ -1203,19 +1370,34 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: & if (text != f.text || array != f.array) && let Some(cell) = sheet.cells.get_mut(c) { - let mut nf = Formula::from_expr(new); + if reads_differently(&old, &new) { + forget_value(cell); + forgotten.push((si, c)); + } + let mut nf = f.rewritten(new); nf.array = array; cell.formula = Some(std::sync::Arc::new(nf)); } } } + let mut moved = Vec::new(); + let mut changed = Vec::new(); for n in wb.names.iter_mut() { if let Ok(expr) = gridcraft_formula::parse(&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); + let new = gridcraft_formula::adjust::adjust(expr.clone(), "\u{0}", target, e); + let text = gridcraft_formula::print(&new); + if text != n.formula { + if reads_differently(&expr, &new) { + changed.push(n.name.to_lowercase()); + } else { + moved.push(n.name.to_lowercase()); + } + } + n.formula = text; } } + (forgotten, moved, changed) } // ---------------------------------------------------------------- clear diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index 40df72df0..1ca22b93a 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -262,7 +262,7 @@ fn define_name(s: &mut Session, p: &Json) -> Result { None | Some("Workbook") => None, Some(sh) => Some(d.wb.sheet_index(sh).ok_or_else(|| bad("formulas.defineName", "no such sheet"))?), }; - let refers = match str_param(p, "refersTo") { + let mut 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('=')) { @@ -280,8 +280,12 @@ fn define_name(s: &mut Session, p: &Json) -> Result { format!("{}!{}", gridcraft_formula::quote_sheet(&sh.name), body) } }; - if gridcraft_formula::parse(&refers).is_err() { - return Err(EngineError::Other("There's a problem with this formula.".into())); + let parsed = gridcraft_formula::parse(&refers).map_err(|_| EngineError::Other("There's a problem with this formula.".into()))?; + super::edit::check_linked_books(&parsed, d.wb.external_books.len())?; + // A workbook prefix is stored as the parser reads it: `[0]Sheet1!$C$1` as `Sheet1!$C$1`, so + // it follows the sheet when it's renamed. + if refers.contains('[') { + refers = gridcraft_formula::print(&parsed); } let comment = str_param(p, "comment").unwrap_or("").to_string(); // Editing an existing name (Name Manager › Edit…): it must exist, and its new name must not @@ -346,7 +350,7 @@ fn rename_name_in_formulas(wb: &mut gridcraft_model::Workbook, old: &str, old_sc let formulas: Vec<(CellRef, std::sync::Arc)> = sh.cells.iter().filter_map(|(c, x)| x.formula.clone().map(|f| (c, f))).collect(); for (c, f) in formulas { let Some(e) = f.expr().and_then(rename) else { continue }; - let mut nf = Formula::from_expr(e); + let mut nf = f.rewritten(e); nf.array = f.array; if let Some(cell) = sh.cells.get_mut(c) { cell.formula = Some(std::sync::Arc::new(nf)); diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index d878e4469..aa0802b21 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -506,7 +506,7 @@ fn table_rename(s: &mut Session, p: &Json) -> Result { o => o, }); if let Some(cell) = sh.cells.get_mut(c) { - cell.formula = Some(std::sync::Arc::new(Formula::from_expr(ne))); + cell.formula = Some(std::sync::Arc::new(f.rewritten(ne))); } } } @@ -1350,8 +1350,9 @@ fn add_cf(s: &mut Session, p: &Json) -> Result { _ => preset_style(g("preset").as_deref().unwrap_or("lightRedFillDarkRedText")), }); let kind = g("type").unwrap_or_else(|| "cellIs".into()); - let value_kind = |v: Option<&Json>, default: CfValueKind| -> CfValueKind { - match v { + // A `formula` threshold can't refer to other workbooks either, as for the rules' formulas. + let value_kind = |v: Option<&Json>, default: CfValueKind| -> Result { + Ok(match v { Some(Json::Object(o)) => { let t = o.get("type").and_then(Json::as_str).unwrap_or("num"); let val = o.get("value").and_then(Json::as_f64).unwrap_or(0.0); @@ -1360,12 +1361,16 @@ fn add_cf(s: &mut Session, p: &Json) -> Result { "max" => CfValueKind::Max, "percent" => CfValueKind::Percent(val), "percentile" => CfValueKind::Percentile(val), - "formula" => CfValueKind::Formula(o.get("value").and_then(Json::as_str).unwrap_or("").into()), + "formula" => { + let f = o.get("value").and_then(Json::as_str).unwrap_or(""); + super::edit::refuse_linked_books(f)?; + CfValueKind::Formula(f.into()) + } _ => CfValueKind::Number(val), } } _ => default, - } + }) }; let rule = match kind.as_str() { "cellIs" => { @@ -1390,9 +1395,17 @@ fn add_cf(s: &mut Session, p: &Json) -> Result { format!("\"{}\"", v.replace('"', "\"\"")) } }; - CfRule::CellIs { op, a: quote(a), b: b.map(quote), style } + let (a, b) = (quote(a), b.map(quote)); + for f in std::iter::once(&a).chain(b.as_ref()) { + super::edit::refuse_linked_books(f)?; + } + CfRule::CellIs { op, a, b, style } + } + "expression" => { + let formula = g("formula").unwrap_or_default().trim_start_matches('=').to_string(); + super::edit::refuse_linked_books(&formula)?; + CfRule::Expression { formula, style } } - "expression" => CfRule::Expression { formula: g("formula").unwrap_or_default().trim_start_matches('=').to_string(), style }, "containsText" => CfRule::ContainsText { text: g("text").unwrap_or_default(), style }, "notContainsText" => CfRule::NotContainsText { text: g("text").unwrap_or_default(), style }, "beginsWith" => CfRule::BeginsWith { text: g("text").unwrap_or_default(), style }, @@ -1428,8 +1441,8 @@ fn add_cf(s: &mut Session, p: &Json) -> Result { CfRule::ColorScale { stops } } "dataBar" => CfRule::DataBar { - min: value_kind(rule.get("min"), CfValueKind::Min), - max: value_kind(rule.get("max"), CfValueKind::Max), + min: value_kind(rule.get("min"), CfValueKind::Min)?, + max: value_kind(rule.get("max"), CfValueKind::Max)?, color: super::format::color_param(rule.get("color")).unwrap_or(Color::rgb(0x63, 0x8E, 0xC6)), gradient: rule.get("gradient").and_then(Json::as_bool).unwrap_or(true), show_value: true, diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 50865a2b7..a90ed74a0 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -314,6 +314,11 @@ pub struct Ctx<'a> { /// The command applied sheet protection itself (row/column inserts and deletes the /// protection allows), so cells it shifts aren't refused as edits of locked cells. pub protection_checked: bool, + /// Names (lowercase) whose references a structural edit only moved, so they read what they + /// did (see `edit::rewrite_all_formulas`). + pub moved_names: Vec, + /// Names whose references changed during any structural rewrite. + pub changed_names: Vec, } pub(crate) fn edit(s: &mut Session, f: impl FnOnce(&mut Ctx) -> Result) -> Result { @@ -325,15 +330,72 @@ pub(crate) fn edit(s: &mut Session, f: impl FnOnce(&mut Ctx) -> Result) -> }) } +/// The defined names and table names an edit added, removed or changed. A formula over a linked +/// name or table that cannot be calculated keeps its saved value unless what it reads changes. +fn changed_names(old: &Workbook, new: &Workbook, moved: &[String]) -> Vec { + if new.external_books.is_empty() { + return Vec::new(); + } + // Defined names are compared by name, whatever the scope: deleting a sheet renumbers the + // scopes after it. + fn group(wb: &Workbook) -> std::collections::HashMap> { + let mut m: std::collections::HashMap> = std::collections::HashMap::new(); + for n in &wb.names { + m.entry(n.name.to_lowercase()).or_default().push(n.formula.as_str()); + } + m.values_mut().for_each(|v| v.sort_unstable()); + m + } + let (before, after) = (group(old), group(new)); + let moved: std::collections::HashSet<&str> = moved.iter().map(String::as_str).collect(); + let mut out: Vec = + before.keys().chain(after.keys()).filter(|k| before.get(*k) != after.get(*k) && !moved.contains(k.as_str())).cloned().collect(); + + // Table structured references are tracked as dynamic reads. A table's range, name, header / + // totals geometry, or column names changing changes what such a formula reads. Compare the + // stable table id so a rename is paired with the same table; missing ids cover delete/convert. + fn tables(wb: &Workbook) -> std::collections::HashMap)> { + wb.sheets + .iter() + .flat_map(|sh| sh.tables.iter()) + .map(|t| (t.id, (t.name.as_str(), t.range, t.header_row, t.totals_row, t.columns.iter().map(|c| c.name.as_str()).collect()))) + .collect() + } + let (before, after) = (tables(old), tables(new)); + for id in before.keys().chain(after.keys()) { + if before.get(id) == after.get(id) { + continue; + } + if let Some((name, ..)) = before.get(id) { + out.push((*name).to_string()); + } + if let Some((name, ..)) = after.get(id) { + out.push((*name).to_string()); + } + } + out.sort_unstable_by_key(|x| x.to_lowercase()); + out.dedup_by(|a, b| a.eq_ignore_ascii_case(b)); + out +} + pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) -> Result { // `GRIDCRAFT_PROFILE` timing. Read the clock only when profiling, and never on wasm: there // `Instant::now` traps (`RuntimeError: unreachable`), which broke every edit in the web app. #[allow(clippy::disallowed_methods)] let t0 = if cfg!(target_arch = "wasm32") { None } else { std::env::var_os("GRIDCRAFT_PROFILE").map(|_| std::time::Instant::now()) }; let mut sel = d.selection.clone(); - let mut ctx = Ctx { wb: (*d.wb).clone(), changed: Vec::new(), structural: false, sel: &mut sel, fit_rows: Vec::new(), protection_checked: false }; + let mut ctx = Ctx { + wb: (*d.wb).clone(), + changed: Vec::new(), + structural: false, + sel: &mut sel, + fit_rows: Vec::new(), + protection_checked: false, + moved_names: Vec::new(), + changed_names: Vec::new(), + }; let r = f(&mut ctx)?; - let Ctx { mut wb, changed, structural, mut fit_rows, protection_checked, .. } = ctx; + let Ctx { mut wb, changed, structural, mut fit_rows, protection_checked, moved_names, changed_names: rewrite_changed_names, .. } = ctx; if !protection_checked { check_protection(&d.wb, &wb, &d.unlocked_ranges)?; } @@ -347,7 +409,11 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) eprintln!("commit: edit {:?}, {} changed", t0.elapsed(), changed.len()); } if structural { - d.calc.recalc_all(&mut wb); + let mut names = changed_names(&d.wb, &wb, &moved_names); + names.extend(rewrite_changed_names); + names.sort_unstable(); + names.dedup(); + d.calc.recalc_all_after_edit(&mut wb, &changed, &names); } else if !changed.is_empty() { d.calc.cells_changed(&mut wb, &changed); } diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index 7870dee4e..f6d8c3794 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -213,9 +213,55 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert: } } +/// Whether a formula's 3-D references cover a different set of sheets after a structural edit. +fn spans_changed(e: &gridcraft_formula::Expr, before: &[String], after: &[String]) -> bool { + let members = |order: &[String], a: &str, b: &str| { + let (Some(i), Some(j)) = (order.iter().position(|x| x.eq_ignore_ascii_case(a)), order.iter().position(|x| x.eq_ignore_ascii_case(b))) else { + return Vec::new(); + }; + order[i.min(j)..=i.max(j)].iter().map(|x| x.to_ascii_lowercase()).collect::>() + }; + let mut changed = false; + e.walk(&mut |x| { + if let gridcraft_formula::Expr::Ref(r) = x + && let gridcraft_formula::SheetSel::Span(a, b) = &r.sheet + { + changed |= members(before, a, b) != members(after, a, b); + } + }); + changed +} +/// Names containing a 3-D reference can keep the same text while their member sheets change. +/// Mark them as changed so formulas that read the name give up a linked workbook's saved value. +fn mark_changed_name_spans(cx: &mut Ctx, before: &[String], after: &[String]) { + let names: Vec = cx + .wb + .names + .iter() + .filter_map(|n| gridcraft_formula::parse(&n.formula).ok().filter(|e| spans_changed(e, before, after)).map(|_| n.name.to_lowercase())) + .collect(); + cx.changed_names.extend(names); +} + /// Rewrites every formula reference (and chart series) for a structural edit on `target`. -fn rewrite(wb: &mut gridcraft_model::Workbook, target: &str, e: &Edit) { - super::edit::rewrite_all_formulas(wb, target, e); +/// Formulas that now read other cells count as changed, and names whose references only moved +/// don't (see `edit::rewrite_all_formulas`). +fn rewrite(cx: &mut Ctx, target: &str, e: &Edit) { + // A single command can apply several edits (multi-area delete, column move). Keep formulas + // forgotten by an earlier step at their final positions so their readers are seeded there. + let old = std::mem::take(&mut cx.changed); + cx.changed = old + .into_iter() + .filter_map(|(si, c)| { + let host = cx.wb.sheet(si).map(|s| s.name.as_str())?; + if host.eq_ignore_ascii_case(target) { map_range(RangeRef::cell(c), e).map(|r| (si, r.start)) } else { Some((si, c)) } + }) + .collect(); + let (forgotten, moved, changed) = super::edit::rewrite_all_formulas(&mut cx.wb, target, e); + cx.changed.extend(forgotten); + cx.moved_names.extend(moved); + cx.changed_names.extend(changed); + let wb = &mut cx.wb; for si in 0..wb.sheets.len() { let host = wb.sheets.get(si).map(|s| s.name.clone()).unwrap_or_default(); let Some(sh) = wb.sheet_mut(si) else { continue }; @@ -284,7 +330,7 @@ fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { } } } - rewrite(&mut cx.wb, &name, &Edit::Insert { axis, at, count }); + rewrite(cx, &name, &Edit::Insert { axis, at, count }); cx.structural = true; Ok(Json::Null) })?; @@ -349,7 +395,7 @@ fn delete_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { } shift_sheet_features(sh, axis, at, count, false); } - rewrite(&mut cx.wb, &name, &Edit::Delete { axis, at, count }); + rewrite(cx, &name, &Edit::Delete { axis, at, count }); } cx.structural = true; Ok(Json::Null) @@ -399,7 +445,7 @@ fn move_columns(s: &mut Session, p: &Json) -> Result { sh.cells.shift_cols(before, count as i64); shift_sheet_features(sh, Axis::Cols, before, count, true); } - rewrite(&mut cx.wb, &name, &Edit::Insert { axis: Axis::Cols, at: before, count }); + rewrite(cx, &name, &Edit::Insert { axis: Axis::Cols, at: before, count }); let mut touched = Vec::new(); { @@ -437,14 +483,14 @@ fn move_columns(s: &mut Session, p: &Json) -> Result { for cell in touched { cx.touch(sheet, cell); } - rewrite(&mut cx.wb, &name, &Edit::Move { from: source, to_row: 0, to_col: destination }); + rewrite(cx, &name, &Edit::Move { from: source, to_row: 0, to_col: destination }); { let sh = cx.sheet_mut(sheet)?; sh.cells.shift_cols(delete_at + count, -(count as i64)); shift_sheet_features(sh, Axis::Cols, delete_at, count, false); } - rewrite(&mut cx.wb, &name, &Edit::Delete { axis: Axis::Cols, at: delete_at, count }); + rewrite(cx, &name, &Edit::Delete { axis: Axis::Cols, at: delete_at, count }); *cx.sel = crate::Selection { active: CellRef::new(active_row, final_start), anchor: CellRef::new(active_row, final_start), @@ -480,7 +526,7 @@ fn insert_cells(s: &mut Session, p: &Json) -> Result { } shift_cell_features(sh, &e); let name = cx.wb.sheet(sheet).map(|s| s.name.clone()).unwrap_or_default(); - rewrite(&mut cx.wb, &name, &e); + rewrite(cx, &name, &e); cx.structural = true; Ok(Json::Null) })?; @@ -515,7 +561,7 @@ fn delete_cells(s: &mut Session, p: &Json) -> Result { shift_cell_features(sh, &e); } // References into the deleted cells become #REF!; the rest move. - rewrite(&mut cx.wb, &name, &e); + rewrite(cx, &name, &e); cx.structural = true; Ok(Json::Null) })?; @@ -725,15 +771,22 @@ fn delete_sheet(s: &mut Session, p: &Json) -> Result { let d = s.doc_mut()?; edit_doc(d, |cx| { cx.wb.sheets.remove(i); + let new_order: Vec = cx.wb.sheets.iter().map(|s| s.name.clone()).collect(); for si in 0..cx.wb.sheets.len() { let keys: Vec<(CellRef, Arc)> = cx.wb.sheets.get(si).map(|s| s.cells.iter().filter_map(|(c, x)| x.formula.clone().map(|f| (c, f))).collect()).unwrap_or_default(); let Some(sh) = cx.wb.sheet_mut(si) else { continue }; for (c, f) in keys { - if let Some(e) = f.expr() { - let ne = gridcraft_formula::adjust::delete_sheet(e, &name, &order); + if let Some(e) = f.expr_arc() { + let ne = gridcraft_formula::adjust::delete_sheet((*e).clone(), &name, &order); if let Some(cell) = sh.cells.get_mut(c) { - cell.formula = Some(Arc::new(gridcraft_model::Formula::from_expr(ne))); + // A reference into the deleted sheet, or a 3-D span losing an interior + // sheet, reads something else now even when its text is unchanged. + if ne != *e || spans_changed(&e, &order, &new_order) { + super::edit::forget_value(cell); + cx.changed.push((si, c)); + } + cell.formula = Some(Arc::new(f.rewritten(ne))); } } } @@ -753,6 +806,7 @@ fn delete_sheet(s: &mut Session, p: &Json) -> Result { } } } + mark_changed_name_spans(cx, &order, &new_order); if cx.wb.active_sheet >= cx.wb.sheets.len() || cx.wb.active_sheet > i { cx.wb.active_sheet = cx.wb.active_sheet.saturating_sub(1).min(cx.wb.sheets.len().saturating_sub(1)); } @@ -784,7 +838,7 @@ fn rename_sheet(s: &mut Session, p: &Json) -> Result { if text != f.text && let Some(cell) = sh.cells.get_mut(c) { - cell.formula = Some(Arc::new(gridcraft_model::Formula::from_expr(ne))); + cell.formula = Some(Arc::new(f.rewritten(ne))); } } } @@ -809,6 +863,8 @@ fn rename_sheet(s: &mut Session, p: &Json) -> Result { } for n in cx.wb.names.iter_mut() { if let Ok(e) = gridcraft_formula::parse(&n.formula) { + // The same cells, under the sheet's new name. + cx.moved_names.push(n.name.to_lowercase()); n.formula = gridcraft_formula::print(&gridcraft_formula::adjust::rename_sheet(e, &old, &name)); } } @@ -821,6 +877,7 @@ fn move_sheet(s: &mut Session, p: &Json) -> Result { let i = target_sheet(s, p)?; let copy = bool_param(p, "copy").unwrap_or(false); let to = p.get("to").and_then(Json::as_u64).map(|v| v as usize).ok_or_else(|| bad("sheet.move", "missing `to`"))?; + let old_order: Vec = s.doc()?.wb.sheets.iter().map(|sh| sh.name.clone()).collect(); edit(s, |cx| { let n = cx.wb.sheets.len(); let Some(src) = cx.wb.sheets.get(i).cloned() else { return Err(bad("sheet.move", "no such sheet")) }; @@ -880,6 +937,22 @@ fn move_sheet(s: &mut Session, p: &Json) -> Result { } } } + let new_order: Vec = cx.wb.sheets.iter().map(|sh| sh.name.clone()).collect(); + for si in 0..cx.wb.sheets.len() { + let keys: Vec<(CellRef, Arc)> = + cx.wb.sheets.get(si).map(|sh| sh.cells.iter().filter_map(|(c, x)| x.formula.clone().map(|f| (c, f))).collect()).unwrap_or_default(); + let Some(sh) = cx.wb.sheet_mut(si) else { continue }; + for (c, f) in keys { + if let Some(e) = f.expr_arc() + && spans_changed(&e, &old_order, &new_order) + && let Some(cell) = sh.cells.get_mut(c) + { + super::edit::forget_value(cell); + cx.changed.push((si, c)); + } + } + } + mark_changed_name_spans(cx, &old_order, &new_order); cx.structural = true; Ok(Json::Null) }) diff --git a/crates/engine/src/fill.rs b/crates/engine/src/fill.rs index 5848bb0ca..071a303a9 100644 --- a/crates/engine/src/fill.rs +++ b/crates/engine/src/fill.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use gridcraft_core::date::{MONTHS, WEEKDAYS, datetime_from_serial, days_in_month, serial_from_ymd}; use gridcraft_core::{CellRef, RangeRef, Value}; -use gridcraft_model::{Cell, Formula}; +use gridcraft_model::Cell; use gridcraft_numfmt::{FormatKind, NumberFormat}; use crate::Result; @@ -98,7 +98,7 @@ pub fn fill(cx: &mut Ctx, sheet: usize, src: RangeRef, target: RangeRef, mode: F if let Some(f) = &cell.formula { if let Some(e) = f.expr() { let shifted = gridcraft_formula::adjust::shift_relative(e, t.row as i64 - sc.row as i64, t.col as i64 - sc.col as i64); - cell.formula = Some(Arc::new(Formula::from_expr(shifted))); + cell.formula = Some(Arc::new(f.rewritten(shifted))); cell.value = Value::Empty; } } else if let Some(v) = match per_cell.get(src_i) { diff --git a/crates/engine/src/tables.rs b/crates/engine/src/tables.rs index 9182cd488..605d71369 100644 --- a/crates/engine/src/tables.rs +++ b/crates/engine/src/tables.rs @@ -244,7 +244,7 @@ pub(crate) fn sync_headers(cx: &mut crate::cmd::Ctx) { o => o, }); if hit && let Some(cell) = cx.wb.sheet_mut(i).and_then(|s| s.cells.get_mut(c)) { - cell.formula = Some(std::sync::Arc::new(Formula::from_expr(ne))); + cell.formula = Some(std::sync::Arc::new(f.rewritten(ne))); } } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..5cf82e405 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -241,6 +241,308 @@ fn formulas_with_the_wrong_argument_count_are_refused_on_entry() { assert_eq!(v(&s, "C1"), Value::text("=DATEDIF(1,2)")); } +#[test] +fn typed_references_to_workbooks_that_are_not_linked_are_refused_on_entry() { + // A file saved with `[2]Sheet1!A1` and only one external link (or `[1]…` and none) can't be + // opened by Excel 365, which refuses such an index on entry. + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "kept"})).unwrap(); + let refused = |s: &mut Session, input: &str| { + let e = s.execute("cell.set", json!({"cell": "A1", "input": input})).unwrap_err().to_string(); + assert!(e.contains("There's a problem with this formula"), "{input}: {e}"); + assert_eq!(v(s, "A1"), Value::text("kept"), "{input} must not replace the cell"); + }; + for input in [ + "=[1]Sheet1!A1", + "='[1]My Sheet'!A1:B2", + "=SUM([1]Jan:Dec!A1)", + "=[1]!Rates", + "=[99999999999999999999]Sheet1!A1", + // Either end of a sheet range, and bracketed prefixes that aren't an index. + "='Sheet1:[5]Dec'!A1", + "='[-1]Sheet1'!A1", + "='[ 1]Sheet1'!A1", + "='[Book2.xlsx]Sheet1'!A1", + ] { + refused(&mut s, input); + } + assert!(s.execute("formulas.defineName", json!({"name": "Linked", "refersTo": "=[1]Sheet1!A1"})).is_err()); + // Conditional formats and data validation can't refer to other workbooks. + let cf = |f: &str| json!({"range": "A1:A3", "rule": {"type": "expression", "formula": f}}); + assert!(s.execute("home.conditionalFormat", cf("=[3]Sheet1!A1>0")).is_err()); + assert!(s.execute("data.validation", json!({"range": "A1", "type": "list", "formula1": "=[3]Sheet1!A1:A3"})).is_err()); + assert!(s.execute("data.validation", json!({"range": "A1", "type": "custom", "formula1": "=[3]Sheet1!A1>0"})).is_err()); + assert!(s.doc().unwrap().wb.active().unwrap().validations.is_empty() && s.doc().unwrap().wb.active().unwrap().cond_formats.is_empty()); + // One link: `[1]` enters, `[2]` doesn't. + let link = gridcraft_model::ExternalBook { + sheets: vec![gridcraft_model::ExternalSheet { + name: "S".into(), + cells: vec![(CellRef::parse("A1").unwrap(), Value::Number(5.0))], + refresh_error: false, + }], + ..Default::default() + }; + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + wb.external_books = vec![link.clone()].into(); + refused(&mut s, "=[2]Sheet1!A1"); + refused(&mut s, "='[1]Jan:[1]Dec'!A1"); + // Only a defined name can follow a bare `[n]!`: a cell reference there would be to a sheet + // called `[1]`. + for input in ["=[1]!A1", "=[1]!$B$2", "=SUM([1]!A1:B2)", "=[0]!A1", "=[1]!#REF!"] { + refused(&mut s, input); + } + assert!(s.execute("formulas.defineName", json!({"name": "BookOnly", "refersTo": "=[1]!A1"})).is_err()); + assert!(s.execute("data.validation", json!({"range": "A1", "type": "custom", "formula1": "=[0]!A1>0"})).is_err()); + // A data bar's formula thresholds are conditional-format formulas too. + let bar = |f: &str| json!({"range": "A1:A3", "rule": {"type": "dataBar", "min": {"type": "formula", "value": f}}}); + assert!(s.execute("home.conditionalFormat", bar("[2]Sheet1!A1")).is_err()); + assert!(s.execute("home.conditionalFormat", bar("[1]S!A1")).is_err()); + assert!(s.doc().unwrap().wb.active().unwrap().cond_formats.is_empty()); + s.execute("home.conditionalFormat", bar("MIN(A1:A3)")).unwrap(); + s.execute("cell.set", json!({"cell": "B1", "input": "=[1]S!$A$1"})).unwrap(); + assert_eq!(v(&s, "B1"), Value::Number(5.0)); + // JSON doesn't carry the link parts, so it doesn't carry their values either: reopened, the + // workbook has no link, and a typed `[1]` is refused rather than saved without one. + let json_bytes = crate::io::save_bytes(&s.doc().unwrap().wb, "book.json").unwrap(); + let (back, _) = crate::io::open_bytes("book.json", &json_bytes).unwrap(); + assert!(back.external_books.is_empty()); + s.execute("formulas.defineName", json!({"name": "Linked", "refersTo": "=[1]Sheet1!A1"})).unwrap(); + // `[0]` is this workbook: entered as the plain reference, showing the local value. + s.execute("cell.set", json!({"cell": "C1", "input": "7"})).unwrap(); + s.execute("formulas.defineName", json!({"name": "Seven", "refersTo": "=Sheet1!$C$1"})).unwrap(); + for (cell, input, text) in + [("D1", "=[0]Sheet1!C1*2", "Sheet1!C1*2"), ("D2", "=SUM([0]Sheet1:Sheet1!C1)", "SUM(Sheet1:Sheet1!C1)"), ("D3", "=[0]!Seven*2", "Seven*2")] + { + s.execute("cell.set", json!({"cell": cell, "input": input})).unwrap(); + let d = s.doc().unwrap(); + let f = d.wb.active().unwrap().cell(CellRef::parse(cell).unwrap()).and_then(|c| c.formula.clone()).unwrap(); + assert_eq!(f.text, text, "{input}"); + } + assert_eq!((v(&s, "D1"), v(&s, "D2"), v(&s, "D3")), (Value::Number(14.0), Value::Number(7.0), Value::Number(14.0))); + // A `[0]` name is stored as the local reference, so it follows a sheet rename. + s.execute("formulas.defineName", json!({"name": "Zero", "refersTo": "=[0]Sheet1!$C$1"})).unwrap(); + assert_eq!(s.doc().unwrap().wb.names.iter().find(|n| n.name == "Zero").unwrap().formula, "Sheet1!$C$1"); + s.execute("cell.set", json!({"cell": "D4", "input": "=Zero"})).unwrap(); + s.execute("sheet.rename", json!({"sheet": 0, "name": "Data"})).unwrap(); + assert_eq!(v(&s, "D4"), Value::Number(7.0)); + // Pasted into a New Window of the same workbook, `[1]` is the same link. + s.execute("edit.copy", json!({"range": "B1"})).unwrap(); + s.execute("view.newWindow", json!({})).unwrap(); + s.execute("edit.paste", json!({"at": "E1"})).unwrap(); + assert_eq!(s.execute("cell.get", json!({"cell": "E1"})).unwrap()["formula"], "=[1]S!$A$1"); + assert_eq!(v(&s, "E1"), Value::Number(5.0)); + // In another workbook it's kept only where that workbook holds the same link at that index; + // otherwise `[1]` there would be a link it doesn't have (or a different workbook): `#REF!`. + let other = gridcraft_model::ExternalBook { + sheets: vec![gridcraft_model::ExternalSheet { name: "T".into(), ..Default::default() }], + ..Default::default() + }; + for (books, formula) in [(vec![], "=#REF!"), (vec![other], "=#REF!"), (vec![link], "=[1]S!$A$1")] { + s.new_workbook(); + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).external_books = books.into(); + s.execute("edit.paste", json!({"at": "E1"})).unwrap(); + assert_eq!(s.execute("cell.get", json!({"cell": "E1"})).unwrap()["formula"], formula); + } +} + +#[test] +fn kept_values_of_uncalculable_linked_names_move_with_their_cells() { + // As read from a file: `[1]!Broken` is defined as #REF! in the link, and the cell shows the 20 + // Excel saved for it. + let mut s = s(); + let names = vec![gridcraft_model::ExternalName { name: "Broken".into(), sheet: None, refers_to: "#REF!".into() }]; + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + wb.external_books = vec![gridcraft_model::ExternalBook { names, ..Default::default() }].into(); + let mut cell = gridcraft_model::Cell::formula(gridcraft_model::Formula::new("=[1]!Broken")); + cell.value = Value::Number(20.0); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse("H11").unwrap(), cell); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse("G11").unwrap(), gridcraft_model::Cell::value(Value::Number(2.0))); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse("G12").unwrap(), gridcraft_model::Cell::value(Value::Number(1.0))); + // Moved by cut and paste. + s.execute("edit.cut", json!({"range": "H11"})).unwrap(); + s.execute("edit.paste", json!({"at": "H15"})).unwrap(); + assert_eq!(v(&s, "H15"), Value::Number(20.0)); + // Moved by a sort, and by inserting rows above it. + s.execute("edit.cut", json!({"range": "H15"})).unwrap(); + s.execute("edit.paste", json!({"at": "H11"})).unwrap(); + s.execute("data.sort", json!({"range": "G11:H12", "header": false, "keys": [{"column": "G"}]})).unwrap(); + assert_eq!(v(&s, "H12"), Value::Number(20.0)); + s.execute("home.insertRows", json!({"rows": "1:2"})).unwrap(); + assert_eq!(v(&s, "H14"), Value::Number(20.0)); +} + +#[test] +fn kept_values_of_uncalculable_linked_names_survive_edits_they_do_not_read() { + // As read from a file: H11 `=[1]!Broken+G11`, where the link saved `Broken` as #REF!, shows + // the 22 Excel saved for it. + let mut s = s(); + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + let names = vec![gridcraft_model::ExternalName { name: "Broken".into(), sheet: None, refers_to: "#REF!".into() }]; + wb.external_books = vec![gridcraft_model::ExternalBook { names, ..Default::default() }].into(); + let mut cell = gridcraft_model::Cell::formula(gridcraft_model::Formula::new("=[1]!Broken+G11")); + cell.value = Value::Number(22.0); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse("H11").unwrap(), cell); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse("G11").unwrap(), gridcraft_model::Cell::value(Value::Number(2.0))); + // Edits that change nothing the formula reads keep its value. + s.execute("cell.set", json!({"cell": "K5", "input": "1"})).unwrap(); + assert_eq!(v(&s, "H11"), Value::Number(22.0)); + s.execute("home.rowHeight", json!({"rows": "30:30", "height": 30})).unwrap(); + assert_eq!(v(&s, "H11"), Value::Number(22.0)); + s.execute("formulas.defineName", json!({"name": "Other", "refersTo": "=1"})).unwrap(); + assert_eq!(v(&s, "H11"), Value::Number(22.0)); + // Changing what it reads calculates it, with the uncalculable name as #REF!. + s.execute("cell.set", json!({"cell": "G11", "input": "3"})).unwrap(); + assert_eq!(v(&s, "H11"), Value::Error(gridcraft_core::CellError::Ref)); +} + +#[test] +fn kept_values_give_way_when_a_table_resize_changes_what_they_read() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1:B3", "values": [["Name", "Qty"], ["a", 1], ["b", 2]]})).unwrap(); + s.execute("insert.table", json!({"range": "A1:B3", "header": true})).unwrap(); + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + wb.external_books = vec![gridcraft_model::ExternalBook { + names: vec![gridcraft_model::ExternalName { name: "Broken".into(), sheet: None, refers_to: "#REF!".into() }], + ..Default::default() + }] + .into(); + let mut linked = gridcraft_model::Cell::formula(gridcraft_model::Formula::new("=[1]!Broken+SUM(Table1[Qty])")); + linked.value = Value::Number(99.0); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse("D1").unwrap(), linked); + let mut unrelated = gridcraft_model::Cell::formula(gridcraft_model::Formula::new("=[1]!Broken+A10")); + unrelated.value = Value::Number(77.0); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse("E1").unwrap(), unrelated); + let d = s.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + d.calc.recalc_all(&mut wb); + d.wb = std::sync::Arc::new(wb); + + assert_eq!((v(&s, "D1"), v(&s, "E1")), (Value::Number(99.0), Value::Number(77.0))); + s.execute("table.resize", json!({"table": "Table1", "range": "A1:B4"})).unwrap(); + assert_eq!(v(&s, "D1"), Value::Error(gridcraft_core::CellError::Ref)); + assert_eq!(v(&s, "E1"), Value::Number(77.0)); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!((v(&s, "D1"), v(&s, "E1")), (Value::Number(99.0), Value::Number(77.0))); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!((v(&s, "D1"), v(&s, "E1")), (Value::Error(gridcraft_core::CellError::Ref), Value::Number(77.0))); +} + +#[test] +fn kept_values_of_uncalculable_linked_names_give_way_when_a_structural_edit_changes_what_they_read() { + // As read from a file, over a name the link saved as #REF!: H11 `=[1]!Broken+SUM(G10:G12)` + // shows 22 and K11 `=[1]!Broken+J11` shows 7, as Excel saved them. + let mut s = s(); + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + let names = vec![gridcraft_model::ExternalName { name: "Broken".into(), sheet: None, refers_to: "#REF!".into() }]; + wb.external_books = vec![gridcraft_model::ExternalBook { names, ..Default::default() }].into(); + for (at, text, saved) in [("H11", "=[1]!Broken+SUM(G10:G12)", 22.0), ("K11", "=[1]!Broken+J11", 7.0)] { + let mut cell = gridcraft_model::Cell::formula(gridcraft_model::Formula::new(text)); + cell.value = Value::Number(saved); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse(at).unwrap(), cell); + } + for (at, n) in [("G11", 2.0), ("J11", 1.0)] { + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse(at).unwrap(), gridcraft_model::Cell::value(Value::Number(n))); + } + // Moving the cells it reads changes nothing it reads: the values stay. + s.execute("home.insertRows", json!({"rows": "1:1"})).unwrap(); + assert_eq!((v(&s, "H12"), v(&s, "K12")), (Value::Number(22.0), Value::Number(7.0))); + // A deleted row inside its range, or a deleted cell it reads, calculates it again. + s.execute("home.deleteRows", json!({"rows": "13:13"})).unwrap(); + assert_eq!((v(&s, "H12"), v(&s, "K12")), (Value::Error(gridcraft_core::CellError::Ref), Value::Number(7.0))); + s.execute("home.deleteColumns", json!({"cols": "J:J"})).unwrap(); + assert_eq!(v(&s, "J12"), Value::Error(gridcraft_core::CellError::Ref)); +} + +/// A session as read from a file whose link saved the name `Broken` as #REF!: `cells` are +/// (cell, input, saved value of a formula), `names` (name, refers to). +fn kept_value_session(cells: &[(&str, &str, f64)], names: &[(&str, &str)]) -> Session { + let mut s = s(); + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + let linked = vec![gridcraft_model::ExternalName { name: "Broken".into(), sheet: None, refers_to: "#REF!".into() }]; + wb.external_books = vec![gridcraft_model::ExternalBook { names: linked, ..Default::default() }].into(); + for (name, refers_to) in names { + let formula = refers_to.to_string(); + wb.names.push(gridcraft_model::DefinedName { name: name.to_string(), scope: None, formula, comment: String::new(), hidden: false }); + } + for (at, input, saved) in cells { + let cell = if let Some(f) = input.strip_prefix('=') { + let mut cell = gridcraft_model::Cell::formula(gridcraft_model::Formula::new(f)); + cell.value = Value::Number(*saved); + cell + } else { + gridcraft_model::Cell::value(Value::Number(input.parse().unwrap())) + }; + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse(at).unwrap(), cell); + } + // Calculated as on loading: the formulas over `Broken` keep their saved values. + let d = s.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + d.calc.recalc_all(&mut wb); + d.wb = std::sync::Arc::new(wb); + s +} + +#[test] +fn kept_values_give_way_when_a_name_indirect_or_a_chain_through_a_name_reads_the_changed_cell() { + let mut s = kept_value_session( + &[ + ("A20", "5", 0.0), + ("D22", "=[1]!Broken+X", 25.0), + ("D23", "=[1]!Broken+INDIRECT(\"A20\")", 25.0), + ("B26", "=X", 5.0), + ("D26", "=[1]!Broken+B26", 25.0), + ("D27", "=[1]!Broken+Y", 9.0), + ], + &[("X", "Sheet1!$A$20"), ("Y", "Sheet1!$A$40")], + ); + s.execute("cell.set", json!({"cell": "A20", "input": "-1"})).unwrap(); + let r = Value::Error(gridcraft_core::CellError::Ref); + assert_eq!([v(&s, "D22"), v(&s, "D23"), v(&s, "B26"), v(&s, "D26")], [r.clone(), r.clone(), Value::Number(-1.0), r]); + // Y reads A40, which didn't change. + assert_eq!(v(&s, "D27"), Value::Number(9.0)); +} + +#[test] +fn kept_values_over_indirect_give_way_on_a_structural_edit_as_on_any_edit() { + // What INDIRECT reads isn't known before it runs: any edit calculates it, an inserted row too. + let mut s = kept_value_session(&[("A20", "5", 0.0), ("D23", "=[1]!Broken+INDIRECT(\"A20\")", 25.0), ("D24", "=[1]!Broken+A20", 25.0)], &[]); + s.execute("home.insertRows", json!({"rows": "1:1"})).unwrap(); + assert_eq!((v(&s, "D24"), v(&s, "D25")), (Value::Error(gridcraft_core::CellError::Ref), Value::Number(25.0))); +} + +#[test] +fn kept_values_give_way_when_a_name_they_use_is_redefined() { + let mut s = + kept_value_session(&[("A20", "5", 0.0), ("D22", "=[1]!Broken+X", 25.0), ("D23", "=[1]!Broken+Y", 9.0)], &[("X", "Sheet1!$A$20"), ("Y", "7")]); + s.execute("formulas.defineName", json!({"name": "X", "refersTo": "=7", "oldName": "X"})).unwrap(); + assert_eq!((v(&s, "D22"), v(&s, "D23")), (Value::Error(gridcraft_core::CellError::Ref), Value::Number(9.0))); +} + +#[test] +fn kept_values_give_way_when_a_deleted_row_changes_a_formula_or_a_name_they_read() { + let r = Value::Error(gridcraft_core::CellError::Ref); + // A deleted row turns B25 `=A20` into `=#REF!`: D24 reads it. Then one turns `Z` into #REF!. + let mut s = kept_value_session( + &[ + ("A20", "5", 0.0), + ("B25", "=A20", 5.0), + ("D25", "=[1]!Broken+B25", 25.0), + ("A50", "1", 0.0), + ("D30", "=[1]!Broken+Z", 9.0), + ("D31", "=[1]!Broken+B32", 4.0), + ], + &[("Z", "Sheet1!$A$50")], + ); + s.execute("home.deleteRows", json!({"rows": "20:20"})).unwrap(); + assert_eq!(v(&s, "D24"), r.clone()); + // D30 (now D29) `=[1]!Broken+Z` reads A49 now: moved, not changed. + assert_eq!(v(&s, "D29"), Value::Number(9.0)); + s.execute("home.deleteRows", json!({"rows": "49:49"})).unwrap(); + assert_eq!(v(&s, "D29"), r); + // D30 `=[1]!Broken+B31` read nothing that changed. + assert_eq!(v(&s, "D30"), Value::Number(4.0)); +} + #[test] fn undo_redo() { let mut s = s(); @@ -1613,6 +1915,161 @@ fn deleting_a_3d_span_end_sheet_keeps_the_rest() { assert_eq!(v(&s, "B1"), direct, "deleting {deleted}"); } } +#[test] +fn kept_values_give_way_when_a_middle_3d_sheet_is_deleted_or_moved() { + let setup = || { + let mut s = s(); + for name in ["Sheet2", "Sheet3", "Results"] { + s.execute("home.insertSheet", json!({"name": name})).unwrap(); + } + for (sheet, n) in [("Sheet1", 3), ("Sheet2", 5), ("Sheet3", 1)] { + s.execute("sheet.activate", json!({"sheet": sheet})).unwrap(); + s.execute("cell.set", json!({"cell": "B1", "input": n.to_string()})).unwrap(); + } + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + wb.external_books = vec![gridcraft_model::ExternalBook { + names: vec![gridcraft_model::ExternalName { name: "Broken".into(), sheet: None, refers_to: "#REF!".into() }], + ..Default::default() + }] + .into(); + let mut cell = gridcraft_model::Cell::formula(gridcraft_model::Formula::new("=[1]!Broken+SUM(Sheet1:Sheet3!B1)")); + cell.value = Value::Number(25.0); + wb.sheet_mut(3).unwrap().cells.set(CellRef::parse("D40").unwrap(), cell); + let d = s.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + d.calc.recalc_all(&mut wb); + d.wb = std::sync::Arc::new(wb); + s.execute("sheet.activate", json!({"sheet": "Results"})).unwrap(); + s + }; + + let mut deleted = setup(); + deleted.execute("home.deleteSheet", json!({"sheet": "Sheet2"})).unwrap(); + assert_eq!(v(&deleted, "D40"), Value::Error(gridcraft_core::CellError::Ref)); + + let mut moved = setup(); + moved.execute("sheet.move", json!({"sheet": "Sheet2", "to": 3})).unwrap(); + moved.execute("sheet.activate", json!({"sheet": "Results"})).unwrap(); + assert_eq!(v(&moved, "D40"), Value::Error(gridcraft_core::CellError::Ref)); +} +#[test] +fn kept_values_give_way_when_a_named_middle_3d_sheet_is_deleted_or_moved() { + let setup = || { + let mut s = s(); + for name in ["Sheet2", "Sheet3", "Results"] { + s.execute("home.insertSheet", json!({"name": name})).unwrap(); + } + for (sheet, n) in [("Sheet1", 3), ("Sheet2", 5), ("Sheet3", 1)] { + s.execute("sheet.activate", json!({"sheet": sheet})).unwrap(); + s.execute("cell.set", json!({"cell": "B1", "input": n.to_string()})).unwrap(); + } + s.execute("formulas.defineName", json!({"name": "W", "refersTo": "=Sheet1:Sheet3!$B$1"})).unwrap(); + s.execute("formulas.defineName", json!({"name": "WW", "refersTo": "=W"})).unwrap(); + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + wb.external_books = vec![gridcraft_model::ExternalBook { + names: vec![gridcraft_model::ExternalName { name: "Broken".into(), sheet: None, refers_to: "#REF!".into() }], + ..Default::default() + }] + .into(); + let mut direct = gridcraft_model::Cell::formula(gridcraft_model::Formula::new("=[1]!Broken+SUM(W)")); + direct.value = Value::Number(25.0); + wb.sheet_mut(3).unwrap().cells.set(CellRef::parse("D42").unwrap(), direct); + let mut chained = gridcraft_model::Cell::formula(gridcraft_model::Formula::new("=[1]!Broken+SUM(WW)")); + chained.value = Value::Number(25.0); + wb.sheet_mut(3).unwrap().cells.set(CellRef::parse("E42").unwrap(), chained); + let d = s.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + d.calc.recalc_all(&mut wb); + d.wb = std::sync::Arc::new(wb); + s.execute("sheet.activate", json!({"sheet": "Results"})).unwrap(); + s + }; + + let mut deleted = setup(); + deleted.execute("home.deleteSheet", json!({"sheet": "Sheet2"})).unwrap(); + assert_eq!( + (v(&deleted, "D42"), v(&deleted, "E42")), + (Value::Error(gridcraft_core::CellError::Ref), Value::Error(gridcraft_core::CellError::Ref)) + ); + deleted.execute("edit.undo", json!({})).unwrap(); + assert_eq!((v(&deleted, "D42"), v(&deleted, "E42")), (Value::Number(25.0), Value::Number(25.0))); + deleted.execute("edit.redo", json!({})).unwrap(); + assert_eq!( + (v(&deleted, "D42"), v(&deleted, "E42")), + (Value::Error(gridcraft_core::CellError::Ref), Value::Error(gridcraft_core::CellError::Ref)) + ); + + let mut moved = setup(); + moved.execute("sheet.move", json!({"sheet": "Sheet2", "to": 3})).unwrap(); + moved.execute("sheet.activate", json!({"sheet": "Results"})).unwrap(); + assert_eq!((v(&moved, "D42"), v(&moved, "E42")), (Value::Error(gridcraft_core::CellError::Ref), Value::Error(gridcraft_core::CellError::Ref))); + moved.execute("edit.undo", json!({})).unwrap(); + assert_eq!((v(&moved, "D42"), v(&moved, "E42")), (Value::Number(25.0), Value::Number(25.0))); + moved.execute("edit.redo", json!({})).unwrap(); + assert_eq!((v(&moved, "D42"), v(&moved, "E42")), (Value::Error(gridcraft_core::CellError::Ref), Value::Error(gridcraft_core::CellError::Ref))); +} + +#[test] +fn kept_values_follow_multi_area_structural_rewrites() { + let linked = || { + let mut s = s(); + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + wb.external_books = vec![gridcraft_model::ExternalBook { + names: vec![gridcraft_model::ExternalName { name: "Broken".into(), sheet: None, refers_to: "#REF!".into() }], + ..Default::default() + }] + .into(); + s + }; + let mut named = linked(); + named.execute("range.setValues", json!({"range": "A50:A52", "values": [[1], [1], [1]]})).unwrap(); + named.execute("formulas.defineName", json!({"name": "W", "refersTo": "=Sheet1!$A$50:$A$52"})).unwrap(); + let wb = std::sync::Arc::make_mut(&mut named.doc_mut().unwrap().wb); + let mut formula = gridcraft_model::Cell::formula(gridcraft_model::Formula::new("=[1]!Broken+SUM(W)")); + formula.value = Value::Number(25.0); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse("D42").unwrap(), formula); + let d = named.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + d.calc.recalc_all(&mut wb); + d.wb = std::sync::Arc::new(wb); + named.execute("home.deleteRows", json!({"rows": "10:10,51:51"})).unwrap(); + assert_eq!(v(&named, "D41"), Value::Error(gridcraft_core::CellError::Ref)); + + let mut chain = linked(); + chain.execute("cell.set", json!({"cell": "A61", "input": "1"})).unwrap(); + let wb = std::sync::Arc::make_mut(&mut chain.doc_mut().unwrap().wb); + for (at, text, saved) in [("D45", "=[1]!Broken+SUM(A60:A62)", 25.0), ("E45", "=[1]!Broken+D45", 50.0)] { + let mut cell = gridcraft_model::Cell::formula(gridcraft_model::Formula::new(text)); + cell.value = Value::Number(saved); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse(at).unwrap(), cell); + } + let d = chain.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + d.calc.recalc_all(&mut wb); + d.wb = std::sync::Arc::new(wb); + chain.execute("home.deleteRows", json!({"rows": "61:61,5:5"})).unwrap(); + assert_eq!((v(&chain, "D44"), v(&chain, "E44")), (Value::Error(gridcraft_core::CellError::Ref), Value::Error(gridcraft_core::CellError::Ref))); +} + +#[test] +fn kept_values_give_way_for_volatile_functions_after_unrelated_edits() { + let mut s = s(); + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + wb.external_books = vec![gridcraft_model::ExternalBook { + names: vec![gridcraft_model::ExternalName { name: "Broken".into(), sheet: None, refers_to: "#REF!".into() }], + ..Default::default() + }] + .into(); + let mut cell = gridcraft_model::Cell::formula(gridcraft_model::Formula::new("=[1]!Broken+NOW()*0")); + cell.value = Value::Number(25.0); + wb.sheet_mut(0).unwrap().cells.set(CellRef::parse("D46").unwrap(), cell); + let d = s.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + d.calc.recalc_all(&mut wb); + d.wb = std::sync::Arc::new(wb); + s.execute("cell.set", json!({"cell": "Z1", "input": "1"})).unwrap(); + assert_eq!(v(&s, "D46"), Value::Error(gridcraft_core::CellError::Ref)); +} #[test] fn xlsx_roundtrip_through_engine() { diff --git a/crates/formula/src/adjust.rs b/crates/formula/src/adjust.rs index d4881aa9e..d09b09f46 100644 --- a/crates/formula/src/adjust.rs +++ b/crates/formula/src/adjust.rs @@ -246,13 +246,14 @@ fn adjust_ref(r: &Reference, edit: &Edit) -> Option { } } -/// Renames sheet references `old` → `new`. +/// Renames sheet references `old` → `new`. A span into a linked workbook (`[1]Jan:Dec`) names +/// that workbook's sheets at both ends, so it never matches a sheet of this one. pub fn rename_sheet(e: Expr, old: &str, new: &str) -> Expr { e.map(&mut |x| match x { Expr::Ref(mut r) => { match &mut r.sheet { SheetSel::Named(n) if n.eq_ignore_ascii_case(old) => *n = new.to_string(), - SheetSel::Span(a, b) => { + SheetSel::Span(a, b) if !linked_span(a) => { if a.eq_ignore_ascii_case(old) { *a = new.to_string(); } @@ -282,6 +283,7 @@ pub fn delete_sheet(e: Expr, name: &str, order: &[String]) -> Expr { Expr::Ref(r) if r.sheet_name().is_some_and(|n| n.eq_ignore_ascii_case(name)) => Expr::Error(CellError::Ref), Expr::Ref(mut r) => { if let SheetSel::Span(a, b) = &r.sheet + && !linked_span(a) && (a.eq_ignore_ascii_case(name) || b.eq_ignore_ascii_case(name)) { let (Some(ia), Some(ib)) = (pos(a), pos(b)) else { return Expr::Ref(r) }; @@ -299,3 +301,9 @@ pub fn delete_sheet(e: Expr, name: &str, order: &[String]) -> Expr { other => other, }) } + +/// Whether a 3-D span whose first sheet is `first` is into a linked workbook (`[1]Jan:Dec`). +/// Excel forbids `[` in sheet names, so no sheet of this workbook starts with one. +fn linked_span(first: &str) -> bool { + first.starts_with('[') +} diff --git a/crates/formula/src/ast.rs b/crates/formula/src/ast.rs index 9b04140cc..da7e710df 100644 --- a/crates/formula/src/ast.rs +++ b/crates/formula/src/ast.rs @@ -39,6 +39,16 @@ pub enum SheetSel { Span(String, String), } +/// `[2]Prices` (a sheet in linked workbook 2) → `(2, "Prices")`; `[2]` alone (before a linked +/// workbook's name) → `(2, "")`. `None` when the name has no workbook index. +pub(crate) fn book_prefix(sheet: &str) -> Option<(usize, &str)> { + let (book, rest) = sheet.strip_prefix('[')?.split_once(']')?; + if book.is_empty() || !book.bytes().all(|b| b.is_ascii_digit()) { + return None; + } + Some((book.parse().ok()?, rest)) +} + #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct Reference { pub sheet: SheetSel, @@ -202,6 +212,9 @@ pub enum Expr { Invoke(Box, Vec), /// An omitted argument `F(1,,3)`. Missing, + /// A reference Excel found deleted in a linked workbook, kept with its link + /// (`[1]Data!#REF!`, `[1]Jan:Dec!#REF!`): `#REF!`, written back with the sheet it was in. + LinkedRefError(SheetSel), Paren(Box), } diff --git a/crates/formula/src/lexer.rs b/crates/formula/src/lexer.rs index 4c41d3609..4c8bcbf00 100644 --- a/crates/formula/src/lexer.rs +++ b/crates/formula/src/lexer.rs @@ -53,6 +53,35 @@ fn is_word_char(c: char) -> bool { c.is_alphanumeric() || matches!(c, '_' | '.' | '$' | '\\' | '?') } +/// `[1]Prices!`, `[1]Jan:Dec!` or `[1]!` (before a linked workbook's name) at `i`: the position of +/// the `!` after a linked workbook's index and unquoted sheet names, and of the `:` between the two +/// sheets of a sheet range (MS-XLSX `sheet-range = [workbook-index] sheet-name ":" sheet-name`). +fn external_sheet_end(chars: &[(usize, char)], i: usize) -> Option<(usize, Option)> { + let ch = |k: usize| chars.get(k).map(|c| c.1); + let word_end = |mut k: usize| { + while ch(k).is_some_and(is_word_char) { + k += 1; + } + k + }; + let mut j = i + 1; + while ch(j).is_some_and(|c| c.is_ascii_digit()) { + j += 1; + } + if j == i + 1 || ch(j) != Some(']') { + return None; + } + let k = word_end(j + 1); + match ch(k) { + Some('!') => Some((k, None)), + Some(':') if k > j + 1 => { + let m = word_end(k + 1); + (m > k + 1 && ch(m) == Some('!')).then_some((m, Some(k))) + } + _ => None, + } +} + /// `A1`, `$A$1`, `A`, `$A`, `1`, `$1`: a word that can end a range, so `A1.:` is a trim /// reference and not a name ending in a dot. fn is_range_end(w: &str) -> bool { @@ -170,6 +199,18 @@ pub fn tokenize_locale(src: &str, locale: crate::FormulaLocale) -> Result { + // Sheet (or sheet range) in a linked workbook, unquoted: `[1]Prices!B2`, + // `[1]Jan:Dec!B2` (`'[1]My Prices'!B2` is lexed as a quoted sheet name). + let (bang, colon) = external_sheet_end(&chars, i).unwrap_or((i, None)); + let text = |a: usize, b: usize| -> String { chars.get(a..b).map(|s| s.iter().map(|c| c.1).collect()).unwrap_or_default() }; + let tok = match colon { + Some(colon) => Tok::Sheet(text(i, colon), Some(text(colon + 1, bang))), + None => Tok::Sheet(text(i, bang), None), + }; + push(&mut out, tok, bang + 1); + i = bang + 1; + } '[' => { // Structured reference body: balanced brackets. let mut depth = 0; diff --git a/crates/formula/src/parser.rs b/crates/formula/src/parser.rs index 555717be9..b375f037f 100644 --- a/crates/formula/src/parser.rs +++ b/crates/formula/src/parser.rs @@ -294,13 +294,24 @@ impl Parser { Ok(Expr::Call(name, args)) } Tok::Sheet(a, b) => { + // `[0]` is this workbook (MS-XLSX): `[0]Sheet1!A1` is `Sheet1!A1`, so structural + // edits and evaluation see the same sheet. + let a = match book_prefix(&a) { + Some((0, rest)) if !rest.is_empty() => rest.to_string(), + _ => a, + }; let sheet = match b { Some(b) => SheetSel::Span(a, b), None => SheetSel::Named(a), }; match self.next() { Tok::Word(w) => self.reference_from_word(sheet, &w), - Tok::Error(CellError::Ref) => Ok(Expr::Error(CellError::Ref)), + // A reference Excel found deleted in a linked workbook keeps the link + // (`[1]Data!#REF!`), so it prints back as written. + Tok::Error(CellError::Ref) => Ok(match &sheet { + SheetSel::Named(s) | SheetSel::Span(s, _) if book_prefix(s).is_some_and(|(n, _)| n > 0) => Expr::LinkedRefError(sheet), + _ => Expr::Error(CellError::Ref), + }), t => Err(self.err(format!("expected a reference after the sheet name, found {}", describe(&t)))), } } @@ -374,6 +385,10 @@ impl Parser { if sheet != SheetSel::Current { // Sheet-scoped name `Sheet1!MyName`. if let SheetSel::Named(s) = sheet { + // `[0]!Rates`: this workbook's `Rates`. + if book_prefix(&s) == Some((0, "")) { + return Ok(Expr::Name(w.to_string())); + } return Ok(Expr::Name(format!("{s}!{w}"))); } return Err(self.err("invalid 3-D reference")); diff --git a/crates/formula/src/printer.rs b/crates/formula/src/printer.rs index 3975a7c61..89b144c51 100644 --- a/crates/formula/src/printer.rs +++ b/crates/formula/src/printer.rs @@ -8,6 +8,18 @@ use crate::ast::*; /// Whether a sheet name needs quotes in a reference. pub fn needs_quotes(name: &str) -> bool { + // A sheet in a linked workbook, `[1]Prices`: Excel quotes it only when the sheet name needs it. + // `[1]` alone (`[1]!Rate`, or `[1]!A1` read from a file) is never quoted: `'[1]'` would name a + // sheet called `[1]`. + match book_prefix(name) { + Some((_, "")) => false, + Some((_, sheet)) => sheet_needs_quotes(sheet), + None => sheet_needs_quotes(name), + } +} + +/// Whether a sheet name, without a linked workbook's `[n]`, needs quotes. +fn sheet_needs_quotes(name: &str) -> bool { if name.is_empty() { return true; } @@ -28,8 +40,9 @@ fn sheet_prefix(s: &SheetSel) -> String { SheetSel::Current => String::new(), SheetSel::Named(n) => format!("{}!", quote_sheet(n)), SheetSel::Span(a, b) => { + // `[1]Jan:Dec!A1`: a linked workbook's index can start the range, not its second sheet. let joined = format!("{a}:{b}"); - if needs_quotes(a) || needs_quotes(b) { format!("'{}'!", joined.replace('\'', "''")) } else { format!("{joined}!") } + if needs_quotes(a) || sheet_needs_quotes(b) { format!("'{}'!", joined.replace('\'', "''")) } else { format!("{joined}!") } } } } @@ -193,10 +206,16 @@ fn write_expr(s: &mut String, e: &Expr, r1c1: Option) { Some(at) => s.push_str(&reference_r1c1(r, at)), None => s.push_str(&reference_a1(r)), }, + Expr::LinkedRefError(sheet) => { + s.push_str(&sheet_prefix(sheet)); + s.push_str("#REF!"); + } Expr::Name(n) => match n.rsplit_once('!') { // A sheet-qualified name is stored with the bare sheet name (`My Sheet!Total`): quote it so it parses back. + // A linked workbook's own name (`[1]!Rate`, `[Source.xlsx]!Rate`) has no sheet to quote. Some((sheet, name)) => { - s.push_str("e_sheet(sheet)); + let book_only = sheet.strip_prefix('[').and_then(|b| b.strip_suffix(']')).is_some_and(|b| !b.is_empty() && !b.contains(['[', ']'])); + s.push_str(&if book_only { sheet.to_string() } else { quote_sheet(sheet) }); s.push('!'); s.push_str(name); } diff --git a/crates/formula/src/tests.rs b/crates/formula/src/tests.rs index d23eba0e2..2e273bd91 100644 --- a/crates/formula/src/tests.rs +++ b/crates/formula/src/tests.rs @@ -77,6 +77,91 @@ fn normalises() { assert_eq!(rt("'Sheet1'!A1"), "Sheet1!A1"); } +#[test] +fn linked_workbook_sheet_ranges_round_trip() { + // MS-XLSX `sheet-range = [workbook-index] sheet-name ":" sheet-name`, quoted or not. + let span = |a: &str, b: &str| SheetSel::Span(a.into(), b.into()); + for (src, sheet) in [ + ("SUM([1]Jan:Dec!A1)", span("[1]Jan", "Dec")), + ("SUM('[1]Jan:Dec'!A1)", span("[1]Jan", "Dec")), + ("SUM('[1]Jan:My Dec'!A1)", span("[1]Jan", "My Dec")), + ("SUM('Jan:[1]Dec'!A1)", span("Jan", "[1]Dec")), + ("[2]Prices!B2", SheetSel::Named("[2]Prices".into())), + ] { + let e = parse(src).unwrap_or_else(|e| panic!("{src}: {e}")); + let first = match &e { + Expr::Call(_, args) => args.first(), + r => Some(r), + }; + let Some(Expr::Ref(r)) = first else { panic!("{src}: {e:?}") }; + assert_eq!(r.sheet, sheet, "{src}"); + // What the printer writes reads back as the same formula. + let printed = print(&e); + assert_eq!(parse(&printed).ok(), Some(e), "{src} printed as {printed}"); + } +} + +#[test] +fn names_of_linked_workbooks_print_as_excel_writes_them() { + // `[1]!Rate` is the linked workbook's own name: Excel writes no quotes, and `'[1]'!Rate` + // would name a sheet called `[1]` in this workbook. A sheet of the link is quoted as needed. + for (src, printed) in [ + ("[1]!Rate", "[1]!Rate"), + ("SUM([1]!Rate,1)", "SUM([1]!Rate,1)"), + ("[1]Prices!Rate", "[1]Prices!Rate"), + ("'My Sheet'!Total", "'My Sheet'!Total"), + ] { + assert_eq!(print(&parse(src).unwrap()), printed, "{src}"); + } +} + +#[test] +fn book_only_references_print_back_without_quotes() { + // `[1]!A1` (only a defined name should follow `[1]!`, but files may hold it) must not become + // `'[1]'!A1`, a reference to a sheet called `[1]`. + for src in ["[1]!A1", "[1]!$B$2", "SUM([1]!A1:B2)", "[0]!A1"] { + assert_eq!(print(&parse(src).unwrap()), src); + } +} + +#[test] +fn deleted_references_into_linked_workbooks_keep_the_link() { + // Excel writes `[1]Data!#REF!` for a reference whose cells were deleted in the linked + // workbook; the link stays in the formula. + for src in ["[1]Data!#REF!+1", "SUM('[1]My Data'!#REF!)", "SUM([1]Jan:Dec!#REF!)"] { + assert_eq!(print(&parse(src).unwrap()), src); + } + assert_eq!(print(&parse("Data!#REF!+1").unwrap()), "#REF!+1"); +} + +#[test] +fn workbook_zero_is_this_workbook() { + // `[0]` (MS-XLSX) is the workbook itself: read as a local reference, it follows its sheet. + for (src, local) in [ + ("[0]Data!B2", "Data!B2"), + ("'[0]My Data'!B2", "'My Data'!B2"), + ("SUM([0]Jan:Dec!A1)", "SUM(Jan:Dec!A1)"), + ("[0]!Rate", "Rate"), + ("[0]Data!Rate", "Data!Rate"), + ("[00]Data!B2", "Data!B2"), + ] { + assert_eq!(parse(src).unwrap(), parse(local).unwrap(), "{src}"); + } + assert_eq!(print(&rename_sheet(parse("[0]Data!B2*2").unwrap(), "Data", "Local")), "Local!B2*2"); +} + +#[test] +fn renaming_or_deleting_a_sheet_leaves_linked_spans_alone() { + // `[1]Jan:Dec` is a span of the linked workbook's sheets; this workbook's `Dec` isn't in it. + let order = ["Jan".to_string(), "Dec".to_string()]; + for src in ["SUM([1]Jan:Dec!A1)+Dec!A1", "SUM('[1]Jan:Dec'!A1)+Dec!A1"] { + let renamed = print(&rename_sheet(parse(src).unwrap(), "Dec", "December")); + assert_eq!(renamed, "SUM([1]Jan:Dec!A1)+December!A1", "{src}"); + let deleted = print(&crate::adjust::delete_sheet(parse(src).unwrap(), "Dec", &order)); + assert_eq!(deleted, "SUM([1]Jan:Dec!A1)+#REF!", "{src}"); + } +} + #[test] fn spill_operator() { for f in ["A1#", "Sheet1!$D$1#", "SUM(B2#)", "'My Sheet'!A1#*2", "ROWS(A1#)+1", "IF(A1,#N/A,#REF!)"] { diff --git a/crates/model/src/cell.rs b/crates/model/src/cell.rs index 8700f9a28..0ddecaa68 100644 --- a/crates/model/src/cell.rs +++ b/crates/model/src/cell.rs @@ -17,6 +17,11 @@ pub struct Formula { /// Legacy array formula (Ctrl+Shift+Enter) covering this range; the formula lives in the /// top-left cell. pub array: Option, + /// Read from a file as a legacy formula (an XLSX `` without dynamic-array metadata), which + /// Excel reads with implicit intersection. Saving keeps a range into a linked workbook in it + /// from turning it into a dynamic array. Kept when the formula is moved, copied or adjusted. + #[serde(skip)] + pub legacy: bool, } impl Formula { @@ -25,12 +30,17 @@ impl Formula { pub fn new(text: &str) -> Formula { let body = text.strip_prefix('=').unwrap_or(text); match gridcraft_formula::parse(body) { - Ok(e) => Formula { text: gridcraft_formula::print(&e), expr: Some(Arc::new(e)), array: None }, - Err(_) => Formula { text: body.to_string(), expr: None, array: None }, + Ok(e) => Formula::from_expr(e), + Err(_) => Formula { text: body.to_string(), expr: None, array: None, legacy: false }, } } pub fn from_expr(e: Expr) -> Formula { - Formula { text: gridcraft_formula::print(&e), expr: Some(Arc::new(e)), array: None } + Formula { text: gridcraft_formula::print(&e), expr: Some(Arc::new(e)), array: None, legacy: false } + } + /// This formula with `e` as its expression (moved, copied or adjusted). It stays legacy if it + /// was; `array` is the caller's to set. + pub fn rewritten(&self, e: Expr) -> Formula { + Formula { legacy: self.legacy, ..Formula::from_expr(e) } } /// The parsed expression, re-parsing after deserialization. pub fn expr(&self) -> Option { diff --git a/crates/model/src/lib.rs b/crates/model/src/lib.rs index a76f30fc3..177bef006 100644 --- a/crates/model/src/lib.rs +++ b/crates/model/src/lib.rs @@ -25,7 +25,7 @@ pub use style::{ Alignment, BorderLine, BorderStyle, Borders, Color, Fill, Font, HAlign, NumFmt, PatternType, Style, StyleId, StyleTable, Theme, Underline, VAlign, VertAlign, }; -pub use workbook::{CalcMode, CalcSettings, DefinedName, DocProps, Workbook}; +pub use workbook::{CalcMode, CalcSettings, DefinedName, DocProps, ExternalBook, ExternalName, ExternalSheet, Workbook}; /// Font used for new workbooks. Only a name: renderers substitute an available open font /// (e.g. the metric-compatible Carlito) when it isn't installed. diff --git a/crates/model/src/workbook.rs b/crates/model/src/workbook.rs index 828ca692e..ce22eefb7 100644 --- a/crates/model/src/workbook.rs +++ b/crates/model/src/workbook.rs @@ -2,7 +2,7 @@ use std::sync::Arc; -use gridcraft_core::DateSystem; +use gridcraft_core::{CellRef, DateSystem, RangeRef, Value}; use gridcraft_formula::Expr; use serde::{Deserialize, Serialize}; @@ -85,6 +85,83 @@ pub struct DocProps { pub description: String, } +/// The values a linked workbook had when the file was last saved with the link updated +/// (`` `sheetDataSet`). `[n]` in a formula is the workbook's `external_books[n - 1]`. +/// GridCraft doesn't open linked files: references into them read these values, as Excel does +/// while the source is closed. +#[derive(Clone, Debug, Default, PartialEq)] +pub struct ExternalBook { + pub sheets: Vec, + /// The linked workbook's defined names (`[1]!Rates`, `[1]Sheet1!Rates`). + pub names: Vec, + /// The link isn't to a workbook (a DDE or OLE link), or its part is missing or unreadable. + /// What formulas read through it can't be calculated, apart from `items`: the cells keep the + /// values saved for them. + pub opaque: bool, + /// A DDE link's items with the values saved for them (`[1]!Price` reads item `Price`). An + /// item saved without values isn't here. + pub items: Vec<(String, Value)>, +} + +#[derive(Clone, Debug, Default, PartialEq)] +pub struct ExternalSheet { + pub name: String, + /// Cached cells, sorted by position (row, then column). + pub cells: Vec<(CellRef, Value)>, + /// The last update of the link couldn't read this sheet (`refreshError`): Excel keeps the + /// cells it had saved, and references to anything else in it are `#REF!`. + pub refresh_error: bool, +} + +#[derive(Clone, Debug, Default, PartialEq)] +pub struct ExternalName { + pub name: String, + /// Index into the book's `sheets` of the sheet the name is scoped to; `None` = workbook scope. + pub sheet: Option, + /// What the name refers to (without `=`); its sheets are the linked workbook's. Empty when + /// the file saved the name without a definition. + pub refers_to: String, +} + +impl ExternalBook { + pub fn sheet(&self, name: &str) -> Option<&ExternalSheet> { + self.sheets.get(self.sheet_index(name)?) + } + /// The position in `sheets` of the sheet named `name`. + pub fn sheet_index(&self, name: &str) -> Option { + self.sheets.iter().position(|s| s.name.eq_ignore_ascii_case(name)) + } + /// The name `name` as seen from sheet `sheet` (`None` = from the workbook): the sheet's own + /// name first, then the workbook-scoped one. + pub fn name(&self, name: &str, sheet: Option<&str>) -> Option<&ExternalName> { + let si = sheet.and_then(|s| self.sheet_index(s)); + let find = |scope: Option| self.names.iter().find(|n| n.sheet == scope && n.name.eq_ignore_ascii_case(name)); + si.and_then(|i| find(Some(i))).or_else(|| find(None)) + } + /// The value saved for DDE item `name` (`[1]!Price`). + pub fn item(&self, name: &str) -> Option<&Value> { + self.items.iter().find(|(n, _)| n.eq_ignore_ascii_case(name)).map(|(_, v)| v) + } +} + +impl ExternalSheet { + /// The cached value at `c` (blank when the link didn't store the cell). + pub fn value(&self, c: CellRef) -> Value { + self.cells.binary_search_by(|(p, _)| p.cmp(&c)).ok().and_then(|i| self.cells.get(i)).map(|(_, v)| v.clone()).unwrap_or_default() + } + /// The cached cells inside `range`, in order. + pub fn cells_in(&self, range: RangeRef) -> impl Iterator { + let from = self.cells.partition_point(|(p, _)| p.row < range.start.row); + self.cells.get(from..).unwrap_or_default().iter().take_while(move |(p, _)| p.row <= range.end.row).filter(move |(p, _)| range.contains(*p)) + } + /// The smallest range holding every cached cell. + pub fn extent(&self) -> Option { + let (first, last) = (self.cells.first()?.0, self.cells.last()?.0); + let (c0, c1) = self.cells.iter().fold((u32::MAX, 0), |(a, b), (p, _)| (a.min(p.col), b.max(p.col))); + Some(RangeRef::new(CellRef::new(first.row, c0), CellRef::new(last.row, c1))) + } +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Workbook { pub sheets: Vec>, @@ -120,6 +197,13 @@ pub struct Workbook { /// `preserved` are relative to. Saving into another folder rewrites them from there. Not in JSON. #[serde(skip)] pub source_dir: Option, + /// Linked workbooks' cached values, in `` order (`[1]` is the first). + /// Read from the file; the link parts themselves are kept for saving in `preserved`. Shared, + /// as sheets are: edits and undo history clone the workbook, never this cache. Not in JSON, + /// like the link parts: a workbook without them has no links, so a typed `[1]` is refused + /// rather than saved into a file that has no link for it. + #[serde(skip)] + pub external_books: Arc>, } impl Default for Workbook { @@ -148,6 +232,7 @@ impl Workbook { csv_delimiter: None, preserved: None, source_dir: None, + external_books: Arc::default(), } } pub fn sheet(&self, i: usize) -> Option<&Sheet> { @@ -219,7 +304,7 @@ impl Workbook { let f = cell.formula.as_ref()?; let e = f.expr()?; let ne = spell_calls(e.clone(), &lambdas); - (ne != e).then(|| (c, Formula { array: f.array, ..Formula::from_expr(ne) })) + (ne != e).then(|| (c, Formula { array: f.array, ..f.rewritten(ne) })) }) .collect(); if fixed.is_empty() { diff --git a/crates/xlsx/src/array_formula.rs b/crates/xlsx/src/array_formula.rs index caa5e8e9f..1535c0d73 100644 --- a/crates/xlsx/src/array_formula.rs +++ b/crates/xlsx/src/array_formula.rs @@ -21,7 +21,7 @@ impl Shape { } pub(crate) fn needs_array(wb: &Workbook, sheet: usize, at: CellRef, formula: &Formula) -> bool { - let Some(expr) = formula.expr_arc() else { return false }; + let Some(expr) = formula.expr_arc().filter(|_| !crate::external::saved_as_read(formula)) else { return false }; let mut scan = Scan { wb, at, remaining: 4096, locals: Vec::new() }; let shape = scan.expr(&expr, sheet, false, 0); // An explicit `@` is written as `_xlfn.SINGLE`, which only dynamic-array formulas carry. diff --git a/crates/xlsx/src/external.rs b/crates/xlsx/src/external.rs new file mode 100644 index 000000000..36e11461e --- /dev/null +++ b/crates/xlsx/src/external.rs @@ -0,0 +1,193 @@ +//! Cached values of linked workbooks (`xl/externalLinks/externalLinkN.xml`). +//! +//! A formula reaching into another workbook (`[1]Prices!B2`, `'[1]P&L'!A7:A99`) names it by its +//! position in ``. Excel stores the values it last read from each linked +//! workbook in the link part (``) and calculates with them while the +//! source is closed. The parts themselves are carried through unchanged by `passthrough`. + +use gridcraft_core::{CellError, CellRef, MAX_COLS, MAX_ROWS, Value}; +use gridcraft_model::{ExternalBook, ExternalName, ExternalSheet}; + +use crate::IoError; +use crate::package::Rel; +use crate::read::Ctx; +use crate::xml::{self, El}; + +/// One book per ``, in order. A link that isn't to a workbook (DDE, OLE), or +/// whose part is missing or can't be read, is an opaque book, so that the later ones keep their +/// positions and formulas through it keep their saved values; a DDE link's book holds the values +/// saved for its items. +pub(crate) fn read(cx: &mut Ctx<'_>, wb_xml: &El, wb_rels: &[Rel]) -> Result, IoError> { + let Some(refs) = wb_xml.child("externalReferences") else { return Ok(vec![]) }; + let mut books = vec![]; + for r in refs.kids("externalReference") { + let target = r.attr("id").and_then(|id| wb_rels.iter().find(|x| x.id == id)).map(|x| x.target.clone()); + let part = match target { + Some(t) => cx.optional_xml(&t)?, + None => None, + }; + let book = match part.as_ref() { + Some(x) => match (x.child("externalBook"), x.child("ddeLink")) { + (Some(b), _) => book(b), + (None, Some(d)) => ExternalBook { opaque: true, items: dde_items(d), ..Default::default() }, + (None, None) => ExternalBook { opaque: true, ..Default::default() }, + }, + None => ExternalBook { opaque: true, ..Default::default() }, + }; + books.push(book); + } + Ok(books) +} + +/// Whether a formula is saved as the legacy formula it was read as, whatever it calculates to: it +/// was read from a file without dynamic-array metadata (and isn't a Ctrl+Shift+Enter array, which +/// keeps its range), and reads a linked workbook (`[1]Data!B:B`, `[1]!Rates`). Excel reads such a +/// cell with implicit intersection; writing it as a dynamic array, because a linked range in it +/// spills here, would change what Excel shows. Formulas typed over a linked workbook are saved +/// like any other. +pub(crate) fn saved_as_read(formula: &gridcraft_model::Formula) -> bool { + use gridcraft_formula::{Expr, SheetSel}; + let Some(expr) = formula.expr_arc().filter(|_| formula.legacy && formula.array.is_none()) else { return false }; + let mut linked = false; + expr.walk(&mut |e| { + linked |= match e { + Expr::Ref(r) => matches!(&r.sheet, SheetSel::Named(s) | SheetSel::Span(s, _) if s.starts_with('[')), + Expr::Name(n) => n.starts_with('['), + Expr::LinkedRefError(_) => true, + _ => false, + } + }); + linked +} + +/// The items of a `` that were saved with values (``, row-major): one +/// value, or an array. +fn dde_items(x: &El) -> Vec<(String, Value)> { + let items = x.child("ddeItems").into_iter().flat_map(|i| i.kids("ddeItem")); + items + .filter_map(|item| { + let name = item.attr("name").map(xml::decode_xstring)?; + let values = item.child("values")?; + let data: Vec = values.kids("value").map(dde_value).collect(); + let (rows, cols) = (values.attr_u32("rows").unwrap_or(1) as usize, values.attr_u32("cols").unwrap_or(1) as usize); + let v = match data.as_slice() { + [one] if rows == 1 && cols == 1 => one.clone(), + _ => Value::from(gridcraft_core::Array::new(rows, cols, data)?), + }; + Some((name, v)) + }) + .collect() +} + +/// One saved DDE value (`…`, number by default). +fn dde_value(v: &El) -> Value { + let text = v.child("val").map(|x| x.text.as_str()).unwrap_or_default(); + match v.attr("t").unwrap_or("n") { + "nil" => Value::Empty, + "str" => Value::text(xml::decode_xstring(text)), + "b" => Value::Bool(matches!(text.trim(), "1" | "true" | "TRUE")), + "e" => Value::Error(CellError::parse(text.trim()).unwrap_or(CellError::Value)), + _ => match text.trim().parse::() { + Ok(n) if n.is_finite() => Value::Number(if n == 0.0 { 0.0 } else { n }), + _ => Value::Error(CellError::Value), + }, + } +} + +fn book(x: &El) -> ExternalBook { + let mut sheets: Vec = x + .child("sheetNames") + .map(|n| { + n.kids("sheetName") + .map(|s| ExternalSheet { name: s.attr("val").map(xml::decode_xstring).unwrap_or_default(), ..Default::default() }) + .collect() + }) + .unwrap_or_default(); + for data in x.child("sheetDataSet").into_iter().flat_map(|d| d.kids("sheetData")) { + let Some(sheet) = data.attr_u32("sheetId").and_then(|i| sheets.get_mut(i as usize)) else { continue }; + sheet.refresh_error = data.flag("refreshError", false); + for c in data.kids("row").flat_map(|r| r.kids("cell")) { + let Some(pos) = c.attr("r").and_then(CellRef::parse).filter(|p| p.row < MAX_ROWS && p.col < MAX_COLS) else { continue }; + let v = value(c); + if !v.is_empty() { + sheet.cells.push((pos, v)); + } + } + sheet.cells.sort_by_key(|(p, _)| *p); + sheet.cells.dedup_by_key(|(p, _)| *p); + } + let names = x + .child("definedNames") + .into_iter() + .flat_map(|d| d.kids("definedName")) + .filter_map(|d| { + // A name saved without `refersTo` is still defined: the cells that use it keep the + // values Excel saved for them. + let refers_to = d.attr("refersTo").unwrap_or_default(); + Some(ExternalName { + name: d.attr("name")?.to_string(), + sheet: d.attr_u32("sheetId").map(|i| i as usize), + refers_to: refers_to.strip_prefix('=').unwrap_or(refers_to).to_string(), + }) + }) + .collect(); + ExternalBook { sheets, names, ..Default::default() } +} + +fn value(c: &El) -> Value { + let Some(v) = c.child("v").map(|v| v.text.as_str()) else { return Value::Empty }; + match c.attr("t").unwrap_or("n") { + "str" | "s" | "inlineStr" => Value::text(xml::decode_xstring(v)), + "b" => Value::Bool(matches!(v.trim(), "1" | "true" | "TRUE")), + "e" => Value::Error(CellError::parse(v.trim()).unwrap_or(CellError::Value)), + _ => match v.trim().parse::() { + Ok(n) if n.is_finite() => Value::Number(if n == 0.0 { 0.0 } else { n }), + _ => Value::Empty, + }, + } +} + +#[cfg(test)] +mod tests { + #[test] + fn names_saved_without_a_definition_are_kept() { + // Excel writes `` for a linked name it couldn't read: still a name + // of the link, which formulas use with the values Excel saved for them. + let x = br##""##; + let book = super::book(&crate::xml::parse(x).unwrap()); + let names: Vec<_> = book.names.iter().map(|n| (n.name.as_str(), n.refers_to.as_str())).collect(); + assert_eq!(names, [("HelloWorld", ""), ("Broken", "#REF!"), ("Rate", "Personal!$A$1")]); + } + + #[test] + fn dde_ole_and_missing_links_are_opaque_and_keep_positions() { + use gridcraft_core::{Array, Value}; + const NS: &str = r#"xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main""#; + let dde = format!( + r#"85.33Enron1#N/A"# + ); + let ole = format!(r#""#); + let book = format!(r#""#); + let rel = |id: u32, n: u32| { + format!( + r#""# + ) + }; + let rels = [rel(11, 1), rel(12, 2), rel(13, 3), rel(14, 4)].concat(); + let refs = r#""#; + // Link 3's part is missing from the package. + let parts = [ + ("xl/externalLinks/externalLink1.xml", dde.as_str()), + ("xl/externalLinks/externalLink2.xml", ole.as_str()), + ("xl/externalLinks/externalLink4.xml", book.as_str()), + ]; + let (wb, _) = crate::read_xlsx(&crate::tests::minimal("", &parts, &rels, refs)).unwrap(); + let books = &wb.external_books; + assert_eq!(books.iter().map(|b| b.opaque).collect::>(), [true, true, true, false]); + let grid = Value::from(Array::new(1, 2, vec![Value::Number(1.0), Value::Error(gridcraft_core::CellError::NA)]).unwrap()); + assert_eq!(books[0].items, [("ENE".to_string(), Value::Number(85.33)), ("Name".into(), Value::text("Enron")), ("Grid".into(), grid)]); + assert_eq!(books[0].item("ene"), Some(&Value::Number(85.33))); + assert!(books[1].items.is_empty() && books[2].items.is_empty()); + assert_eq!(books[3].sheet_index("Data"), Some(0)); + } +} diff --git a/crates/xlsx/src/lib.rs b/crates/xlsx/src/lib.rs index 3300f97fe..d49ba16f6 100644 --- a/crates/xlsx/src/lib.rs +++ b/crates/xlsx/src/lib.rs @@ -18,6 +18,7 @@ mod chartex; mod crypto; mod csv; mod drawing; +mod external; mod feature_bags; mod fmla; mod ods; diff --git a/crates/xlsx/src/passthrough.rs b/crates/xlsx/src/passthrough.rs index dd881c1e9..134d7314c 100644 --- a/crates/xlsx/src/passthrough.rs +++ b/crates/xlsx/src/passthrough.rs @@ -428,7 +428,7 @@ impl KeptReader { } x.push_str(""); p.elements.push(PreservedXml { slot: "externalReferences".into(), xml: x }); - cx.warn("links to external workbooks were kept; they are not followed or refreshed"); + cx.warn("links to external workbooks were kept; they are not followed or refreshed (formulas use the values saved with them)"); } Err(why) => cx.warn(format!("links to external workbooks were dropped: {why}")), } diff --git a/crates/xlsx/src/read.rs b/crates/xlsx/src/read.rs index 1a479081c..bc20cc322 100644 --- a/crates/xlsx/src/read.rs +++ b/crates/xlsx/src/read.rs @@ -247,6 +247,8 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { } wb.active_sheet = file_to_model.get(active_tab).copied().flatten().unwrap_or(0); + wb.external_books = Arc::new(crate::external::read(&mut cx, &wb_xml, &wb_rels)?); + // Defined names. Chartex parts refer to their data through hidden `_xlchart.*` names. let mut chart_names: Vec<(String, String)> = vec![]; if let Some(dn) = wb_xml.child("definedNames") { diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs index 0194d2268..eb057901f 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -450,7 +450,7 @@ fn read_cell(cx: &mut Ctx<'_>, sheet: &mut Sheet, st: &mut RowState, c: &El, pos return; } let dynamic = c.attr("cm").is_some(); - let formula = c.child("f").and_then(|f| read_formula(cx, st, f, pos, dynamic)); + let formula = c.child("f").and_then(|f| read_formula(cx, st, f, pos, dynamic)).map(|f| Formula { legacy: !dynamic, ..f }); let value = if formula.is_none() && st.dynamic.iter().any(|r| r.contains(pos) && r.start != pos) { Value::Empty } else { value }; sheet.cells.set(pos, Cell { value, formula: formula.map(Arc::new), style }); } diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index 78e1afaae..7c050b439 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -91,16 +91,18 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( } } } - // Dynamic-array spills: anchor → range, and the spilled values to cache. + // Dynamic-array spills: anchor → range, and the spilled values to cache. A legacy formula + // over a linked workbook is saved as read, without its spill (`external::saved_as_read`). + let as_read = |a: &CellRef| sheet.cells.get(*a).and_then(|c| c.formula.as_ref()).is_some_and(|f| crate::external::saved_as_read(f)); let dynamic: BTreeMap = sheet .spill_ranges .iter() - .filter(|(a, r)| !r.is_single() && sheet.cells.get(**a).is_some_and(|c| c.formula.as_ref().is_some_and(|f| f.array.is_none()))) + .filter(|(a, r)| !r.is_single() && sheet.cells.get(**a).is_some_and(|c| c.formula.as_ref().is_some_and(|f| f.array.is_none())) && !as_read(a)) .map(|(a, r)| (*a, *r)) .collect(); let mut extra: BTreeMap = BTreeMap::new(); // Values inside spills and legacy array ranges. - for (a, r) in sheet.spill_ranges.iter().filter(|(_, r)| !r.is_single()) { + for (a, r) in sheet.spill_ranges.iter().filter(|(a, r)| !r.is_single() && !as_read(a)) { for c in r.iter().take(1_000_000) { if c != *a && let Some(v) = sheet.spill.get(&c) diff --git a/crates/xlsx/src/tests/array_export.rs b/crates/xlsx/src/tests/array_export.rs index b90b98668..60f00c047 100644 --- a/crates/xlsx/src/tests/array_export.rs +++ b/crates/xlsx/src/tests/array_export.rs @@ -409,3 +409,30 @@ fn implicit_intersection_metadata_distinguishes_table_rows_and_columns() { &[("Data[@Input]*2", false), ("SUM(Data[Input])", false), ("SUM(Data[Input]*2)", true), ("SQRT(Data[Input])", true)], ); } + +/// A legacy formula over a linked workbook (read without `cm`) is saved as it was read, even when +/// it spills here: Excel reads it with implicit intersection. One read as a dynamic array, or +/// typed, is saved by its shape, whether or not it spills. +#[test] +fn legacy_formulas_into_linked_workbooks_stay_legacy_even_when_they_spill() { + let body = r#"[1]Data!B:B14[1]Data!B2:B41"#; + let (mut wb, _) = read_xlsx(&super::minimal(body, &[], "", "")).unwrap(); + let sh = wb.sheet_mut(0).unwrap(); + // As calculated: both spill. + for (a, r) in [("D6", "D6:D11"), ("E6", "E6:E8")] { + sh.spill_ranges.insert(at(a), range(r)); + for c in range(r).iter().skip(1) { + sh.spill.insert(c, Value::Number(1.0)); + } + } + // Typed, blocked by a value under it: no spill. + sh.cells.set(at("F6"), formula("[1]Data!B2:B4", 0.0)); + sh.cells.set(at("F7"), Cell::value(Value::Number(1.0))); + let bytes = write_xlsx(&wb).unwrap(); + let sheet = Package::open(&bytes).unwrap().read_xml("xl/worksheets/sheet1.xml").unwrap().unwrap(); + assert_scalar(&sheet, "D6"); + assert_eq!(cell(&sheet, "D6").child("f").unwrap().text, "[1]Data!B:B"); + assert!(sheet.child("sheetData").unwrap().kids("row").flat_map(|r| r.kids("c")).all(|c| c.attr("r") != Some("D7")), "no spilled values"); + assert_dynamic(&sheet, "E6", "E6:E8"); + assert_eq!(cell(&sheet, "F6").attr("cm"), Some("1")); +} diff --git a/crates/xlsx/src/tests/passthrough_tests.rs b/crates/xlsx/src/tests/passthrough_tests.rs index d844daa05..ef63cf946 100644 --- a/crates/xlsx/src/tests/passthrough_tests.rs +++ b/crates/xlsx/src/tests/passthrough_tests.rs @@ -209,6 +209,10 @@ fn unmodelled_parts_survive_a_round_trip() { assert!(data.preserved.is_some(), "{:?}", rep.warnings); assert!(!rep.warnings.iter().any(|w| w.contains("dropped")), "{:?}", rep.warnings); assert_eq!(data.comments.len(), 1, "the note is modelled"); + // The values saved with the link are read, for formulas into the linked workbook. + let book = &wb.external_books; + assert_eq!(book.len(), 1); + assert_eq!(book[0].sheet("sheet1").map(|s| s.value(gridcraft_core::CellRef::new(0, 0))), Some(gridcraft_core::Value::Number(7.0))); let out = write_xlsx(&wb).unwrap(); let u = Unpacked::new(&out); diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 901802905..a813dc090 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -882,6 +882,37 @@ fn implicit_intersection_is_written_as_single() { assert_eq!(sh.cell(at("D3")).and_then(|c| c.formula.as_ref()).and_then(|f| f.array), None); } +/// Formulas into linked workbooks, read from a file, save back as Excel wrote them: legacy cells +/// stay legacy, a deleted reference keeps its link, `[1]!A1` gains no quotes, and `[0]` (this +/// workbook) reads as the local reference. +#[test] +fn formulas_into_linked_workbooks_save_back_as_written() { + let cell = |r: &str, f: &str| format!(r#"{f}0"#); + let body = format!( + "{}{}{}{}{}{}", + cell("A1", "[1]Data!$B:$B"), + cell("B1", "[1]Data!$B$2:$B$4*2"), + cell("C1", "[1]Data!#REF!+1"), + cell("D1", "[1]!A1"), + cell("E1", "[0]Data!B2"), + cell("F1", "SUM('[1]My Data'!B2:B4)"), + ); + let (wb, _) = read_xlsx(&super::minimal(&body, &[], "", "")).unwrap(); + let text = |a: &str| wb.sheet(0).unwrap().cell(at(a)).and_then(|c| c.formula.as_ref()).map(|f| f.text.clone()).unwrap(); + assert_eq!(text("E1"), "Data!B2"); + let (xml, _) = sheet1_xml(&write_xlsx(&wb).unwrap()); + for (r, f) in [ + ("A1", "[1]Data!$B:$B"), + ("B1", "[1]Data!$B$2:$B$4*2"), + ("C1", "[1]Data!#REF!+1"), + ("D1", "[1]!A1"), + ("E1", "Data!B2"), + ("F1", "SUM('[1]My Data'!B2:B4)"), + ] { + assert!(xml.contains(&format!("{f}")) || xml.contains(&format!("{f}")), "{r}: {xml}"); + } +} + #[test] fn threaded_comments_write_persons_and_linked_legacy_notes() { let wb = sample(); diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 158149fb3..0d82011ca 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** trivial (formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; previously trivial: XLSX chart sheets are kept on save (#727)) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** minor (formulas into linked workbooks calculate from the values saved with the link; previously trivial: formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests. Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h** @@ -86,6 +86,8 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| | 2026-10-14 | trivial | Formula parser: text longer than Excel's 8,192 characters, and postfix chains (`1%%%…`, `f(1)(1)…`) past the 256-level nesting limit, are parse errors (the cell keeps its formula text and cached value) instead of trees deep enough to overflow the stack when a hostile file is opened, saved or dropped | +| 2026-10-13 | trivial | Linked workbooks: a typed `[n]` that isn't one of the workbook's links is refused when a formula is entered in a cell or a defined name, as Excel does (conditional formats, including data-bar formula thresholds, and data validation refuse references to other workbooks; pasted into another workbook they become `#REF!`); a file saved with one doesn't open in Excel. Only a defined name can follow a bare `[n]!` (`=[1]!A1` is refused). `[0]` is this workbook: `=[0]Sheet1!A1` is entered as `=Sheet1!A1`, and `[0]` read from a file is the local reference, so it follows inserted rows and sheet renames. The link cache isn't saved in GridCraft JSON, like the link parts | +| 2026-10-13 | minor | XLSX: formulas into other workbooks (`[1]Prices!B2`, `'[2]P&L'!A1:B9`, `[1]!Rates`, names over them) calculate from the values saved with the link (`externalLink` `sheetDataSet` and `definedNames`), as Excel does while the source is closed; they were all `#REF!`. Sheets whose last refresh failed keep their saved cells (others `#REF!`, whole columns included); names inside a linked name are the linked workbook's; DDE items read the values saved with the link; a linked name the saved data can't calculate (`#REF!`, no definition, an OLE link or DDE item saved without values) keeps the cell's saved value until an edit changes what the formula reads (a cell, directly or through other formulas, local names and tables; a name it uses; a cell or part of a range a row, column or sheet delete removes; a table it reads changing shape; a sheet deleted from or moved out of a 3-D span it reads, directly or through a name; anything, for INDIRECT, OFFSET and volatile functions such as NOW and RAND), which recalculates it with that value as `#REF!`; the *IF(S) range arguments refuse linked-workbook values like Excel (`#VALUE!`); linked sheets print unquoted when the name allows, and `[1]Data!#REF!` keeps its link. Formulas over linked workbooks are saved as legacy or dynamic as they were read, even when they spill here. Over 4,224 real-world files: 37,681 fewer mismatching formulas (68,964 → 31,283), 78 fewer files mismatching | | 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) | | 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) | | 2026-10-12 | minor | `.xls` import reads the calculation settings (CalcMode, CalcIter, CalcCount, CalcDelta, CalcPrecision), so circular models calculate instead of showing `#CIRC!` (#724) |