make spv writer return an error indication
authorBen Pfaff <blp@cs.stanford.edu>
Wed, 10 Dec 2025 19:38:09 +0000 (11:38 -0800)
committerBen Pfaff <blp@cs.stanford.edu>
Wed, 10 Dec 2025 19:38:09 +0000 (11:38 -0800)
rust/pspp/src/output/drivers/spv.rs
rust/pspp/src/spv/write.rs

index d4be110591d048a8e8af386f9f73dc28ebfa4fd8..a0bdd1d720fffdbda3ae7a20ac3ec4466e034e54 100644 (file)
@@ -73,7 +73,7 @@ where
 
 impl SpvDriver<File> {
     pub fn new(config: &SpvConfig) -> std::io::Result<Self> {
-        let mut writer = Writer::for_writer(File::create(&config.file)?);
+        let mut writer = Writer::for_writer(File::create(&config.file)?).unwrap();
         if let Some(page_setup) = &config.page_setup {
             writer = writer.with_page_setup(page_setup.clone());
         }
@@ -87,7 +87,7 @@ where
 {
     pub fn for_writer(writer: W) -> Self {
         Self {
-            writer: Writer::for_writer(writer),
+            writer: Writer::for_writer(writer).unwrap(),
         }
     }
 }
index 93ae30b3d00df9add4e25b6689e1e20c1262d26b..f83b4b38acf8f6f1b68ee22fba19626d8ae64fe4 100644 (file)
@@ -22,12 +22,13 @@ use std::{
 
 use binrw::{BinWrite, Endian};
 use chrono::Utc;
+use displaydoc::Display;
 use enum_map::EnumMap;
 use quick_xml::{
     ElementWriter, Writer as XmlWriter,
     events::{BytesText, attributes::Attribute},
 };
-use zip::{ZipWriter, result::ZipResult, write::SimpleFileOptions};
+use zip::{ZipWriter, result::ZipError, write::SimpleFileOptions};
 
 use crate::{
     format::{Format, Type},
@@ -46,6 +47,19 @@ use crate::{
     util::ToSmallString,
 };
 
+/// An error writing an SPV file.
+#[derive(Debug, Display, thiserror::Error)]
+pub enum Error {
+    /// {0}
+    ZipError(#[from] ZipError),
+
+    /// {0}
+    IoError(#[from] std::io::Error),
+
+    /// {0}
+    BinrwError(#[from] binrw::Error),
+}
+
 /// SPSS viewer (SPV) file writer.
 pub struct Writer<W>
 where
@@ -64,19 +78,17 @@ where
 {
     /// Creates a new `Writer` to write an SPV file to underlying stream
     /// `writer`.
-    pub fn for_writer(writer: W) -> Self {
+    pub fn for_writer(writer: W) -> Result<Self, Error> {
         let mut writer = ZipWriter::new(writer);
-        writer
-            .start_file("META-INF/MANIFEST.MF", SimpleFileOptions::default())
-            .unwrap();
-        writer.write_all("allowPivoting=true".as_bytes()).unwrap();
-        Self {
+        writer.start_file("META-INF/MANIFEST.MF", SimpleFileOptions::default())?;
+        writer.write_all("allowPivoting=true".as_bytes())?;
+        Ok(Self {
             writer,
             needs_page_break: false,
             next_table_id: 1,
             next_heading_id: 1,
             page_setup: None,
-        }
+        })
     }
 
     /// Returns this `Writer` with `page_setup` set up to be written with the
@@ -100,11 +112,11 @@ where
 
     /// Closes the underlying file and returns the inner writer and the final
     /// I/O result.
-    pub fn close(mut self) -> ZipResult<W> {
+    pub fn close(mut self) -> Result<W, Error> {
         self.writer
             .start_file("META-INF/MANIFEST.MF", SimpleFileOptions::default())?;
         write!(&mut self.writer, "allowPivoting=true")?;
-        self.writer.finish()
+        Ok(self.writer.finish()?)
     }
 
     fn page_break_before(&mut self) -> bool {
@@ -118,7 +130,8 @@ where
         item: &Item,
         pivot_table: &PivotTable,
         structure: &mut XmlWriter<X>,
-    ) where
+    ) -> Result<(), Error>
+    where
         X: Write,
     {
         let table_id = self.next_table_id;
@@ -126,13 +139,12 @@ where
 
         let mut content = Vec::new();
         let mut cursor = Cursor::new(&mut content);
-        pivot_table.write_le(&mut cursor).unwrap();
+        pivot_table.write_le(&mut cursor)?;
 
         let table_name = format!("{table_id:011}_lightTableData.bin");
         self.writer
-            .start_file(&table_name, SimpleFileOptions::default())
-            .unwrap(); // XXX
-        self.writer.write_all(&content).unwrap(); // XXX
+            .start_file(&table_name, SimpleFileOptions::default())?; // XXX
+        self.writer.write_all(&content)?; // XXX
 
         self.container(structure, item, "vtb:table", |element| {
             element
@@ -158,23 +170,29 @@ where
                             Ok(())
                         })?;
                     Ok(())
-                })
-                .unwrap();
-        });
+                })?;
+            Ok(())
+        })?;
+        Ok(())
     }
 
-    fn write_text<X>(&mut self, item: &Item, text: &Text, structure: &mut XmlWriter<X>)
+    fn write_text<X>(
+        &mut self,
+        item: &Item,
+        text: &Text,
+        structure: &mut XmlWriter<X>,
+    ) -> Result<(), Error>
     where
         X: Write,
     {
         self.container(structure, item, "vtx:text", |w| {
             w.with_attribute(("type", text.type_.as_xml_str()))
-                .write_text_content(BytesText::new(&text.content.display(()).to_string()))
-                .unwrap();
-        });
+                .write_text_content(BytesText::new(&text.content.display(()).to_string()))?;
+            Ok(())
+        })
     }
 
