diff --git a/crates/engine/src/macros/mod.rs b/crates/engine/src/macros/mod.rs index 5819e79fd..d236a3f85 100644 --- a/crates/engine/src/macros/mod.rs +++ b/crates/engine/src/macros/mod.rs @@ -128,6 +128,11 @@ pub fn with_names(wb: &Workbook) -> crate::Result> { /// The JSON format's bytes for `wb`: the workbook, with the macros in a newer format put back into /// the `macros` list as the file held them. pub fn json_bytes(wb: &Workbook) -> crate::Result> { + // Every path that adds a macro checks it, so this refuses only one that skipped the check, as + // the xlsx save does, instead of writing a macro the next open would drop. + for m in &wb.macros { + check(m).map_err(crate::EngineError::Other)?; + } let bytes = if wb.macros_unread.is_empty() { serde_json::to_vec_pretty(wb) } else { diff --git a/crates/engine/src/macros/tests.rs b/crates/engine/src/macros/tests.rs index d87857177..dc25de621 100644 --- a/crates/engine/src/macros/tests.rs +++ b/crates/engine/src/macros/tests.rs @@ -213,7 +213,11 @@ fn a_macro_built_in_code_goes_through_the_size_check_too() { assert!(check(&big).unwrap_err().contains("too large")); let mut wb = Workbook::new(); wb.macros.push(big); - assert!(crate::io::save_bytes(&wb, "book.xlsx").is_err(), "a macro that fails `check` fails the save"); + // A macro that fails `check` fails the save in either format, rather than being written and + // dropped the next time the file opens. + for file in ["book.xlsx", "book.json"] { + assert!(crate::io::save_bytes(&wb, file).is_err(), "{file}"); + } } fn visible(name: &str) -> DefinedName { @@ -326,7 +330,8 @@ fn damaged_or_oversized_macros_are_dropped_with_a_warning() { let mut wb = Workbook::new(); wb.macros = (0..MAX_MACROS + 5).map(|i| Macro { name: format!("M{i}"), ..Macro::default() }).collect(); wb.macros[0].steps = vec![Step::command("x", json!({})); MAX_MACRO_STEPS + 1]; - let bytes = crate::io::save_bytes(&wb, "many.json").unwrap(); + // Written as another program would: a GridCraft save refuses a macro that fails `check`. + let bytes = serde_json::to_vec(&wb).unwrap(); let (back, warnings) = crate::io::open_bytes("many.json", &bytes).unwrap(); // Only the first MAX_MACROS entries are read, as in xlsx. assert_eq!(back.macros.len(), MAX_MACROS - 1); @@ -339,7 +344,8 @@ fn damaged_or_oversized_macros_are_dropped_with_a_warning() { assert!(err.contains("too large"), "{err}"); let mut wb = Workbook::new(); wb.macros.push(Macro { name: "Huge".into(), steps: vec![Step::command("cell.set", json!({"input": huge}))], ..Macro::default() }); - let bytes = crate::io::save_bytes(&wb, "huge.json").unwrap(); + // Written as another program would: a GridCraft save refuses a macro that fails `check`. + let bytes = serde_json::to_vec(&wb).unwrap(); let (back, warnings) = crate::io::open_bytes("huge.json", &bytes).unwrap(); assert!(back.macros.is_empty() && warnings[0].starts_with("1 macro "), "{warnings:?}"); } @@ -368,7 +374,8 @@ fn json_field_macros_get_the_name_check_too() { let mut wb = Workbook::new(); wb.macros.push(Macro { name: "a\nb".into(), ..Macro::default() }); wb.macros.push(Macro { name: "Fine".into(), ..Macro::default() }); - let bytes = crate::io::save_bytes(&wb, "ctl.json").unwrap(); + // Written as another program would: a GridCraft save refuses a macro that fails `check`. + let bytes = serde_json::to_vec(&wb).unwrap(); let (back, warnings) = crate::io::open_bytes("ctl.json", &bytes).unwrap(); let names: Vec<&str> = back.macros.iter().map(|m| m.name.as_str()).collect(); assert_eq!(names, ["Fine"]);