Merge pull request #609 from MarekSpodBiedry:issue-552

crates/xlsx/src/tests/mod.rs

@@ -10,6 +10,7 @@ mod passthrough_tests;
 mod pivot_tests;
 mod richdata_tests;
 mod roundtrip;
+mod sheet_names;
 mod smooth_roundtrip;
 mod trendline_roundtrip;
 mod xls_tests;

crates/xlsx/src/tests/sheet_names.rs

@@ -0,0 +1,23 @@
+//! Sheet names XLSX cannot hold are refused on export instead of breaking formulas on reopen.
+
+use std::sync::Arc;
+
+use gridcraft_model::{Sheet, Workbook};
+
+use crate::{IoError, write_xlsx};
+
+fn book(names: &[&str]) -> Workbook {
+    let mut wb = Workbook::new();
+    wb.sheets = names.iter().map(|n| Arc::new(Sheet::new(*n))).collect();
+    wb
+}
+
+#[test]
+fn unrepresentable_sheet_names_are_refused() {
+    let long_a = format!("{}X", "A".repeat(31));
+    let long_b = format!("{}Y", "A".repeat(31));
+    for names in [vec!["Data", "a/b"], vec![long_a.as_str(), long_b.as_str()], vec!["Data", "data"], vec!["Data", ""]] {
+        assert!(matches!(write_xlsx(&book(&names)), Err(IoError::Format(_))), "{names:?}");
+    }
+    assert!(write_xlsx(&book(&["Data", "Other"])).is_ok());
+}

crates/xlsx/src/write.rs

@@ -161,6 +161,30 @@ pub fn abs_range(r: &RangeRef) -> String {
     format!("${}${}:${}${}", c(r.start.col), r.start.row + 1, c(r.end.col), r.end.row + 1)
 }
 
+/// Refuses sheet names XLSX cannot hold. The reader repairs such names (cuts, strips, renames
+/// duplicates) but formulas keep the old spelling, so the saved file would turn into `#REF!`.
+fn check_sheet_names(wb: &Workbook) -> Result<(), IoError> {
+    let mut seen = std::collections::HashSet::new();
+    for sheet in &wb.sheets {
+        let name = &sheet.name;
+        let bad = if name.is_empty() {
+            Some("is blank")
+        } else if name.chars().count() > 31 {
+            Some("is longer than 31 characters")
+        } else if name.chars().any(|c| matches!(c, ':' | '\\' | '/' | '?' | '*' | '[' | ']')) {
+            Some("contains one of : \\ / ? * [ ]")
+        } else if !seen.insert(name.to_ascii_lowercase()) {
+            Some("is used by another sheet")
+        } else {
+            None
+        };
+        if let Some(why) = bad {
+            return Err(IoError::Format(format!("sheet name \"{name}\" {why}; rename the sheet before saving as XLSX")));
+        }
+    }
+    Ok(())
+}
+
 pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result<Vec<u8>, IoError> {
     if wb.sheets.is_empty() {
         return Err(IoError::Format("a workbook needs at least one sheet".into()));
@@ -168,6 +192,7 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result<Vec<u8>, IoError> {
     if wb.sheets.iter().any(|sheet| sheet.cell_pictures.keys().any(|c| sheet.cell(*c).is_some_and(|cell| cell.formula.is_some()))) {
         return Err(IoError::Format("a static cell picture cannot also contain a formula".into()));
     }
+    check_sheet_names(wb)?;
     let mut out = Out {
         parts: vec![],
         overrides: vec![],