diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 7527b37aa..ec73e2e2a 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -401,7 +401,9 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) } /// The most written cells a macro's footprint check records. -const TOUCH_LOG_LIMIT: usize = 1_000_000; +/// The most cells a dry run's touch log keeps; a footprint whose log is full may be missing writes +/// that kept a cell's value, so a shift is refused then. +pub(crate) const TOUCH_LOG_LIMIT: usize = 1_000_000; /// Excel's message for an edit of a locked cell on a protected sheet. pub(crate) const PROTECTED: &str = "The cell or chart you're trying to change is on a protected sheet."; diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index 889d3dd0f..ab67a9ce2 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -772,6 +772,10 @@ pub struct Footprint { /// Steps executed by the end of the dry run that made this footprint (the run's one budget). #[serde(skip)] pub executed: u64, + /// The dry run wrote more cells than the touch log keeps, so writes that kept a cell's value + /// may be missing from `cells`: a shift cannot trust the block. + #[serde(skip_serializing_if = "std::ops::Not::not")] + pub log_full: bool, pub sheets: Vec, pub sheets_added: Vec, pub sheets_removed: Vec, @@ -946,6 +950,7 @@ pub fn footprint(s: &mut Session, p: &Prepared, clock: Stopwatch, time_limit_ms: rolled?; let (after, touched) = if matches!(m.stopped, Some((_, true))) { (before.clone(), Vec::new()) } else { (after, touched) }; let mut fp = diff(&before, &after, &p.inputs, &touched); + fp.log_full = touched.len() >= crate::cmd::TOUCH_LOG_LIMIT; fp.executed = m.executed; fp.error = outcome.err(); fp.stopped = m.stopped.map(|(msg, _)| msg); @@ -1094,6 +1099,9 @@ fn make_room(s: &mut Session, fp: &Footprint, p: &Prepared, down: bool) -> Resul if !fp.sheets_removed.is_empty() || fp.objects_removed > 0 { return Err(EngineError::Other("Shifting can't make room: the macro deletes sheets or objects.".into())); } + if fp.log_full { + return Err(EngineError::Other("Shifting can't make room: the macro writes more cells than the footprint can follow.".into())); + } let wb = s.doc()?.wb.clone(); let env = Env { wb: &wb, active_sheet: usize::MAX, inputs: &[], output: None, item: None }; let mut moves = Vec::new(); diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index d2aa786d8..285716b83 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -285,6 +285,28 @@ fn a_shift_that_would_move_an_input_is_refused() { assert_eq!(v(&s, "B3").display(), "Ann", "the input did not move"); } +#[test] +fn a_shift_is_refused_when_the_footprint_cannot_follow_every_write() { + // The touch log keeps at most a million writes; past that, a write that kept a cell's value + // is missing from the footprint, so a shift could split a row the macro writes. The footprint + // says so and the shift is refused (overwrite, with no dry run, still runs). + let mut s = s(); + set(&mut s, "A1", "x"); + set(&mut s, "E2", "old"); + put( + &mut s, + json!({"name": "Flood", "steps": [ + {"command": "edit.fillDown", "params": {"range": "A1:A1000001"}}, + {"command": "cell.set", "params": {"cell": "E2", "input": "new"}} + ]}), + ); + let c = s.execute("macro.check", json!({"name": "Flood"})).unwrap(); + assert_eq!(c["footprint"]["logFull"], true, "{c}"); + let err = s.execute("macro.run", json!({"name": "Flood", "onConflict": "shiftDown"})).unwrap_err().to_string(); + assert!(err.contains("more cells than the footprint can follow"), "{err}"); + assert_eq!(v(&s, "E2").display(), "old", "nothing changed"); +} + #[test] fn a_dry_run_leaves_a_clean_workbook_clean() { // `view.zoom` and `sheet.activate` move the saved mark to the current workbook on a clean