Engine: a macro in a newer format is dropped with a warning, not saved back as version 1

Opening a workbook accepted a macro whose format version `v` was above MACRO_VERSION. Serde
dropped the fields this build doesn't know, and the next save wrote the reduced macro back
with `v: 2`, so a newer build would read it as a complete version 2 macro.

A version above MACRO_VERSION is now refused by Macro::check_limits, and parse_macro checks
the raw `v` before reading the steps. Such a macro takes the existing per-macro path: it is
left out with the usual single warning (which now names a newer format), and the rest of the
workbook opens. Tests cover JSON and xlsx hidden names, with current and missing versions as
controls.

crates/engine/src/macros/mod.rs

@@ -20,6 +20,10 @@ const MACRO_KEY: &str = "macro_";
 /// Reads a macro from JSON, as `macro.save` and files give it. A command step may also use the
 /// short form `{"command": "cell.set", "params": {…}}`.
 pub fn parse_macro(v: &Json) -> Result<Macro, String> {
+    // A newer format is refused before its steps are read, so the message says why.
+    if let Some(found) = v.get("v").and_then(Json::as_u64) {
+        gridcraft_model::check_version(found)?;
+    }
     let mut v = v.clone();
     if let Some(steps) = v.get_mut("steps") {
         normalize_steps(steps, 1)?;
@@ -99,7 +103,7 @@ pub fn with_names(wb: &Workbook) -> crate::Result<Option<Workbook>> {
 }
 
 /// After a file is opened: moves macros out of hidden names into `Workbook::macros` and drops
-/// macros that are damaged or over the limits. Returns a warning for what was dropped.
+/// macros that are damaged, over the limits or in a newer format. Returns a warning for what was dropped.
 pub fn load(wb: &mut Workbook) -> Vec<String> {
     let mut warnings = Vec::new();
     let mut bad = 0usize;
@@ -156,7 +160,7 @@ pub fn load(wb: &mut Workbook) -> Vec<String> {
     }
     if bad > 0 {
         warnings.push(format!(
-            "{bad} macro{} in the workbook couldn't be opened: {} damaged or over the limits ({} macros, {} steps each, Repeat and If nested {} deep).",
+            "{bad} macro{} in the workbook couldn't be opened: {} damaged, made by a newer version of GridCraft, or over the limits ({} macros, {} steps each, Repeat and If nested {} deep).",
             if bad == 1 { "" } else { "s" },
             if bad == 1 { "it is" } else { "they are" },
             gridcraft_model::MAX_MACROS,

crates/engine/src/macros/tests.rs

@@ -278,3 +278,42 @@ fn a_damaged_json_macro_drops_alone() {
     assert!(warnings.len() == 1 && warnings[0].starts_with("3 macros"), "{warnings:?}");
     assert_eq!(back.sheet(0).unwrap().value(gridcraft_core::CellRef::new(0, 0)), gridcraft_core::Value::text("kept"));
 }
+
+#[test]
+fn a_macro_in_a_newer_format_is_dropped_not_rewritten() {
+    // A newer build's macro (`v` 2, with a field this build doesn't know) would lose that field
+    // here and be saved back as if complete, so it is dropped with the usual warning instead.
+    // Version 1 and a missing version are read as before.
+    let future = json!({"name": "Future", "v": MACRO_VERSION + 1, "fromALaterBuild": {"x": 1}, "steps": [{"command": "cell.set"}]});
+    let current = json!({"name": "Current", "v": MACRO_VERSION, "steps": [{"command": "cell.set"}]});
+    let missing = json!({"name": "Missing"});
+    let err = parse_macro(&future).unwrap_err();
+    assert!(err.contains("newer version") && err.contains("format 2"), "{err}");
+    assert!(parse_macro(&json!({"name": "Huge", "v": u64::from(u32::MAX) + 1})).unwrap_err().contains("newer version"));
+    // A macro built in code goes through the same check on every path.
+    assert!(Macro { name: "Built".into(), v: MACRO_VERSION + 1, ..Macro::default() }.check_limits().is_err());
+    let check = |file: &str, back: &Workbook, warnings: &[String]| {
+        let names: Vec<&str> = back.macros.iter().map(|m| m.name.as_str()).collect();
+        assert_eq!(names, ["Current", "Missing"], "{file}");
+        assert!(back.macros.iter().all(|m| m.v == MACRO_VERSION), "{file}");
+        assert!(warnings.len() == 1 && warnings[0].starts_with("1 macro ") && warnings[0].contains("newer version"), "{file}: {warnings:?}");
+        // Saving and opening again leaves no reduced copy of it behind.
+        let (again, warnings) = crate::io::open_bytes(file, &crate::io::save_bytes(back, file).unwrap()).unwrap();
+        assert!(warnings.is_empty(), "{file}: {warnings:?}");
+        assert_eq!(again.macros, back.macros, "{file}");
+    };
+    // The JSON format's field.
+    let mut v: serde_json::Value = serde_json::from_slice(&crate::io::save_bytes(&Workbook::new(), "book.json").unwrap()).unwrap();
+    v["macros"] = json!([future, current, missing]);
+    let (back, warnings) = crate::io::open_bytes("book.json", v.to_string().as_bytes()).unwrap();
+    check("book.json", &back, &warnings);
+    // Hidden names in xlsx.
+    let mut wb = Workbook::new();
+    for (i, m) in [&future, &current, &missing].into_iter().enumerate() {
+        wb.names.push(hidden(&format!("_gc_macro_{i}_0"), &m.to_string()));
+    }
+    let bytes = gridcraft_xlsx::write_xlsx(&wb).unwrap();
+    let (back, warnings) = crate::io::open_bytes("book.xlsx", &bytes).unwrap();
+    assert!(back.names.is_empty(), "{:?}", names_of(&back));
+    check("book.xlsx", &back, &warnings);
+}

crates/model/src/macros.rs

@@ -63,9 +63,10 @@ impl Macro {
         count(&self.steps)
     }
 
-    /// Checks the size limits ([`MAX_MACRO_STEPS`], [`MAX_MACRO_DEPTH`], name and text lengths) and
-    /// that the name has no control characters.
+    /// Checks the format version ([`check_version`]), the size limits ([`MAX_MACRO_STEPS`],
+    /// [`MAX_MACRO_DEPTH`], name and text lengths) and that the name has no control characters.
     pub fn check_limits(&self) -> Result<(), String> {
+        check_version(u64::from(self.v))?;
         let name = self.name.trim();
         if name.is_empty() {
             return Err("A macro needs a name.".into());
@@ -119,6 +120,15 @@ impl Macro {
     }
 }
 
+/// Refuses a format version newer than [`MACRO_VERSION`]: this build would drop the fields it
+/// doesn't know, and a save would then write the rest back as if it were the whole macro.
+pub fn check_version(v: u64) -> Result<(), String> {
+    if v > u64::from(MACRO_VERSION) {
+        return Err(format!("This macro was made by a newer version of GridCraft (macro format {v}); this version reads format {MACRO_VERSION}."));
+    }
+    Ok(())
+}
+
 /// Reads the `macros` field of a JSON workbook as raw entries, whatever they hold, so they can be
 /// checked one at a time.
 pub(crate) fn entries<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Vec<serde_json::Value>, D::Error> {

docs/macros.md

@@ -49,6 +49,12 @@ Macros are saved in the workbook: as a field in GridCraft's JSON format, and in
 defined names (`_gc_macro_<n>_<k>`, the macro's JSON split into string literals within Excel's
 limits), which other spreadsheet applications keep but do not run.
 
+Each macro carries its format version, `v` (1 in this version; a missing `v` reads as 1). When a
+file is opened, a macro that is damaged, over the limits or in a newer format (a `v` above 1) is
+left out with one warning, and the rest of the workbook and its other macros open. A newer
+macro is never read in part, so a save can't write it back with its newer fields missing; a
+macro that was left out is not in the saved file.
+
 ## Automate scripts and VBA
 
 Automate scripts (`automate.recordActions`, `newScript`, `allScripts`, `runScript`,