-    fn write_item<X>(&mut self, item: &Item, structure: &mut XmlWriter<X>)
+    fn write_item<X>(&mut self, item: &Item, structure: &mut XmlWriter<X>) -> Result<(), Error>
     where
         X: Write,
     {
@@ -195,17 +213,18 @@ where
                         w.create_element("label")
                             .write_text_content(BytesText::new(&item.label()))?;
                         for child in &children.0 {
-                            self.write_item(&child, w);
+                            self.write_item(&child, w).map_err(std::io::Error::other)?;
                         }
                         Ok(())
-                    })
-                    .unwrap();
+                    })?;
+                Ok(())
             }
             Details::Message(diagnostic) => {
                 self.write_text(item, &Text::from(diagnostic.as_ref()), structure)
             }
             Details::PageBreak => {
                 self.needs_page_break = true;
+                Ok(())
             }
             Details::Table(pivot_table) => self.write_table(item, pivot_table, structure),
             Details::Text(text) => self.write_text(item, text, structure),
@@ -218,9 +237,10 @@ where
         item: &Item,
         inner_elem: &str,
         closure: F,
-    ) where
+    ) -> Result<(), Error>
+    where
         X: Write,
-        F: FnOnce(ElementWriter<X>),
+        F: FnOnce(ElementWriter<X>) -> Result<(), Error>,
     {
         writer
             .create_element("container")
@@ -232,16 +252,15 @@ where
             .write_inner_content(|w| {
                 let mut element = w
                     .create_element("label")
-                    .write_text_content(BytesText::new(&item.label()))
-                    .unwrap()
+                    .write_text_content(BytesText::new(&item.label()))?
                     .create_element(inner_elem);
                 if let Some(command_name) = &item.command_name {
                     element = element.with_attribute(("commandName", command_name.as_str()));
                 };
                 closure(element);
                 Ok(())
-            })
-            .unwrap();
+            })?;
+        Ok(())
     }
 }
 
@@ -540,10 +559,10 @@ impl<W> Writer<W>
 where
     W: Write + Seek,
 {
-    pub fn write(&mut self, item: &Item) {
+    pub fn write(&mut self, item: &Item) -> Result<(), Error> {
         if item.details.is_page_break() {
             self.needs_page_break = true;
-            return;
+            return Ok(());
         }
 
         let mut headings = XmlWriter::new(Cursor::new(Vec::new()));
@@ -569,35 +588,32 @@ where
             ))
             .with_attribute(("xmlns:vtx", "http://xml.spss.com/spss/viewer/viewer-text"))
             .with_attribute(("xmlns:vtb", "http://xml.spss.com/spss/viewer/viewer-table"));
-        element
-            .write_inner_content(|w| {
-                w.create_element("label")
-                    .write_text_content(BytesText::new("Output"))?;
-                if let Some(page_setup) = self.page_setup.take() {
-                    write_page_setup(&page_setup, w)?;
-                }
-                self.write_item(item, w);
-                Ok(())
-            })
-            .unwrap();
+        element.write_inner_content(|w| {
+            w.create_element("label")
+                .write_text_content(BytesText::new("Output"))?;
+            if let Some(page_setup) = self.page_setup.take() {
+                write_page_setup(&page_setup, w)?;
+            }
+            self.write_item(item, w);
+            Ok(())
+        })?;
 
         let headings = headings.into_inner().into_inner();
         let heading_id = self.next_heading_id;
         self.next_heading_id += 1;
-        self.writer
-            .start_file(
-                format!(
-                    "outputViewer{heading_id:010}{}.xml",
-                    if item.details.is_heading() {
-                        "_heading"
-                    } else {
-                        ""
-                    }
-                ),
-                SimpleFileOptions::default(),
-            )
-            .unwrap(); // XXX
-        self.writer.write_all(&headings).unwrap(); // XXX
+        self.writer.start_file(
+            format!(
+                "outputViewer{heading_id:010}{}.xml",
+                if item.details.is_heading() {
+                    "_heading"
+                } else {
+                    ""
+                }
+            ),
+            SimpleFileOptions::default(),
+        )?; // XXX
+        self.writer.write_all(&headings)?; // XXX
+        Ok(())
     }
 }
 
@@ -725,7 +741,9 @@ impl Leaf {
             0u8,
             0u8,
             2u32,
-            data_indexes.next().unwrap() as u32,
+            data_indexes
+                .next()
+                .expect("should have as many data indexes as leaves") as u32,
             0u32,
         )
             .write_le(writer)