From 70408e565376f111adb004bfe283b0c30d0becc6 Mon Sep 17 00:00:00 2001 From: Frank Colson Date: Tue, 17 Feb 2026 15:44:00 -0700 Subject: [PATCH 1/2] fix(cell): serialize formula cache with correct cell types Make formula cell XML type emission follow cached raw value kinds instead of forcing t="str". Also fix error serialization to emit the actual cached error token (for example #N/A) rather than hard-coding #VALUE!. Add unit and integration coverage for typed formula cache writeback, roundtrip semantics, and shared-formula metadata stability. --- src/structs/cell.rs | 3 +- src/structs/cell_value.rs | 34 +++++- tests/integration_test.rs | 219 +++++++++++++++++++++++++++++++++++++- 3 files changed, 252 insertions(+), 4 deletions(-) diff --git a/src/structs/cell.rs b/src/structs/cell.rs index 4d1efabf..aec2e6c1 100644 --- a/src/structs/cell.rs +++ b/src/structs/cell.rs @@ -703,8 +703,7 @@ impl Cell { write_text_node(writer, prm); } "e" => { - let prm = "#VALUE!"; - write_text_node(writer, prm); + write_text_node(writer, self.value()); } _ => write_text_node_conversion(writer, self.value()), } diff --git a/src/structs/cell_value.rs b/src/structs/cell_value.rs index 2908f61e..5306955f 100644 --- a/src/structs/cell_value.rs +++ b/src/structs/cell_value.rs @@ -52,7 +52,15 @@ impl CellValue { #[inline] pub(crate) fn data_type_crate(&self) -> &str { match &self.formula { - Some(_) => "str", + Some(_) => match &self.raw_value { + CellRawValue::String(_) | CellRawValue::RichText(_) | CellRawValue::Lazy(_) => { + "str" + } + CellRawValue::Bool(_) => "b", + CellRawValue::Error(_) => "e", + CellRawValue::Numeric(_) => "n", + CellRawValue::Empty => "", + }, None => self.raw_value.data_type(), } } @@ -440,4 +448,28 @@ mod tests { assert!(cell.raw_value.is_error()); assert_eq!(cell.raw_value, CellRawValue::Error(CellErrorType::Null)); } + + #[test] + fn formula_cached_value_data_type_tracks_raw_value_kind() { + let mut obj = CellValue::default(); + + obj.set_formula("1+1").set_formula_result_default("2"); + assert_eq!(obj.data_type_crate(), "n"); + + obj.set_formula_result_default("TRUE"); + assert_eq!(obj.data_type_crate(), "b"); + + obj.set_formula_result_default("#N/A"); + assert_eq!(obj.data_type_crate(), "e"); + + obj.set_formula_result_default("OK"); + assert_eq!(obj.data_type_crate(), "str"); + + obj.set_formula_result_default(""); + assert_eq!(obj.data_type_crate(), ""); + + obj.remove_formula(); + obj.set_value_string("OK"); + assert_eq!(obj.data_type_crate(), "s"); + } } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 6fc2bcce..b572a1c3 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -3,7 +3,10 @@ extern crate chrono; extern crate umya_spreadsheet; -use std::time::Instant; +use std::{ + io::Read, + time::Instant, +}; use umya_spreadsheet::*; @@ -2208,3 +2211,217 @@ fn issue_298() { let path = std::path::Path::new("./tests/result_files/r_issue_298.xlsx"); let _unused = writer::xlsx::write(&book, path); } + +fn workbook_to_xlsx_bytes(book: &Workbook) -> Vec { + let mut output = Vec::new(); + writer::xlsx::write_writer(book, &mut output).unwrap(); + output +} + +fn zip_entry_to_string(xlsx: &[u8], entry_name: &str) -> String { + let cursor = std::io::Cursor::new(xlsx); + let mut archive = zip::ZipArchive::new(cursor).unwrap(); + let mut entry = archive.by_name(entry_name).unwrap(); + let mut xml = String::new(); + entry.read_to_string(&mut xml).unwrap(); + xml +} + +fn cell_fragment(sheet_xml: &str, coordinate: &str) -> String { + let start = sheet_xml + .find(&format!("") { + return fragment[..(end + 4)].to_string(); + } + + let end = fragment + .find("/>") + .expect("cell node should be either empty or explicitly closed"); + fragment[..(end + 2)].to_string() +} + +fn shared_formula_signatures( + sheet_xml: &str, +) -> Vec<(String, Option, Option, Option)> { + let mut reader = quick_xml::Reader::from_str(sheet_xml); + reader.config_mut().trim_text(true); + + let mut signatures = Vec::new(); + let mut buf = Vec::new(); + let mut current_cell = String::new(); + let mut shared_si: Option = None; + let mut shared_ref: Option = None; + let mut shared_text: Option = None; + let mut in_shared_formula = false; + + loop { + match reader.read_event_into(&mut buf) { + Ok(quick_xml::events::Event::Start(ref e)) => match e.name().into_inner() { + b"c" => { + current_cell.clear(); + for attribute in e.attributes().flatten() { + if attribute.key.into_inner() == b"r" { + current_cell = String::from_utf8_lossy(attribute.value.as_ref()).into(); + } + } + } + b"f" => { + let mut formula_type: Option = None; + shared_si = None; + shared_ref = None; + shared_text = None; + for attribute in e.attributes().flatten() { + let value = String::from_utf8_lossy(attribute.value.as_ref()).into_owned(); + match attribute.key.into_inner() { + b"t" => formula_type = Some(value), + b"si" => shared_si = Some(value), + b"ref" => shared_ref = Some(value), + _ => {} + } + } + in_shared_formula = formula_type.as_deref() == Some("shared"); + } + _ => {} + }, + Ok(quick_xml::events::Event::Empty(ref e)) => { + if e.name().into_inner() == b"f" { + let mut formula_type: Option = None; + let mut si: Option = None; + let mut reference: Option = None; + for attribute in e.attributes().flatten() { + let value = String::from_utf8_lossy(attribute.value.as_ref()).into_owned(); + match attribute.key.into_inner() { + b"t" => formula_type = Some(value), + b"si" => si = Some(value), + b"ref" => reference = Some(value), + _ => {} + } + } + if formula_type.as_deref() == Some("shared") { + signatures.push((current_cell.clone(), si, reference, None)); + } + } + } + Ok(quick_xml::events::Event::Text(e)) => { + if in_shared_formula { + shared_text = Some(e.unescape().unwrap().into_owned()); + } + } + Ok(quick_xml::events::Event::End(ref e)) => { + if e.name().into_inner() == b"f" { + if in_shared_formula { + signatures.push(( + current_cell.clone(), + shared_si.clone(), + shared_ref.clone(), + shared_text.clone(), + )); + } + in_shared_formula = false; + shared_si = None; + shared_ref = None; + shared_text = None; + } + } + Ok(quick_xml::events::Event::Eof) => break, + Err(e) => panic!("failed to parse sheet xml: {e}"), + _ => {} + } + buf.clear(); + } + + signatures +} + +#[test] +fn formula_cached_values_are_written_with_typed_xml_and_roundtrip() { + let mut book = new_file(); + let sheet = book.sheet_mut(0).unwrap(); + + sheet + .get_cell_mut("A1") + .set_formula("1+1") + .set_formula_result_default("2"); + sheet + .get_cell_mut("A2") + .set_formula("1=1") + .set_formula_result_default("TRUE"); + sheet + .get_cell_mut("A3") + .set_formula("NA()") + .set_formula_result_default("#N/A"); + sheet + .get_cell_mut("A4") + .set_formula("T(\"ok\")") + .set_formula_result_default("ok"); + sheet + .get_cell_mut("A5") + .set_formula("1/0") + .set_formula_result_default(""); + + let xlsx = workbook_to_xlsx_bytes(&book); + let sheet_xml = zip_entry_to_string(&xlsx, "xl/worksheets/sheet1.xml"); + + let a1 = cell_fragment(&sheet_xml, "A1"); + assert!(a1.contains("1+1")); + assert!(a1.contains("2")); + assert!(!a1.contains("t=\"str\"")); + assert!(!a1.contains("t=\"b\"")); + assert!(!a1.contains("t=\"e\"")); + + let a2 = cell_fragment(&sheet_xml, "A2"); + assert!(a2.contains("t=\"b\"")); + assert!(a2.contains("1")); + + let a3 = cell_fragment(&sheet_xml, "A3"); + assert!(a3.contains("t=\"e\"")); + assert!(a3.contains("#N/A")); + + let a4 = cell_fragment(&sheet_xml, "A4"); + assert!(a4.contains("t=\"str\"")); + assert!(a4.contains("ok")); + + let a5 = cell_fragment(&sheet_xml, "A5"); + assert!(a5.contains("")); + + let roundtrip = reader::xlsx::read_reader(std::io::Cursor::new(xlsx), true).unwrap(); + let roundtrip_sheet = roundtrip.sheet(0).unwrap(); + + let a1_cell = roundtrip_sheet.get_cell("A1").unwrap(); + assert_eq!(a1_cell.formula(), "1+1"); + assert_eq!(a1_cell.raw_value(), &CellRawValue::Numeric(2f64)); + + let a2_cell = roundtrip_sheet.get_cell("A2").unwrap(); + assert_eq!(a2_cell.formula(), "1=1"); + assert_eq!(a2_cell.raw_value(), &CellRawValue::Bool(true)); + + let a3_cell = roundtrip_sheet.get_cell("A3").unwrap(); + assert_eq!(a3_cell.formula(), "NA()"); + assert_eq!(a3_cell.raw_value(), &CellRawValue::Error(CellErrorType::NA)); + + let a4_cell = roundtrip_sheet.get_cell("A4").unwrap(); + assert_eq!(a4_cell.formula(), "T(\"ok\")"); + assert!(matches!( + a4_cell.raw_value(), + CellRawValue::String(value) if value.as_ref() == "ok" + )); +} + +#[test] +fn write_keeps_shared_formula_metadata_stable() { + let source_path = std::path::Path::new("./tests/test_files/issue_194.xlsx"); + let source_bytes = std::fs::read(source_path).unwrap(); + let source_sheet_xml = zip_entry_to_string(&source_bytes, "xl/worksheets/sheet1.xml"); + let source_shared = shared_formula_signatures(&source_sheet_xml); + assert!(!source_shared.is_empty()); + + let workbook = reader::xlsx::read_reader(std::io::Cursor::new(source_bytes), true).unwrap(); + let rewritten = workbook_to_xlsx_bytes(&workbook); + let rewritten_sheet_xml = zip_entry_to_string(&rewritten, "xl/worksheets/sheet1.xml"); + let rewritten_shared = shared_formula_signatures(&rewritten_sheet_xml); + + assert_eq!(rewritten_shared, source_shared); +} From 55ceb527ecd2482b00c4c6da7156610fb6ae2973 Mon Sep 17 00:00:00 2001 From: Frank Colson Date: Tue, 17 Feb 2026 15:46:01 -0700 Subject: [PATCH 2/2] feat(cell): add typed formula cached-result setters Add explicit helpers to set formula cached values as number, bool, error, string, or blank without removing the formula object. Expose these helpers on both CellValue and Cell, and add regression coverage for API behavior and XML output typing. --- src/structs/cell.rs | 33 ++++++++++++++++ src/structs/cell_value.rs | 82 +++++++++++++++++++++++++++++++++++++++ tests/integration_test.rs | 49 +++++++++++++++++++++++ 3 files changed, 164 insertions(+) diff --git a/src/structs/cell.rs b/src/structs/cell.rs index aec2e6c1..24af5200 100644 --- a/src/structs/cell.rs +++ b/src/structs/cell.rs @@ -350,6 +350,39 @@ impl Cell { self } + #[inline] + pub fn set_formula_result_number(&mut self, value: T) -> &mut Self + where + T: Into, + { + self.cell_value.set_formula_result_number(value); + self + } + + #[inline] + pub fn set_formula_result_bool(&mut self, value: bool) -> &mut Self { + self.cell_value.set_formula_result_bool(value); + self + } + + #[inline] + pub fn set_formula_result_error(&mut self, value: crate::CellErrorType) -> &mut Self { + self.cell_value.set_formula_result_error(value); + self + } + + #[inline] + pub fn set_formula_result_string>(&mut self, value: S) -> &mut Self { + self.cell_value.set_formula_result_string(value); + self + } + + #[inline] + pub fn set_formula_result_blank(&mut self) -> &mut Self { + self.cell_value.set_formula_result_blank(); + self + } + #[inline] pub fn set_blank(&mut self) -> &mut Self { self.cell_value.set_blank(); diff --git a/src/structs/cell_value.rs b/src/structs/cell_value.rs index 5306955f..fbc262fd 100644 --- a/src/structs/cell_value.rs +++ b/src/structs/cell_value.rs @@ -274,6 +274,59 @@ impl CellValue { self } + /// Sets a formula cached result as a numeric value. + /// + /// This method only updates the cached value (``) and does not remove + /// the formula object (``). + #[inline] + pub fn set_formula_result_number(&mut self, value: T) -> &mut Self + where + T: Into, + { + self.raw_value = CellRawValue::Numeric(value.into()); + self + } + + /// Sets a formula cached result as a boolean value. + /// + /// This method only updates the cached value (``) and does not remove + /// the formula object (``). + #[inline] + pub fn set_formula_result_bool(&mut self, value: bool) -> &mut Self { + self.raw_value = CellRawValue::Bool(value); + self + } + + /// Sets a formula cached result as an Excel error value. + /// + /// This method only updates the cached value (``) and does not remove + /// the formula object (``). + #[inline] + pub fn set_formula_result_error(&mut self, value: CellErrorType) -> &mut Self { + self.raw_value = CellRawValue::Error(value); + self + } + + /// Sets a formula cached result as a string value. + /// + /// This method only updates the cached value (``) and does not remove + /// the formula object (``). + #[inline] + pub fn set_formula_result_string>(&mut self, value: S) -> &mut Self { + self.raw_value = CellRawValue::String(value.into().into_boxed_str()); + self + } + + /// Sets a formula cached result as blank. + /// + /// This method only updates the cached value (``) and does not remove + /// the formula object (``). + #[inline] + pub fn set_formula_result_blank(&mut self) -> &mut Self { + self.raw_value = CellRawValue::Empty; + self + } + #[inline] pub fn set_error>(&mut self, value: S) -> &mut Self { self.set_value_crate(value); @@ -472,4 +525,33 @@ mod tests { obj.set_value_string("OK"); assert_eq!(obj.data_type_crate(), "s"); } + + #[test] + fn typed_formula_result_helpers_preserve_formula() { + let mut obj = CellValue::default(); + obj.set_formula("A1+1"); + + obj.set_formula_result_number(3.5); + assert_eq!(obj.formula(), "A1+1"); + assert_eq!(obj.raw_value, CellRawValue::Numeric(3.5)); + + obj.set_formula_result_bool(false); + assert_eq!(obj.formula(), "A1+1"); + assert_eq!(obj.raw_value, CellRawValue::Bool(false)); + + obj.set_formula_result_error(CellErrorType::Ref); + assert_eq!(obj.formula(), "A1+1"); + assert_eq!(obj.raw_value, CellRawValue::Error(CellErrorType::Ref)); + + obj.set_formula_result_string("OK"); + assert_eq!(obj.formula(), "A1+1"); + assert!(matches!( + &obj.raw_value, + CellRawValue::String(value) if value.as_ref() == "OK" + )); + + obj.set_formula_result_blank(); + assert_eq!(obj.formula(), "A1+1"); + assert_eq!(obj.raw_value, CellRawValue::Empty); + } } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index b572a1c3..ccd45009 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -2425,3 +2425,52 @@ fn write_keeps_shared_formula_metadata_stable() { assert_eq!(rewritten_shared, source_shared); } + +#[test] +fn typed_formula_result_helpers_write_expected_types() { + let mut book = new_file(); + let sheet = book.sheet_mut(0).unwrap(); + + sheet + .get_cell_mut("B1") + .set_formula("10/2") + .set_formula_result_number(5.0); + sheet + .get_cell_mut("B2") + .set_formula("1=2") + .set_formula_result_bool(false); + sheet + .get_cell_mut("B3") + .set_formula("NA()") + .set_formula_result_error(CellErrorType::NA); + sheet + .get_cell_mut("B4") + .set_formula("T(\"value\")") + .set_formula_result_string("value"); + sheet + .get_cell_mut("B5") + .set_formula("1/0") + .set_formula_result_blank(); + + let xlsx = workbook_to_xlsx_bytes(&book); + let sheet_xml = zip_entry_to_string(&xlsx, "xl/worksheets/sheet1.xml"); + + let b1 = cell_fragment(&sheet_xml, "B1"); + assert!(!b1.contains("t=\"str\"")); + assert!(b1.contains("5")); + + let b2 = cell_fragment(&sheet_xml, "B2"); + assert!(b2.contains("t=\"b\"")); + assert!(b2.contains("0")); + + let b3 = cell_fragment(&sheet_xml, "B3"); + assert!(b3.contains("t=\"e\"")); + assert!(b3.contains("#N/A")); + + let b4 = cell_fragment(&sheet_xml, "B4"); + assert!(b4.contains("t=\"str\"")); + assert!(b4.contains("value")); + + let b5 = cell_fragment(&sheet_xml, "B5"); + assert!(b5.contains("")); +}