Compare commits

..
Author SHA1 Message Date
Valentin Klein ee24d50ac8 Calc: overlapping spills resolve by anchor order, incremental matches full recalc (#2483)
## What

When two array formulas want the same cells, the one that already has its spill keeps them, whichever anchor comes first (this is what Excel does, measured on this PR by Harrison-Booth). The other formula shows `#SPILL!` until the winner is cleared, then it spills. When neither has a spill yet (a first calculation with no remembered layout), the anchor first in reading order (row, then column) wins, so the result never depends on evaluation order.

Before, an incremental recalculation let the existing spill keep its cells, while a full recalculation laid everything out from scratch and gave them to the earlier anchor. The two disagreed. A full recalculation now starts from the layout the sheet held, so both agree.

Also fixed: clearing a spilling formula did not unblock an array blocked by its old area when that array sat below or to the right of it.

## Why

Issue #2480: on the sales sample, `cell.set C14 =TRANSPOSE(B18:B24)` followed by `range.fill =D14:E18` over E13:F13 left F13 as `#SPILL!` in memory but `0` after a full recalc (the fuzzer, `command_sequence_fuzz` seed 0, hits this on #795). Now C14 keeps its cells, F13 is `#SPILL!`, and a full recalc gives the same.

Known limits: the xlsx reader does not restore spill layouts, so a workbook opened from a file is laid out by reading order only (Excel keeps the layout the file records); and when both anchors already held a spill and one grows into the other, reading order decides (Excel keeps the cells the other owns). The rule is documented on `Standing` in `crates/calc/src/recalc.rs`.

## Test

In `crates/engine/src/tests.rs`: the two-command sequence from the issue; both entry orders of an overlap, plus clearing the winner; and a no-history layout, where the earlier anchor wins in both entry orders. All compare cells with a full recalc.

## How verified

- `cargo fmt --check`, `cargo clippy -p gridcraft-calc -p gridcraft-engine --all-targets -- -D warnings`, `cargo test -p gridcraft-calc -p gridcraft-engine` (includes the fuzz test): all pass.

Fixes #2480
2026-11-15 17:27:51 +01:00
8 changed files with 400 additions and 29 deletions
Generated
-1
View File
@@ -2349,7 +2349,6 @@ dependencies = [
"craft-offcrypto",
"craft-opc",
"craft-xls",
"craft-xml",
"gridcraft-core",
"gridcraft-formula",
"gridcraft-functions",
-1
View File
@@ -40,7 +40,6 @@ craft-opc = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0
# turn them into workbooks.
craft-csv = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0bc842cbde430955825553ab7b976f4", version = "0.1.0" }
# Excel 97-2003 (.xls, BIFF8) workbooks; `crates/xlsx/src/xls.rs` maps them onto the model.
craft-xml = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0bc842cbde430955825553ab7b976f4", version = "0.1.0" }
craft-xls = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0bc842cbde430955825553ab7b976f4", version = "0.1.0" }
# foundation
+119 -4
View File
@@ -48,6 +48,8 @@ pub struct Calc {
/// Cells whose formulas have errors from cycles.
pub circular: Vec<Key>,
pub last_recalc_cells: usize,
/// Which anchors held a spill when the current calculation started (see [`Standing`]).
standing: Standing,
/// The cells whose value is `#SPILL!`, kept so that finding the formulas an emptied cell or a
/// freed spill area may unblock doesn't walk every formula or window of cells. Filled in
/// `rebuild`, kept up to date by `cells_changed` and by the write-back of a run; an entry is
@@ -55,6 +57,45 @@ pub struct Calc {
spill_errors: SpillErrors,
}
/// Who gets the cells when two arrays want the same ones. Excel (measured by a reviewer on
/// PR #2483) lets the spill that already exists keep its cells, whichever anchor comes first;
/// the other formula shows `#SPILL!`, and clearing the winner lets it spill. The rule here:
///
/// 1. an anchor that held a spill before this calculation outranks one that did not;
/// 2. otherwise the anchor first in reading order (row, then column) wins.
///
/// Rule 2 alone decides when nothing is remembered, so the result never depends on hash order
/// or evaluation order. A full recalculation remembers the layout the sheet held before it
/// started (rule 1), so it agrees with the incremental one; a workbook that is calculated for
/// the first time has no such layout (rule 2 only).
///
/// Known limits: (a) the xlsx reader does not restore spill layouts, so a workbook opened from a
/// file is laid out by rule 2 alone, although Excel keeps the layout the file records; (b) when
/// both anchors held a spill and one grows into the other, rule 2 decides, where Excel keeps
/// the cells the other already owns.
#[derive(Clone, Debug, Default)]
struct Standing {
/// `true`: `keys` are the anchors that held a spill (full recalculation, which lays every
/// spill out afresh). `false`: `keys` are the formulas that held none (incremental).
held: bool,
keys: HashSet<Key>,
}
impl Standing {
fn had_spill(&self, k: Key) -> bool {
self.keys.contains(&k) == self.held
}
/// Whether `a` takes cells from `b` when both want them.
fn outranks(&self, a: Key, b: Key) -> bool {
match (self.had_spill(a), self.had_spill(b)) {
(true, false) => true,
(false, true) => false,
_ => (a.1.row, a.1.col) < (b.1.row, b.1.col),
}
}
}
/// A set of cells by row, each row's columns in their own set: a window of rows and columns is
/// found by one column range per row that has an entry, so the cost follows the cells in the
/// window, not those to the side of it in the same rows.
@@ -101,6 +142,7 @@ impl Default for Calc {
fixed_now: None,
circular: vec![],
last_recalc_cells: 0,
standing: Standing::default(),
spill_errors: SpillErrors::default(),
}
}
@@ -269,6 +311,14 @@ impl Calc {
/// Recalculates every formula (F9 / Ctrl+Alt+F9, and after loading).
pub fn recalc_all(&mut self, wb: &mut Workbook) {
self.rebuild(wb);
// The layout the sheets hold now decides who keeps cells two arrays both want.
// (A Python cell that holds a spill counts too, as it does in the incremental pass.)
let mut held = HashSet::default();
for i in 0..wb.sheets.len() {
if let Some(sh) = wb.sheet(i) {
held.extend(sh.spill_ranges.keys().filter(|a| sh.cell(**a).is_some_and(|c| c.formula.is_some() || c.py.is_some())).map(|a| (i, *a)));
}
}
// 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() {
@@ -280,7 +330,9 @@ impl Calc {
}
}
let all: Vec<Key> = self.graph.nodes.keys().copied().collect();
self.standing = Standing { held: true, keys: held };
self.run(wb, all, true);
self.standing = Standing::default();
}
/// Updates the graph for cells whose content changed and recalculates what depends on them.
@@ -391,6 +443,13 @@ impl Calc {
}
}
}
// So can the area a removed formula spilled over.
for (si, r) in &cleared {
match emptied.iter_mut().find(|x| x.0 == *si) {
Some(x) => x.1 = CellRef::new(x.1.row.max(r.end.row), x.1.col.max(r.end.col)),
None => emptied.push((*si, r.end)),
}
}
// An emptied cell can unblock an anchor any distance above or to the left of it.
for (si, end) in emptied {
seeds.extend(self.unblock_candidates(wb, si, end));
@@ -472,10 +531,22 @@ impl Calc {
fn run(&mut self, wb: &mut Workbook, mut dirty: Vec<Key>, full: bool) {
let mut dynamic_again = !self.graph.dynamic.is_empty();
self.last_recalc_cells = 0;
let mut seen: HashSet<Key> = HashSet::default();
if !full {
self.standing = Standing::default();
}
for _round in 0..8 {
if dirty.is_empty() {
break;
}
if !full {
// What each formula held before this calculation touched it.
for &k in &dirty {
if seen.insert(k) && wb.sheet(k.0).is_some_and(|s| !s.spill_ranges.contains_key(&k.1)) {
self.standing.keys.insert(k);
}
}
}
// Row-major order is a good topological guess; on-demand evaluation fixes the rest.
dirty.sort_by_key(|(s, c)| (*s, c.row, c.col));
let mut exprs: HashMap<Key, std::sync::Arc<Expr>> = HashMap::with_capacity_and_hasher(dirty.len(), Default::default());
@@ -493,6 +564,7 @@ impl Calc {
let tp = prof_now();
let (results, spills, cycles) = {
let mut host = PassHost::new(wb, &exprs, &mut rng, now).with_python(python);
host.standing = Some(&self.standing);
host.iterate = wb.calc.iterative;
for k in &dirty {
host.calculate(*k);
@@ -591,7 +663,40 @@ impl Calc {
if let Some(array) = new {
let end = CellRef::new(key.1.row.saturating_add(array.rows as u32 - 1), key.1.col.saturating_add(array.cols as u32 - 1));
let range = RangeRef::new(key.1, end);
let blocked = spill_is_blocked(sheet, key.1, range, end);
// Two arrays that want the same cells (see [`Standing`]): a spill already laid out
// that this anchor outranks is taken over, and recalculated.
let standing = &self.standing;
// A spill can only be in the way where it has cells besides its anchor, and an
// anchor itself blocks the array. When no spilled cell lies in the range, the owners
// are not scanned: a full recalculation of many disjoint spills stays linear.
let outranked: Vec<(CellRef, RangeRef)> = if range.iter().any(|cell| cell != key.1 && sheet.spill.contains_key(&cell)) {
#[cfg(test)]
OWNER_SCANS.with(|n| n.set(n.get() + 1));
sheet
.spill_ranges
.iter()
.filter(|(o, r)| **o != key.1 && r.intersects(&range) && standing.outranks(*key, (key.0, **o)))
.map(|(o, r)| (*o, *r))
.collect()
} else {
Vec::new()
};
let blocked = spill_is_blocked(sheet, *key, range, end, standing);
if !blocked {
for (o, r) in outranked {
for c in r.iter() {
sheet.spill.remove(&c);
}
sheet.spill_ranges.remove(&o);
if let Some(cell) = sheet.cells.get_mut(o) {
cell.value = Value::Error(CellError::Spill);
self.spill_errors.insert((key.0, o));
}
spill_changes.push((key.0, o));
self.graph.dependents_of_range(key.0, r, &mut spill_changes);
freed.push((key.0, r));
}
}
if blocked {
if let Some(cell) = sheet.cells.get_mut(key.1) {
cell.value = Value::Error(CellError::Spill);
@@ -628,12 +733,22 @@ impl Calc {
}
}
fn spill_is_blocked(sheet: &gridcraft_model::Sheet, anchor: CellRef, range: RangeRef, end: CellRef) -> bool {
// Owner scans done by `write_back`, to check that disjoint spills never walk every owner.
#[cfg(test)]
thread_local! {
pub(crate) static OWNER_SCANS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
/// Whether the array at `anchor` can't be laid out over `range`. A spill already laid out that
/// `anchor` outranks does not block it: it is taken over when written back.
fn spill_is_blocked(sheet: &gridcraft_model::Sheet, anchor: Key, range: RangeRef, end: CellRef, standing: &Standing) -> bool {
let outranked =
|cell: CellRef| sheet.spill_ranges.iter().any(|(o, r)| *o != anchor.1 && r.contains(cell) && standing.outranks(anchor, (anchor.0, *o)));
end.row >= gridcraft_core::MAX_ROWS
|| end.col >= gridcraft_core::MAX_COLS
|| range.iter().any(|cell| cell != anchor && (sheet.cells.has(cell) || sheet.spill.contains_key(&cell)))
|| range.iter().any(|cell| cell != anchor.1 && (sheet.cells.has(cell) || (sheet.spill.contains_key(&cell) && !outranked(cell))))
|| sheet.merges.iter().any(|merge| merge.intersects(&range))
|| sheet.tables.iter().any(|table| table.range.contains(anchor))
|| sheet.tables.iter().any(|table| table.range.contains(anchor.1))
}
fn fill_spill(sheet: &mut gridcraft_model::Sheet, anchor: CellRef, range: RangeRef, array: &Array) {
+13 -3
View File
@@ -31,6 +31,8 @@ pub(super) struct PassHost<'a> {
/// every row, so once per sheet rather than once per range read.
used: HashMap<usize, Option<RangeRef>>,
rng: &'a mut u64,
/// Who held a spill before the calculation; unset: every formula did (reading order decides).
pub(super) standing: Option<&'a super::Standing>,
now: f64,
cycle: bool,
pub(super) cycles: Vec<Key>,
@@ -99,6 +101,7 @@ impl<'a> PassHost<'a> {
spill_index: SpillIndex::default(),
used: HashMap::default(),
rng,
standing: None,
now,
cycle: false,
cycles: vec![],
@@ -160,19 +163,26 @@ fn spill_hits_overlap(index: &SpillIndex, sheet: usize, hits: &[CellRef], range:
}
/// Whether an array result at `k` can't spill: it is in a table, or its area runs off the sheet,
/// overlaps merged cells, holds other cells, or another formula's spill (unless that formula is
/// recalculated in this pass, and so may spill elsewhere now).
/// overlaps merged cells, holds other cells, or the spill of another formula that outranks it
/// (see `Standing`; unless that formula is recalculated in this pass, and so may spill elsewhere
/// now). A spill it outranks gives way to it when written back.
fn spill_blocked(host: &PassHost<'_>, (sheet, anchor): Key, a: &Array) -> bool {
let Some(sh) = host.wb.sheet(sheet) else { return false };
let end =
CellRef::new(anchor.row.saturating_add((a.rows as u32).saturating_sub(1)), anchor.col.saturating_add((a.cols as u32).saturating_sub(1)));
let range = RangeRef::new(anchor, end);
let own = sh.spill_ranges.get(&anchor);
let default = super::Standing::default();
let standing = host.standing.unwrap_or(&default);
let spilled_over = |c: CellRef| {
!own.is_some_and(|o| o.contains(c))
&& sh.spill_ranges.iter().any(|(o, r)| {
// A formula or Python cell recalculated in this pass lays its array out again.
*o != anchor && r.contains(c) && !host.exprs.contains_key(&(sheet, *o)) && !host.python.contains_key(&(sheet, *o))
*o != anchor
&& r.contains(c)
&& !host.exprs.contains_key(&(sheet, *o))
&& !host.python.contains_key(&(sheet, *o))
&& standing.outranks((sheet, *o), (sheet, anchor))
})
};
end.row >= gridcraft_core::MAX_ROWS
+99
View File
@@ -2960,6 +2960,105 @@ fn formulas_read_a_dirty_python_cell_after_it_is_laid_out() {
assert_eq!((t.num("A1"), t.num("A2"), t.num("A3")), (5.0, 100.0, 111.0));
}
#[test]
fn disjoint_spills_do_not_scan_every_spill_owner() {
// Many independent two-cell arrays: write-back must not walk all earlier owners for each one.
let mut t = T::new();
for i in 1..=2000 {
t.wb.sheet_mut(0).unwrap().cells.set(c(&format!("A{}", i * 2 - 1)), Cell::formula(Formula::new("=SEQUENCE(2)")));
}
crate::recalc::OWNER_SCANS.with(|n| n.set(0));
t.calc.recalc_all(&mut t.wb);
assert_eq!((t.num("A2"), t.num("A4000")), (2.0, 2.0));
let scans = crate::recalc::OWNER_SCANS.with(|n| n.get());
assert!(scans < 10, "{scans} owner scans for 2000 disjoint spills");
}
#[test]
fn overlapping_spill_edit_sequences_match_a_full_recalc() {
// Random short edit sequences on a small grid of arrays that run into each other (sizes from
// shared cells, so one edit resizes several at once), with constants and clears in their way.
// After every edit the incremental result must equal a full recalc of the same workbook.
let seeds: u64 = std::env::var("GRIDCRAFT_SPILL_SEEDS").ok().and_then(|v| v.parse().ok()).unwrap_or(200);
let steps: usize = std::env::var("GRIDCRAFT_SPILL_STEPS").ok().and_then(|v| v.parse().ok()).unwrap_or(20);
let only: Option<u64> = std::env::var("GRIDCRAFT_SPILL_SEED").ok().and_then(|v| v.parse().ok());
let next = |state: &mut u64| {
*state = state.wrapping_add(0x9E37_79B9_7F4A_7C15);
let mut z = *state;
z = (z ^ (z >> 30)).wrapping_mul(0xBF58_476D_1CE4_E5B9);
z = (z ^ (z >> 27)).wrapping_mul(0x94D0_49BB_1331_11EB);
z ^ (z >> 31)
};
let seeds: Vec<u64> = match only {
Some(s) => vec![s],
None => (0..seeds).collect(),
};
for seed in seeds {
let mut state = seed;
let mut t = T::new();
// Sizes in J1:J4.
for (i, n) in [2, 3, 1, 4].iter().enumerate() {
t.set(&format!("J{}", i + 1), &n.to_string());
}
let mut log: Vec<(String, String)> = Vec::new();
for step in 0..steps {
let r = next(&mut state);
let at = format!("{}{}", (b'A' + (r % 6) as u8) as char, 1 + (r >> 8) % 6);
let kind = (r >> 16) % 14;
let (at, input) = match kind {
0..=3 => {
let a = 1 + (r >> 24) % 4;
let b = 1 + (r >> 28) % 4;
(at, format!("=SEQUENCE($J${a},$J${b})"))
}
4 => (at, format!("=SEQUENCE({},{})", 1 + (r >> 24) % 3, 1 + (r >> 28) % 3)),
5 => (format!("J{}", 1 + (r >> 24) % 4), (1 + (r >> 28) % 4).to_string()),
6 => (at, "x".to_string()),
7 | 8 => (at, String::new()),
9 => (format!("K{}", 1 + (r >> 24) % 6), "=SUM(A1:F6)".to_string()),
10 => {
// An earlier edit again (often putting back what a cell held before, as undo does).
match log.len() {
0 => (at, String::new()),
n => log[(r >> 24) as usize % n].clone(),
}
}
11 => (format!("{}{}", (b'A' + ((r >> 24) % 6) as u8) as char, 1 + (r >> 32) % 6), (1 + (r >> 40) % 3).to_string()),
// Several cells in one edit (a fill, or clearing a range): a 2x2 block from `at`.
12 => (format!("{at}+"), String::new()),
_ => (format!("{at}+"), format!("=SEQUENCE($J${},$J${})", 1 + (r >> 24) % 4, 1 + (r >> 28) % 4)),
};
log.push((at.clone(), input.clone()));
if let Some(from) = at.strip_suffix('+') {
let from = c(from);
let keys: Vec<(usize, CellRef)> =
(0..2).flat_map(|dr| (0..2).map(move |dc| (0, CellRef { row: from.row + dr, col: from.col + dc }))).collect();
for k in &keys {
let cell = if input.is_empty() { Cell::value(Value::Empty) } else { Cell::formula(Formula::new(&input)) };
t.wb.sheet_mut(0).unwrap().cells.set(k.1, cell);
}
t.calc.cells_changed(&mut t.wb, &keys);
} else {
t.set(&at, &input);
}
let mut fresh = t.wb.clone();
Calc::new().recalc_all(&mut fresh);
let (a, b) = (t.wb.sheet(0).unwrap(), fresh.sheet(0).unwrap());
for row in 0..12 {
for col in 0..12 {
let cell = CellRef { row, col };
assert_eq!(a.value(cell), b.value(cell), "seed {seed} step {step}: {cell} incremental vs full recalc after {log:?}");
}
}
let mut ka: Vec<_> = a.spill_ranges.iter().map(|(k, v)| (*k, *v)).collect();
let mut kb: Vec<_> = b.spill_ranges.iter().map(|(k, v)| (*k, *v)).collect();
ka.sort_by_key(|(k, _)| (k.row, k.col));
kb.sort_by_key(|(k, _)| (k.row, k.col));
assert_eq!(ka, kb, "seed {seed} step {step}: spill layout after {log:?}");
}
}
}
/// A computed criterion whose formula runs a database function over the same criteria counts as
/// not met inside itself: before, each record's check ran the function again for every record, at
/// every level, so the work grew exponentially with the nesting allowed.
+121
View File
@@ -2940,6 +2940,127 @@ fn validation_formulas_are_relative_and_see_the_new_value() {
assert!(!ok(&mut s, "C1", "50"));
}
/// Every value of the active sheet as the incremental recalculation left it must equal a full one.
fn assert_matches_full_recalc(s: &Session) {
let wb = &s.doc().unwrap().wb;
let mut fresh = (**wb).clone();
gridcraft_calc::Calc::new().recalc_all(&mut fresh);
let (a, b) = (wb.active().unwrap(), fresh.active().unwrap());
for row in 0..30 {
for col in 0..12 {
let cell = CellRef { row, col };
assert_eq!(a.value(cell), b.value(cell), "{cell} in memory vs after a full recalc");
}
}
}
#[test]
fn filling_a_formula_next_to_a_spill_matches_a_full_recalc() {
// Found by the command-sequence fuzzer: C14 spills along row 14; filling `=D14:E18` over
// E13:F13 puts two array formulas whose areas run into that spill and into each other. The
// incremental pass left F13 as #SPILL! while a full recalc gave F13 its array and C14
// #SPILL!. The spill that already exists keeps its cells (as in Excel), in both.
let mut s = s();
s.execute("range.setValues", json!({"range": "B18", "values": [[1], [2], [3], [4], [5], [6], [7]]})).unwrap();
s.execute("cell.set", json!({"cell": "C14", "input": "=TRANSPOSE(B18:B24)"})).unwrap();
s.execute("selection.set", json!({"range": "E13"})).unwrap();
s.execute("range.fill", json!({"range": "E13:F13", "input": "=D14:E18"})).unwrap();
assert_eq!(v(&s, "C14"), Value::Number(1.0));
assert_eq!(v(&s, "I14"), Value::Number(7.0));
assert_eq!(v(&s, "F13"), Value::Error(gridcraft_core::CellError::Spill));
assert_matches_full_recalc(&s);
}
#[test]
fn overlapping_spills_keep_the_one_that_exists_whichever_anchor_is_first() {
use gridcraft_core::CellError::Spill;
// E10 spills along row 10, F8 down over F10: F8 is first in reading order, E10 may be first
// to spill. Excel keeps whichever spilled first.
for (first, second) in [(("E10", "=SEQUENCE(1,3)"), ("F8", "=SEQUENCE(5)")), (("F8", "=SEQUENCE(5)"), ("E10", "=SEQUENCE(1,3)"))] {
let mut s = s();
s.execute("cell.set", json!({"cell": first.0, "input": first.1})).unwrap();
s.execute("cell.set", json!({"cell": second.0, "input": second.1})).unwrap();
assert_ne!(v(&s, first.0), Value::Error(Spill), "{} keeps its cells", first.0);
assert_eq!(v(&s, second.0), Value::Error(Spill), "{} is blocked", second.0);
assert_matches_full_recalc(&s);
// Clearing the winner lets the other one spill.
s.execute("edit.clearContents", json!({"range": first.0})).unwrap();
assert_ne!(v(&s, second.0), Value::Error(Spill), "{} spills once {} is cleared", second.0, first.0);
assert_matches_full_recalc(&s);
}
}
#[test]
fn a_spill_growing_into_another_and_back_matches_a_full_recalc_at_every_step() {
use gridcraft_core::CellError::Spill;
// Both arrays already spill; F8 (first in reading order) grows down over E10's spill, takes
// its cells, shrinks back and lets it spill again, then the steps are undone, redone, edited
// inside the contested area and cleared. Every state must match a full recalc.
let mut s = s();
s.execute("cell.set", json!({"cell": "H1", "input": "1"})).unwrap();
s.execute("cell.set", json!({"cell": "F8", "input": "=SEQUENCE(H1)"})).unwrap();
s.execute("cell.set", json!({"cell": "E10", "input": "=SEQUENCE(1,3)*10"})).unwrap();
assert_eq!(v(&s, "G10"), Value::Number(30.0));
assert_matches_full_recalc(&s);
let steps: [(&str, serde_json::Value); 9] = [
("cell.set", json!({"cell": "H1", "input": "5"})),
("cell.set", json!({"cell": "E10", "input": "=SEQUENCE(1,2)*10"})),
("cell.set", json!({"cell": "H1", "input": "1"})),
("edit.undo", json!({})),
("edit.undo", json!({})),
("edit.redo", json!({})),
("cell.set", json!({"cell": "F11", "input": "x"})),
("cell.set", json!({"cell": "H1", "input": "2"})),
("edit.clearContents", json!({"range": "E8:F10"})),
];
for (i, (id, params)) in steps.iter().enumerate() {
s.execute(id, params.clone()).unwrap();
assert_matches_full_recalc(&s);
if i == 0 {
// Both held a spill: reading order gives F8 the cells.
assert_eq!(v(&s, "F12"), Value::Number(5.0));
assert_eq!(v(&s, "E10"), Value::Error(Spill));
}
if i == 2 {
assert_eq!(v(&s, "E10"), Value::Number(10.0));
assert_eq!(v(&s, "F10"), Value::Number(20.0));
}
if i == 3 {
// Undone back to the grown F8.
assert_eq!(v(&s, "F12"), Value::Number(5.0));
assert_eq!(v(&s, "E10"), Value::Error(Spill));
}
if i == 8 {
assert_eq!((v(&s, "F8"), v(&s, "E10"), v(&s, "F9")), (Value::Empty, Value::Empty, Value::Empty));
}
}
}
#[test]
fn overlapping_spills_with_no_history_go_to_the_earlier_anchor() {
use gridcraft_core::CellError::Spill;
// A workbook calculated for the first time (a file that carries no layout) has no entry
// order: the anchor first in reading order wins, whatever order the formulas were set in.
for order in [["E10", "F8"], ["F8", "E10"]] {
let mut s = s();
for a in order {
let input = if a == "E10" { "=SEQUENCE(1,3)" } else { "=SEQUENCE(5)" };
s.execute("cell.set", json!({"cell": a, "input": input})).unwrap();
}
let mut fresh = (*s.doc().unwrap().wb).clone();
for i in 0..fresh.sheets.len() {
let sh = fresh.sheet_mut(i).unwrap();
sh.spill.clear();
sh.spill_ranges.clear();
}
gridcraft_calc::Calc::new().recalc_all(&mut fresh);
let a = fresh.active().unwrap();
let cell = |r: &str| a.value(CellRef::parse(r).unwrap());
assert_ne!(cell("F8"), Value::Error(Spill), "order {order:?}");
assert_eq!(cell("E10"), Value::Error(Spill), "order {order:?}");
}
}
#[test]
fn multi_area_rule_formulas_are_relative_to_the_top_left_cell_of_all_areas() {
// Issue #3152: Excel writes the formula for the top-left cell of all the areas (H26), whatever
-1
View File
@@ -23,7 +23,6 @@ craft-csv = { workspace = true }
craft-offcrypto = { workspace = true }
craft-opc = { workspace = true }
craft-xls = { workspace = true }
craft-xml = { workspace = true }
thiserror = { workspace = true }
quick-xml = { workspace = true }
zip = { workspace = true }
+48 -19
View File
@@ -359,9 +359,31 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res
// ---------------------------------------------------------------- writing helpers
// XML 1.0 byte escaping is shared; spreadsheet string and phonetic policy stays here.
use craft_xml::is_xml_char;
pub use craft_xml::{esc, esc_attr, push_esc};
/// Escapes text content.
pub fn esc(s: &str) -> String {
let mut o = String::with_capacity(s.len());
push_esc(&mut o, s);
o
}
/// Appends [`esc`]`(s)` to `o`.
pub fn push_esc(o: &mut String, s: &str) {
if s.bytes().all(|b| matches!(b, b'\t' | b'\n' | 0x20..=0x7E) && !matches!(b, b'&' | b'<' | b'>')) {
o.push_str(s);
return;
}
o.reserve(s.len());
for c in s.chars() {
match c {
'&' => o.push_str("&amp;"),
'<' => o.push_str("&lt;"),
'>' => o.push_str("&gt;"),
'\r' => o.push_str("&#13;"),
c if is_xml_char(c) => o.push(c),
_ => {}
}
}
}
/// Appends `n` in decimal.
pub fn push_u64(o: &mut String, mut n: u64) {
@@ -400,6 +422,29 @@ pub fn push_a1(o: &mut String, c: gridcraft_core::CellRef) {
push_u64(o, u64::from(c.row) + 1);
}
/// Escapes an attribute value (double-quoted).
pub fn esc_attr(s: &str) -> String {
let mut o = String::with_capacity(s.len());
for c in s.chars() {
match c {
'&' => o.push_str("&amp;"),
'<' => o.push_str("&lt;"),
'>' => o.push_str("&gt;"),
'"' => o.push_str("&quot;"),
'\n' => o.push_str("&#10;"),
'\r' => o.push_str("&#13;"),
'\t' => o.push_str("&#9;"),
c if is_xml_char(c) => o.push(c),
_ => {}
}
}
o
}
fn is_xml_char(c: char) -> bool {
matches!(c, '\t' | '\n' | '\r' | '\u{20}'..='\u{D7FF}' | '\u{E000}'..='\u{FFFD}' | '\u{10000}'..='\u{10FFFF}')
}
/// ST_Xstring encoding for cell text: characters XML can't carry and carriage returns become
/// `_xHHHH_`, and literal `_xHHHH_` sequences are protected with `_x005F_`. The result still needs
/// [`esc`].
@@ -760,19 +805,3 @@ mod tests {
assert_eq!("1.5E20".parse::<f64>().unwrap(), 1.5e20);
}
}
#[cfg(test)]
mod shared_escape_parity {
use super::*;
#[test]
fn xml_content_and_attributes_preserve_grid_bytes() {
let value = "&<>\"'\t\n\r\0\u{1f}\u{fffe}\u{ffff}é😀";
assert_eq!(esc(value), "&amp;&lt;&gt;\"'\t\n&#13;é😀");
assert_eq!(esc_attr(value), "&amp;&lt;&gt;&quot;'&#9;&#10;&#13;é😀");
let mut appended = "prefix:".to_owned();
push_esc(&mut appended, value);
assert_eq!(appended, format!("prefix:{}", esc(value)));
assert_eq!(esc("plain ASCII\t\n"), "plain ASCII\t\n");
assert_eq!(decode_xstring(&encode_xstring("_x000D_\r\0😀")), "_x000D_\r\0😀");
}
}