From 3818aef83e0e2a8b35e3cdbcc6de77fb1288fcd1 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 15 Jul 2025 17:19:15 -0700 Subject: [PATCH] cleanup sys module --- rust/pspp/src/crypto/mod.rs | 7 + rust/pspp/src/format/parse.rs | 2 +- rust/pspp/src/main.rs | 69 +-- rust/pspp/src/output/pivot/look_xml.rs | 2 +- rust/pspp/src/output/pivot/mod.rs | 8 +- rust/pspp/src/sys/cooked.rs | 735 +++++++++++++++++-------- rust/pspp/src/sys/encoding.rs | 3 - rust/pspp/src/sys/mod.rs | 6 +- rust/pspp/src/sys/raw.rs | 53 +- rust/pspp/src/sys/raw/records.rs | 18 +- rust/pspp/src/sys/test.rs | 46 +- 11 files changed, 600 insertions(+), 349 deletions(-) diff --git a/rust/pspp/src/crypto/mod.rs b/rust/pspp/src/crypto/mod.rs index 40ccfe0ca2..8401ef7b72 100644 --- a/rust/pspp/src/crypto/mod.rs +++ b/rust/pspp/src/crypto/mod.rs @@ -107,6 +107,13 @@ where /// This reads enough of the file to verify that it is in the expected /// format and returns an error if it cannot be read or is not the expected /// format. + /// + /// `reader` doesn't need to be [BufRead], and probably should not be. The + /// [EncryptedReader] returned by [unlock] or [unlock_literal] will be + /// [BufRead]. + /// + /// [unlock]: Self::unlock + /// [unlock_literal]: Self::unlock_literal pub fn new(mut reader: R) -> Result { let header = EncryptedHeader::read_le(&mut NoSeek::new(&mut reader)).map_err( diff --git a/rust/pspp/src/format/parse.rs b/rust/pspp/src/format/parse.rs index 0c7c6dee37..d2f6b22bc7 100644 --- a/rust/pspp/src/format/parse.rs +++ b/rust/pspp/src/format/parse.rs @@ -133,7 +133,7 @@ enum ParseErrorKind { TrailingGarbage(String), /// Invalid date. - #[error("{0}")] + #[error(transparent)] InvalidDate(#[from] DateError), /// Invalid zoned decimal (Z) syntax. diff --git a/rust/pspp/src/main.rs b/rust/pspp/src/main.rs index 37a9750158..d82c5d4c2b 100644 --- a/rust/pspp/src/main.rs +++ b/rust/pspp/src/main.rs @@ -17,13 +17,19 @@ use anyhow::{anyhow, Result}; use clap::{Args, Parser, Subcommand, ValueEnum}; use encoding_rs::Encoding; -use pspp::crypto::EncryptedFile; -use pspp::sys::cooked::{Error, Headers, SystemFile}; -use pspp::sys::raw::{infer_encoding, Decoder, Magic, Reader, Record, Warning}; -use std::fs::File; -use std::io::{stdout, BufReader, Write}; -use std::path::{Path, PathBuf}; -use std::str; +use pspp::{ + crypto::EncryptedFile, + sys::{ + raw::{infer_encoding, Decoder, Magic, Reader, Record}, + ReaderOptions, Records, + }, +}; +use std::{ + fs::File, + io::{stdout, BufReader, Write}, + path::{Path, PathBuf}, + str, +}; use thiserror::Error as ThisError; use zeroize::Zeroizing; @@ -77,31 +83,14 @@ struct CsvOptions { } impl Convert { - fn warn(warning: Warning) { + fn warn(warning: anyhow::Error) { eprintln!("warning: {warning}"); } - fn err(error: Error) { - eprintln!("error: {error}"); - } - fn run(self) -> Result<()> { - let mut reader = Reader::new(BufReader::new(File::open(&self.input)?), Self::warn)?; - let records = reader.records().collect::, _>>()?; - let mut decoder = Decoder::with_inferred_encoding(&records, |w| Self::warn(w))?; - let mut decoded_records = Vec::new(); - for record in records { - decoded_records.push(record.decode(&mut decoder)); - } - let headers = Headers::new(decoded_records, &mut |e| Self::err(e))?; - let SystemFile { - dictionary, cases, .. - } = headers.decode( - reader.header().clone().decode(&mut decoder), - reader.cases(), - decoder.encoding, - |e| Self::err(e), - ); + let (dictionary, _, cases) = ReaderOptions::new() + .open_file(&self.input, Self::warn)? + .into_parts(); let writer = match self.output { Some(path) => Box::new(File::create(path)?) as Box, None => Box::new(stdout()), @@ -288,21 +277,15 @@ fn dissect( None => infer_encoding(&records, &mut |e| eprintln!("{e}"))?, }; let mut decoder = Decoder::new(encoding, |e| eprintln!("{e}")); - let mut decoded_records = Vec::new(); - for record in records { - decoded_records.push(record.decode(&mut decoder)); - } - let headers = Headers::new(decoded_records, &mut |e| eprintln!("{e}"))?; - let SystemFile { - dictionary, - metadata, - cases: _, - } = headers.decode( - reader.header().clone().decode(&mut decoder), - reader.cases(), - encoding, - |e| eprintln!("{e}"), - ); + let records = Records::from_raw(records, &mut decoder); + let (dictionary, metadata, _) = records + .decode( + reader.header().clone().decode(&mut decoder), + reader.cases(), + encoding, + |e| eprintln!("{e}"), + ) + .into_parts(); println!("{dictionary:#?}"); println!("{metadata:#?}"); } diff --git a/rust/pspp/src/output/pivot/look_xml.rs b/rust/pspp/src/output/pivot/look_xml.rs index 76e0c0bb8d..adddcac90c 100644 --- a/rust/pspp/src/output/pivot/look_xml.rs +++ b/rust/pspp/src/output/pivot/look_xml.rs @@ -404,7 +404,7 @@ impl FromStr for Dimension { #[derive(ThisError, Debug, PartialEq, Eq)] enum DimensionParseError { /// Invalid number. - #[error("{0}")] + #[error(transparent)] ParseFloatError(ParseFloatError), /// Unknown unit. diff --git a/rust/pspp/src/output/pivot/mod.rs b/rust/pspp/src/output/pivot/mod.rs index f8301aa43c..5d94390b30 100644 --- a/rust/pspp/src/output/pivot/mod.rs +++ b/rust/pspp/src/output/pivot/mod.rs @@ -727,16 +727,16 @@ impl Default for Look { #[derive(ThisError, Debug)] pub enum ParseLookError { - #[error("{0}")] + #[error(transparent)] XmlError(#[from] DeError), - #[error("{0}")] + #[error(transparent)] Utf8Error(#[from] Utf8Error), - #[error("{0}")] + #[error(transparent)] BinError(#[from] BinError), - #[error("{0}")] + #[error(transparent)] IoError(#[from] std::io::Error), } diff --git a/rust/pspp/src/sys/cooked.rs b/rust/pspp/src/sys/cooked.rs index d539511411..63bf26cf04 100644 --- a/rust/pspp/src/sys/cooked.rs +++ b/rust/pspp/src/sys/cooked.rs @@ -14,10 +14,17 @@ // You should have received a copy of the GNU General Public License along with // this program. If not, see . -use std::{collections::BTreeMap, ops::Range}; +use std::{ + collections::BTreeMap, + fs::File, + io::{Read, Seek}, + ops::Range, + path::Path, +}; use crate::{ calendar::date_time_to_pspp, + crypto::EncryptedFile, data::{Datum, RawString}, dictionary::{ Dictionary, InvalidRole, MissingValues, MissingValuesError, MultipleResponseSet, @@ -28,300 +35,549 @@ use crate::{ hexfloat::HexFloat, identifier::{ByIdentifier, Error as IdError, Identifier}, output::pivot::{Group, Value}, - sys::{ - encoding::Error as EncodingError, - raw::{ - self, - records::{ - Compression, DocumentRecord, EncodingRecord, Extension, FileAttributesRecord, - FloatInfoRecord, HeaderRecord, IntegerInfoRecord, LongName, LongNamesRecord, - LongStringMissingValueRecord, LongStringValueLabelRecord, MultipleResponseRecord, - NumberOfCasesRecord, ProductInfoRecord, RawFormat, ValueLabel, ValueLabelRecord, - VarDisplayRecord, VariableAttributesRecord, VariableRecord, VariableSetRecord, - VeryLongStringsRecord, ZHeader, ZTrailer, - }, - Cases, DecodedRecord, RawDatum, RawWidth, + sys::raw::{ + self, infer_encoding, + records::{ + Compression, DocumentRecord, EncodingRecord, Extension, FileAttributesRecord, + FileHeader, FloatInfoRecord, IntegerInfoRecord, LongName, LongNamesRecord, + LongStringMissingValueRecord, LongStringValueLabelRecord, MultipleResponseRecord, + NumberOfCasesRecord, ProductInfoRecord, RawFormat, ValueLabel, ValueLabelRecord, + VarDisplayRecord, VariableAttributesRecord, VariableRecord, VariableSetRecord, + VeryLongStringsRecord, }, + Cases, DecodedRecord, RawDatum, RawWidth, Reader, }, }; +use anyhow::{anyhow, Error as AnyError}; +use binrw::io::BufReader; use chrono::{NaiveDate, NaiveDateTime, NaiveTime}; use encoding_rs::Encoding; use indexmap::set::MutableValues; use itertools::Itertools; use thiserror::Error as ThisError; +/// A warning for decoding [Records] into a [SystemFile]. #[derive(ThisError, Clone, Debug)] pub enum Error { - #[error("Missing header record")] - MissingHeaderRecord, - - #[error("{0}")] - EncodingError(EncodingError), - - #[error("Using default encoding {0}.")] - UsingDefaultEncoding(String), - - #[error("Variable record from offset {:#x} to {:#x} specifies width {width} not in valid range [-1,255).", offsets.start, offsets.end)] - InvalidVariableWidth { offsets: Range, width: i32 }, - - #[error("This file has corrupted metadata written by a buggy version of PSPP. To ensure that other software can read it correctly, save a new copy of the file.")] - InvalidLongMissingValueFormat, - - #[error("File creation date {creation_date} is not in the expected format \"DD MMM YY\" format. Using 01 Jan 1970.")] - InvalidCreationDate { creation_date: String }, - - #[error("File creation time {creation_time} is not in the expected format \"HH:MM:SS\" format. Using midnight.")] - InvalidCreationTime { creation_time: String }, - + /// File creation date is not in the expected format format. + #[error("File creation date {0} is not in the expected format \"DD MMM YY\" format. Using 01 Jan 1970.")] + InvalidCreationDate( + /// Date. + String, + ), + + /// File creation time is not in the expected format. + #[error("File creation time {0} is not in the expected format \"HH:MM:SS\" format. Using midnight.")] + InvalidCreationTime( + /// Time. + String, + ), + + /// Invalid variable name. #[error("{id_error} Renaming variable to {new_name}.")] InvalidVariableName { + /// Identifier error. id_error: IdError, + /// New name. new_name: Identifier, }, + /// Invalid print format. #[error( - "Substituting {new_spec} for invalid print format on variable {variable}. {format_error}" + "Substituting {new_format} for invalid print format on variable {variable}. {format_error}" )] InvalidPrintFormat { - new_spec: Format, + /// New format. + new_format: Format, + /// Variable. variable: Identifier, + /// Underlying error. format_error: FormatError, }, + /// Invalid write format. #[error( - "Substituting {new_spec} for invalid write format on variable {variable}. {format_error}" + "Substituting {new_format} for invalid write format on variable {variable}. {format_error}" )] InvalidWriteFormat { - new_spec: Format, + /// New format. + new_format: Format, + /// Variable. variable: Identifier, + /// Underlying error. format_error: FormatError, }, + /// Renaming variable with duplicate name {duplicate_name} to {new_name}.. #[error("Renaming variable with duplicate name {duplicate_name} to {new_name}.")] DuplicateVariableName { + /// Duplicate name. duplicate_name: Identifier, + /// New name. new_name: Identifier, }, - #[error("Dictionary index {dict_index} is outside valid range [1,{max_index}].")] - InvalidDictIndex { dict_index: usize, max_index: usize }, - - #[error("Dictionary index {0} refers to a long string continuation.")] - DictIndexIsContinuation(usize), - - #[error("At offset {offset:#x}, one or more variable indexes for value labels referred to long string continuation records: {indexes:?}")] - LongStringContinuationIndexes { offset: u64, indexes: Vec }, - + /// Variable index {start_index} is a {width} that should be followed by + /// long string continuation records through index {end_index} (inclusive), + /// but index {error_index} is not a continuation. #[error("Variable index {start_index} is a {width} that should be followed by long string continuation records through index {end_index} (inclusive), but index {error_index} is not a continuation")] MissingLongStringContinuation { + /// Width of variable. width: RawWidth, + /// First variable index. start_index: usize, + /// Last variable index. end_index: usize, + /// Index of error. error_index: usize, }, + /// Invalid long string value labels. #[error( "At offsets {:#x}...{:#x}, record types 3 and 4 may not add value labels to one or more long string variables: {}", .offsets.start, .offsets.end, variables.iter().join(", ") )] InvalidLongStringValueLabels { + /// Range of file offsets. offsets: Range, + /// Variables. variables: Vec, }, - #[error("Variables associated with value label are not all of identical type. Variable {numeric_var} is numeric, but variable {string_var} is string.")] - ValueLabelsDifferentTypes { - numeric_var: Identifier, - string_var: Identifier, - }, - + /// Variable has duplicate value labels. #[error("{variable} has duplicate value labels for the following value(s): {}", values.iter().join(", "))] DuplicateValueLabels { + /// Variable. variable: Identifier, + /// Duplicate values. values: Vec, }, + /// Invalid multiple response set name. #[error("Invalid multiple response set name. {0}")] - InvalidMrSetName(IdError), + InvalidMrSetName( + /// Identifier error. + IdError, + ), + /// Multiple response set includes unknown variable. #[error("Multiple response set {mr_set} includes unknown variable {short_name}.")] UnknownMrSetVariable { + /// Multiple response set name. mr_set: Identifier, + /// Short name of variable. short_name: Identifier, }, + /// Multiple response set {mr_set} includes variable {variable} more than once. #[error("Multiple response set {mr_set} includes variable {variable} more than once.")] DuplicateMrSetVariable { + /// Multiple response set name. mr_set: Identifier, + /// Duplicated variable. variable: Identifier, }, + /// Multiple response set {0} has no variables. #[error("Multiple response set {0} has no variables.")] - EmptyMrSet(Identifier), + EmptyMrSet( + /// Multiple response set name. + Identifier, + ), + /// Multiple response set {0} has only one variable. #[error("Multiple response set {0} has only one variable.")] - OneVarMrSet(Identifier), + OneVarMrSet( + /// Multiple response set name. + Identifier, + ), + /// Multiple response set {0} contains both string and numeric variables. #[error("Multiple response set {0} contains both string and numeric variables.")] - MixedMrSet(Identifier), + MixedMrSet( + /// Multiple response set name. + Identifier, + ), + /// Invalid numeric format for counted value {number} in multiple response set {mr_set}. #[error( "Invalid numeric format for counted value {number} in multiple response set {mr_set}." )] - InvalidMDGroupCountedValue { mr_set: Identifier, number: String }, + InvalidMDGroupCountedValue { + /// Multiple response set name. + mr_set: Identifier, + /// Value that should be numeric. + number: String, + }, + /// Counted value {value} has width {width}, but it must be no wider than + /// {max_width}, the width of the narrowest variable in multiple response + /// set {mr_set}. #[error("Counted value {value} has width {width}, but it must be no wider than {max_width}, the width of the narrowest variable in multiple response set {mr_set}.")] TooWideMDGroupCountedValue { + /// Multiple response set name. mr_set: Identifier, + /// Counted value. value: String, + /// Width of counted value. width: usize, + /// Maximum allowed width of counted value. max_width: u16, }, - #[error("Long string value label for variable {name} has width {width}, which is not in the valid range [{min_width},{max_width}].")] - InvalidLongValueLabelWidth { - name: Identifier, - width: u32, - min_width: u16, - max_width: u16, - }, - + /// Ignoring long string value label for unknown variable {0}. #[error("Ignoring long string value label for unknown variable {0}.")] - UnknownLongStringValueLabelVariable(Identifier), + UnknownLongStringValueLabelVariable( + /// Variable name. + Identifier, + ), + /// Ignoring long string value label for numeric variable {0}. #[error("Ignoring long string value label for numeric variable {0}.")] - LongStringValueLabelNumericVariable(Identifier), + LongStringValueLabelNumericVariable( + /// Variable name. + Identifier, + ), + /// Invalid variable name {0} in variable attribute record. #[error("Invalid variable name {0} in variable attribute record.")] - UnknownAttributeVariableName(Identifier), - - #[error("Invalid attribute name. {0}")] - InvalidAttributeName(IdError), - - #[error("Invalid short name in long variable name record. {0}")] - InvalidShortName(IdError), + UnknownAttributeVariableName( + /// Variable name. + Identifier, + ), + /// Unknown short name {0} in long variable name record. #[error("Unknown short name {0} in long variable name record.")] - UnknownShortName(Identifier), - - #[error("Invalid name in long variable name record. {0}")] - InvalidLongName(IdError), + UnknownShortName( + /// Short variable name. + Identifier, + ), + /// Duplicate long variable name {0}. #[error("Duplicate long variable name {0}.")] - DuplicateLongName(Identifier), - - #[error("Invalid variable name in very long string record. {0}")] - InvalidLongStringName(IdError), + DuplicateLongName( + /// Long variable name. + Identifier, + ), + /// Very long string entry for unknown variable {0}. #[error("Very long string entry for unknown variable {0}.")] - UnknownVeryLongString(Identifier), + UnknownVeryLongString( + /// Variable name. + Identifier, + ), + /// Variable with short name {short_name} listed in very long string record + /// with width {width}, which requires only one segment. #[error("Variable with short name {short_name} listed in very long string record with width {width}, which requires only one segment.")] - ShortVeryLongString { short_name: Identifier, width: u16 }, + ShortVeryLongString { + /// Short variable name. + short_name: Identifier, + /// Invalid width. + width: u16, + }, + /// Variable with short name {short_name} listed in very long string record + /// with width {width} requires string segments for {n_segments} dictionary + /// indexes starting at index {index}, but the dictionary only contains + /// {len} indexes. #[error("Variable with short name {short_name} listed in very long string record with width {width} requires string segments for {n_segments} dictionary indexes starting at index {index}, but the dictionary only contains {len} indexes.")] VeryLongStringOverflow { + /// Short variable name. short_name: Identifier, + /// Width. width: u16, + /// Starting index. index: usize, + /// Expected number of segments. n_segments: usize, + /// Number of indexes in dictionary. len: usize, }, + /// Variable with short name {short_name} listed in very long string record + /// with width {width} has segment {index} of width {actual} (expected + /// {expected}). #[error("Variable with short name {short_name} listed in very long string record with width {width} has segment {index} of width {actual} (expected {expected}).")] VeryLongStringInvalidSegmentWidth { + /// Variable short name. short_name: Identifier, + /// Variable width. width: u16, + /// Variable index. index: usize, + /// Actual width. actual: usize, + /// Expected width. expected: usize, }, - #[error("Invalid variable name in long string value label record. {0}")] - InvalidLongStringValueLabelName(IdError), - - #[error("Invalid variable name in attribute record. {0}")] - InvalidAttributeVariableName(IdError), - - // XXX This is risky because `text` might be arbitarily long. - #[error("Text string contains invalid bytes for {encoding} encoding: {text}")] - MalformedString { encoding: String, text: String }, - + /// File contains multiple {0:?} records. #[error("File contains multiple {0:?} records.")] - MoreThanOne(&'static str), + MoreThanOne( + /// Record name. + &'static str, + ), + /// File designates string variable {name} (index {index}) as weight + /// variable, but weight variables must be numeric. #[error("File designates string variable {name} (index {index}) as weight variable, but weight variables must be numeric.")] - InvalidWeightVar { name: Identifier, index: u32 }, + InvalidWeightVar { + /// Variable name. + name: Identifier, + /// Variable index. + index: u32, + }, + /// File weight variable index {index} is invalid because it exceeds maximum + /// variable index {max_index}. #[error( "File weight variable index {index} is invalid because it exceeds maximum variable index {max_index}." )] - WeightIndexOutOfRange { index: u32, max_index: usize }, + WeightIndexOutOfRange { + /// Variable index. + index: u32, + /// Maximum variable index. + max_index: usize, + }, + /// File weight variable index {index} is invalid because it refers to long + /// string continuation for variable {name}. #[error( "File weight variable index {index} is invalid because it refers to long string continuation for variable {name}." )] - WeightIndexStringContinuation { index: u32, name: Identifier }, + WeightIndexStringContinuation { + /// Variable index. + index: u32, + /// Variable name. + name: Identifier, + }, - #[error("{0}")] - InvalidRole(InvalidRole), + /// Invalid role. + #[error(transparent)] + InvalidRole( + /// Role error. + InvalidRole, + ), + /// File header claims {expected} variable positions but {actual} were read + /// from file. #[error("File header claims {expected} variable positions but {actual} were read from file.")] - WrongVariablePositions { actual: usize, expected: usize }, + WrongVariablePositions { + /// Actual number of variable positions. + actual: usize, + /// Number of variable positions claimed by file header. + expected: usize, + }, + /// Unknown variable name \"{name}\" in long string missing value record. #[error("Unknown variable name \"{name}\" in long string missing value record.")] - LongStringMissingValueUnknownVariable { name: Identifier }, + LongStringMissingValueUnknownVariable { + /// Variable name. + name: Identifier, + }, + /// Invalid long string missing value for {} variable {name}. #[error("Invalid long string missing value for {} variable {name}.", width.display_adjective())] - LongStringMissingValueBadWdith { name: Identifier, width: VarWidth }, + LongStringMissingValueBadWdith { + /// Variable name. + name: Identifier, + /// Variable width. + width: VarWidth, + }, + /// Long string missing values record says variable {name} has {count} + /// missing values, but only 1 to 3 missing values are allowed. #[error("Long string missing values record says variable {name} has {count} missing values, but only 1 to 3 missing values are allowed.")] - LongStringMissingValueInvalidCount { name: Identifier, count: usize }, + LongStringMissingValueInvalidCount { + /// Variable name. + name: Identifier, + /// Claimed number of missing values. + count: usize, + }, + /// Long string missing values for variable {0} are too wide. #[error("Long string missing values for variable {0} are too wide.")] - MissingValuesTooWide(Identifier), - + MissingValuesTooWide( + /// Variable name. + Identifier, + ), + + /// Unknown extension record with subtype {subtype} at offset {offset:#x}, + /// consisting of {count} {size}-byte units. Please feel free to report + /// this as a bug. #[error("Unknown extension record with subtype {subtype} at offset {offset:#x}, consisting of {count} {size}-byte units. Please feel free to report this as a bug.")] UnknownExtensionRecord { + /// Extension record file starting offset. offset: u64, + /// Extension record subtype. subtype: u32, + /// Extension record per-element size. size: u32, + /// Number of elements in extension record. count: u32, }, + /// Floating-point representation indicated by system file ({0}) differs from expected (1). #[error( "Floating-point representation indicated by system file ({0}) differs from expected (1)." )] - UnexpectedFloatFormat(i32), + UnexpectedFloatFormat( + /// Floating-point format. + i32, + ), + /// Integer format indicated by system file ({actual}) differs from + /// expected ({expected}).. #[error( "Integer format indicated by system file ({actual}) differs from expected ({expected})." )] - UnexpectedEndianess { actual: i32, expected: i32 }, + UnexpectedEndianess { + /// Endianness declared by system file. + actual: i32, + /// Actual endianness used in system file. + expected: i32, + }, + /// System file specifies value {actual:?} ({}) as {name} but {expected:?} ({}) was expected.. #[error( "System file specifies value {actual:?} ({}) as {name} but {expected:?} ({}) was expected.", HexFloat(*actual), HexFloat(*expected) )] UnexpectedFloatValue { + /// Actual floating-point value in system file. actual: f64, + /// Expected floating-point value in system file. expected: f64, + /// Name for this special floating-point value. name: &'static str, }, + /// Variable set \"{variable_set}\" includes unknown variable {variable}. #[error("Variable set \"{variable_set}\" includes unknown variable {variable}.")] UnknownVariableSetVariable { + /// Name of variable set. variable_set: String, + /// Variable name. variable: Identifier, }, + /// Dictionary has {expected} variables but {actual} variable display + /// entries are present. #[error( "Dictionary has {expected} variables but {actual} variable display entries are present." )] - WrongNumberOfVarDisplay { expected: usize, actual: usize }, + WrongNumberOfVarDisplay { + /// Expected number of variable-display entries. + expected: usize, + /// Number of variable-display entries actually present. + actual: usize, + }, +} + +/// Options for reading a system file. +#[derive(Default, Clone, Debug)] +pub struct ReaderOptions { + /// Character encoding for text in the system file. + /// + /// If not set, the character encoding will be determined from reading the + /// file, or a default encoding will be used. + pub encoding: Option<&'static Encoding>, + + /// Password to use to unlock an encrypted system file. + /// + /// For an encrypted system file, this must be set to the (encoded or + /// unencoded) password. + /// + /// For a plaintext system file, this must be `None`. + pub password: Option, +} + +impl ReaderOptions { + /// Construct a new `SystemFileReader` without specifying an encoding or + /// password. + pub fn new() -> Self { + Self::default() + } + + /// Causes the file to be read using the specified `encoding`, or with a + /// default if `encoding` is None. + pub fn with_encoding(self, encoding: Option<&'static Encoding>) -> Self { + Self { encoding, ..self } + } + + /// Causes the file to be read by decrypting it with the given `password` or + /// without decrypting if `encoding` is None. + pub fn with_password(self, password: Option) -> Self { + Self { password, ..self } + } + + /// Opens the file at `path`, reporting warnings using `warn`. + pub fn open_file(self, path: P, warn: F) -> Result + where + P: AsRef, + F: FnMut(AnyError), + { + let file = File::open(path)?; + if self.password.is_some() { + // Don't create `BufReader`, because [EncryptedReader] will buffer. + self.open_reader(file, warn) + } else { + self.open_reader(BufReader::new(file), warn) + } + } + + /// Opens the file read from `reader`, reporting warnings using `warn`. + pub fn open_reader(self, reader: R, warn: F) -> Result + where + R: Read + Seek + 'static, + F: FnMut(AnyError), + { + if let Some(password) = &self.password { + Self::open_reader_inner( + EncryptedFile::new(reader)? + .unlock(password.as_bytes()) + .map_err(|_| anyhow!("Incorrect password."))?, + self.encoding, + warn, + ) + } else { + Self::open_reader_inner(reader, self.encoding, warn) + } + } + + fn open_reader_inner( + reader: R, + encoding: Option<&'static Encoding>, + mut warn: F, + ) -> Result + where + R: Read + Seek + 'static, + F: FnMut(AnyError), + { + let mut reader = Reader::new(reader, |warning| warn(warning.into()))?; + let records = reader.records().collect::, _>>()?; + let header = reader.header().clone(); + let cases = reader.cases(); + let encoding = if let Some(encoding) = encoding { + encoding + } else { + infer_encoding(&records, |warning| warn(warning.into()))? + }; + let mut decoder = raw::Decoder::new(encoding, |warning| warn(warning.into())); + let header = header.decode(&mut decoder); + let records = records + .into_iter() + .map(|record| record.decode(&mut decoder)) + .collect::(); + let encoding = decoder.into_encoding(); + + Ok(records.decode(header, cases, encoding, |e| warn(e.into()))) + } } /// The content of an SPSS system file. +#[derive(Debug)] pub struct SystemFile { /// The system file dictionary. pub dictionary: Dictionary, @@ -333,169 +589,188 @@ pub struct SystemFile { pub cases: Cases, } -#[derive(Clone, Debug)] -pub struct Headers { +impl SystemFile { + /// Returns the individual parts of the [SystemFile]. + pub fn into_parts(self) -> (Dictionary, Metadata, Cases) { + (self.dictionary, self.metadata, self.cases) + } +} + +/// Decoded records in a system file, arranged by type. +/// +/// The `Vec` fields are all in order read from the file. +#[derive(Clone, Debug, Default)] +pub struct Records { + /// Variable records. pub variable: Vec>, + + /// Value label records. pub value_label: Vec>, + + /// Document records. pub document: Vec>, - pub integer_info: Option, - pub float_info: Option, - pub var_display: Option, + + /// Integer info record. + pub integer_info: Vec, + + /// Float info record. + pub float_info: Vec, + + /// Variable display record. + pub var_display: Vec, + + /// Multiple response set records. pub multiple_response: Vec>, + + /// Long string value label records. pub long_string_value_labels: Vec>, + + /// Long string missing value records. pub long_string_missing_values: Vec>, - pub encoding: Option, - pub number_of_cases: Option, + + /// Encoding record. + pub encoding: Vec, + + /// Number of cases record. + pub number_of_cases: Vec, + + /// Variable sets records. pub variable_sets: Vec, - pub product_info: Option, + + /// Product info record. + pub product_info: Vec, + + /// Long variable naems records. pub long_names: Vec, + + /// Very long string variable records. pub very_long_strings: Vec, + + /// File attribute records. pub file_attributes: Vec, + + /// Variable attribute records. pub variable_attributes: Vec, - pub other_extension: Vec, - pub end_of_headers: Option, - pub z_header: Option, - pub z_trailer: Option, -} -fn take_first( - mut vec: Vec, - record_name: &'static str, - warn: &mut impl FnMut(Error), -) -> Option { - if vec.len() > 1 { - warn(Error::MoreThanOne(record_name)); - } - vec.drain(..).next() + /// Other extension records. + pub other_extension: Vec, } -impl Headers { - pub fn new( - records: Vec, - mut warn: impl FnMut(Error), - ) -> Result { - let mut variable = Vec::new(); - let mut value_label = Vec::new(); - let mut document = Vec::new(); - let mut integer_info = Vec::new(); - let mut float_info = Vec::new(); - let mut var_display = Vec::new(); - let mut multiple_response = Vec::new(); - let mut long_string_value_labels = Vec::new(); - let mut long_string_missing_values = Vec::new(); - let mut encoding = Vec::new(); - let mut number_of_cases = Vec::new(); - let mut variable_sets = Vec::new(); - let mut product_info = Vec::new(); - let mut long_names = Vec::new(); - let mut very_long_strings = Vec::new(); - let mut file_attributes = Vec::new(); - let mut variable_attributes = Vec::new(); - let mut other_extension = Vec::new(); - let mut end_of_headers = Vec::new(); - let mut z_header = Vec::new(); - let mut z_trailer = Vec::new(); - - for record in records { +impl Extend for Records { + fn extend(&mut self, iter: T) + where + T: IntoIterator, + { + for record in iter { match record { DecodedRecord::Variable(record) => { - variable.push(record); + self.variable.push(record); } DecodedRecord::ValueLabel(record) => { - value_label.push(record); + self.value_label.push(record); } DecodedRecord::Document(record) => { - document.push(record); + self.document.push(record); } DecodedRecord::IntegerInfo(record) => { - integer_info.push(record); + self.integer_info.push(record); } DecodedRecord::FloatInfo(record) => { - float_info.push(record); + self.float_info.push(record); } DecodedRecord::VariableSets(record) => { - variable_sets.push(record); + self.variable_sets.push(record); } DecodedRecord::VarDisplay(record) => { - var_display.push(record); + self.var_display.push(record); } DecodedRecord::MultipleResponse(record) => { - multiple_response.push(record); + self.multiple_response.push(record); } DecodedRecord::LongStringValueLabels(record) => { - long_string_value_labels.push(record) + self.long_string_value_labels.push(record) } DecodedRecord::LongStringMissingValues(record) => { - long_string_missing_values.push(record); + self.long_string_missing_values.push(record); } DecodedRecord::Encoding(record) => { - encoding.push(record); + self.encoding.push(record); } DecodedRecord::NumberOfCases(record) => { - number_of_cases.push(record); + self.number_of_cases.push(record); } DecodedRecord::ProductInfo(record) => { - product_info.push(record); + self.product_info.push(record); } DecodedRecord::LongNames(record) => { - long_names.push(record); + self.long_names.push(record); } DecodedRecord::VeryLongStrings(record) => { - very_long_strings.push(record); + self.very_long_strings.push(record); } DecodedRecord::FileAttributes(record) => { - file_attributes.push(record); + self.file_attributes.push(record); } DecodedRecord::VariableAttributes(record) => { - variable_attributes.push(record); + self.variable_attributes.push(record); } DecodedRecord::OtherExtension(record) => { - other_extension.push(record); - } - DecodedRecord::EndOfHeaders(record) => { - end_of_headers.push(record); - } - DecodedRecord::ZHeader(record) => { - z_header.push(record); - } - DecodedRecord::ZTrailer(record) => { - z_trailer.push(record); + self.other_extension.push(record); } + DecodedRecord::EndOfHeaders(_) + | DecodedRecord::ZHeader(_) + | DecodedRecord::ZTrailer(_) => (), } } + } +} - Ok(Headers { - variable, - value_label, - document, - integer_info: take_first(integer_info, "integer info", &mut warn), - float_info: take_first(float_info, "float info", &mut warn), - var_display: take_first(var_display, "variable display", &mut warn), - multiple_response, - long_string_value_labels, - long_string_missing_values, - encoding: take_first(encoding, "encoding", &mut warn), - number_of_cases: take_first(number_of_cases, "number of cases", &mut warn), - variable_sets, - product_info: take_first(product_info, "product info", &mut warn), - long_names, - very_long_strings, - file_attributes, - variable_attributes, - other_extension, - end_of_headers: take_first(end_of_headers, "end of headers", &mut warn), - z_header: take_first(z_header, "z_header", &mut warn), - z_trailer: take_first(z_trailer, "z_trailer", &mut warn), - }) +impl FromIterator for Records { + fn from_iter(iter: T) -> Self + where + T: IntoIterator, + { + let mut records = Records::default(); + records.extend(iter); + records + } +} + +impl Records { + /// Constructs `Records` from the raw records in `iter`, decoding them with + /// `decoder`. + pub fn from_raw(iter: T, decoder: &mut raw::Decoder) -> Self + where + T: IntoIterator, + { + iter.into_iter() + .map(|record| record.decode(decoder)) + .collect() } + /// Decodes this [Records] along with `header` and `cases` into a + /// [SystemFile]. `encoding` is the encoding that was used to decode these + /// records. Uses `warn` to report warnings. pub fn decode( mut self, - header: HeaderRecord, + header: FileHeader, mut cases: Cases, encoding: &'static Encoding, mut warn: impl FnMut(Error), ) -> SystemFile { + for (count, record_name) in [ + (self.integer_info.len(), "integer info"), + (self.float_info.len(), "float info"), + (self.var_display.len(), "variable display"), + (self.encoding.len(), "encoding"), + (self.number_of_cases.len(), "number of cases"), + (self.product_info.len(), "product info"), + ] { + if count > 1 { + warn(Error::MoreThanOne(record_name)); + } + } + let mut dictionary = Dictionary::new(encoding); let file_label = fix_line_ends(header.file_label.trim_end_matches(' ')); @@ -516,7 +791,7 @@ impl Headers { .map(trim_end_spaces) .collect(); - if let Some(integer_info) = &self.integer_info { + if let Some(integer_info) = self.integer_info.first() { let floating_point_rep = integer_info.floating_point_rep; if floating_point_rep != 1 { warn(Error::UnexpectedFloatFormat(floating_point_rep)) @@ -532,7 +807,7 @@ impl Headers { } }; - if let Some(float_info) = &self.float_info { + if let Some(float_info) = self.float_info.get(0) { for (expected, expected2, actual, name) in [ (f64::MIN, None, float_info.sysmis, "SYSMIS"), (f64::MAX, None, float_info.highest, "HIGHEST"), @@ -558,7 +833,7 @@ impl Headers { if n_vars != nominal_case_size as usize && self .integer_info - .as_ref() + .get(0) .is_none_or(|info| info.version.0 != 13) { warn(Error::WrongVariablePositions { @@ -624,7 +899,7 @@ impl Headers { variable.width, |new_spec, format_error| { warn(Error::InvalidPrintFormat { - new_spec, + new_format: new_spec, variable: variable.name.clone(), format_error, }) @@ -635,7 +910,7 @@ impl Headers { variable.width, |new_spec, format_error| { warn(Error::InvalidWriteFormat { - new_spec, + new_format: new_spec, variable: variable.name.clone(), format_error, }) @@ -753,7 +1028,7 @@ impl Headers { } } - if let Some(display) = &self.var_display { + if let Some(display) = self.var_display.first() { if display.0.len() != dictionary.variables.len() { warn(Error::WrongNumberOfVarDisplay { expected: dictionary.variables.len(), @@ -1057,6 +1332,10 @@ pub struct Metadata { } impl Metadata { + /// Returns a pivot table [Group] and associated [Value]s that describe this + /// `Metadata` if they are put into a [PivotTable]. + /// + /// [PivotTable]: crate::output::pivot::PivotTable pub fn to_pivot_rows(&self) -> (Group, Vec) { let mut group = Group::new("File Information"); let mut values = Vec::new(); @@ -1107,24 +1386,16 @@ impl Metadata { (group, values) } - fn decode( - header: &HeaderRecord, - headers: &Headers, - mut warn: impl FnMut(Error), - ) -> Self { + fn decode(header: &FileHeader, headers: &Records, mut warn: impl FnMut(Error)) -> Self { let header = &header; let creation_date = NaiveDate::parse_from_str(&header.creation_date, "%e %b %y") .unwrap_or_else(|_| { - warn(Error::InvalidCreationDate { - creation_date: header.creation_date.to_string(), - }); + warn(Error::InvalidCreationDate(header.creation_date.to_string())); Default::default() }); let creation_time = NaiveTime::parse_from_str(&header.creation_time, "%H:%M:%S") .unwrap_or_else(|_| { - warn(Error::InvalidCreationTime { - creation_time: header.creation_time.to_string(), - }); + warn(Error::InvalidCreationTime(header.creation_time.to_string())); Default::default() }); let creation = NaiveDateTime::new(creation_date, creation_time); @@ -1141,12 +1412,12 @@ impl Metadata { compression: header.compression, n_cases: headers .number_of_cases - .as_ref() + .first() .map(|record| record.n_cases) .or_else(|| header.n_cases.map(|n| n as u64)), product, - product_ext: headers.product_info.as_ref().map(|pe| fix_line_ends(&pe.0)), - version: headers.integer_info.as_ref().map(|ii| ii.version), + product_ext: headers.product_info.first().map(|pe| fix_line_ends(&pe.0)), + version: headers.integer_info.first().map(|ii| ii.version), } } } diff --git a/rust/pspp/src/sys/encoding.rs b/rust/pspp/src/sys/encoding.rs index 7ca705465d..f4fecbab62 100644 --- a/rust/pspp/src/sys/encoding.rs +++ b/rust/pspp/src/sys/encoding.rs @@ -16,9 +16,6 @@ //! Character encodings in system files. -// Warn about missing docs, but not for items declared with `#[cfg(test)]`. -#![cfg_attr(not(test), warn(missing_docs))] - use std::sync::LazyLock; use crate::locale_charset::locale_charset; diff --git a/rust/pspp/src/sys/mod.rs b/rust/pspp/src/sys/mod.rs index 10b51ced6d..e99770fdce 100644 --- a/rust/pspp/src/sys/mod.rs +++ b/rust/pspp/src/sys/mod.rs @@ -22,7 +22,11 @@ //! facilitate interchange between even the oldest and newest versions of //! software. -pub mod cooked; +// Warn about missing docs, but not for items declared with `#[cfg(test)]`. +#![cfg_attr(not(test), warn(missing_docs))] + +mod cooked; +pub use cooked::*; pub mod encoding; pub mod raw; diff --git a/rust/pspp/src/sys/raw.rs b/rust/pspp/src/sys/raw.rs index 572e6b94ee..997bea3ca9 100644 --- a/rust/pspp/src/sys/raw.rs +++ b/rust/pspp/src/sys/raw.rs @@ -19,9 +19,6 @@ //! This module facilitates reading records from system files in all of their //! raw details. Most readers will want to use higher-level interfaces. -// Warn about missing docs, but not for items declared with `#[cfg(test)]`. -#![cfg_attr(not(test), warn(missing_docs))] - use crate::{ data::{Case, Datum, RawStr, RawString}, dictionary::{VarType, VarWidth}, @@ -32,7 +29,7 @@ use crate::{ encoding::{default_encoding, get_encoding, Error as EncodingError}, raw::records::{ AttributeWarning, Compression, DocumentRecord, EncodingRecord, Extension, - ExtensionWarning, FileAttributesRecord, FloatInfoRecord, HeaderRecord, HeaderWarning, + ExtensionWarning, FileAttributesRecord, FileHeader, FloatInfoRecord, HeaderWarning, IntegerInfoRecord, LongNameWarning, LongNamesRecord, LongStringMissingValueRecord, LongStringMissingValuesWarning, LongStringValueLabelRecord, LongStringValueLabelWarning, MultipleResponseRecord, MultipleResponseWarning, @@ -70,10 +67,10 @@ pub mod records; #[derive(Debug)] pub struct Error { /// Range of file offsets where the error occurred. - offsets: Option>, + pub offsets: Option>, /// Details of the error. - details: ErrorDetails, + pub details: ErrorDetails, } impl std::error::Error for Error {} @@ -117,6 +114,10 @@ pub enum ErrorDetails { #[error("Not an SPSS system file")] NotASystemFile, + /// Encrypted. + #[error("File is encrypted but no password was supplied.")] + Encrypted, + /// Bad [Magic]. #[error("Invalid magic number {0:?}")] BadMagic([u8; 4]), @@ -362,7 +363,7 @@ pub enum ErrorDetails { }, /// Encoding error. - #[error("{0}")] + #[error(transparent)] EncodingError( /// The error. #[from] @@ -377,10 +378,10 @@ pub enum ErrorDetails { #[derive(Debug)] pub struct Warning { /// Range of file offsets where the warning occurred. - offsets: Option>, + pub offsets: Option>, /// Details of the warning. - details: WarningDetails, + pub details: WarningDetails, } impl std::error::Error for Warning {} @@ -484,7 +485,7 @@ pub enum WarningDetails { }, /// Encoding error. - #[error("{0}")] + #[error(transparent)] EncodingError(#[from] EncodingError), } @@ -799,6 +800,11 @@ impl<'de> Decoder<'de> { } } + /// Drops this decoder, returning its encoding. + pub fn into_encoding(self) -> &'static Encoding { + self.encoding + } + fn warn(&mut self, warning: Warning) { (self.warn)(warning) } @@ -1155,17 +1161,6 @@ enum ReaderState { } /// Reads records from a system file in their raw form. -/// -/// This is the lowest-level way to read a system file. To read a system file: -/// -/// 1. Read the raw [Record]s using `Reader`. -/// -/// 2. Create a [Decoder] with an appropriate character encoding. -/// -/// 3. Decode the records into [DecodedRecord]s using the [Decoder]. -/// -/// 4. Assemble the decoded records into [Headers](super::cooked::Headers) -/// and decode them with [Headers::decode](super::cooked::Headers::decode). pub struct Reader<'a, R> where R: Read + Seek + 'static, @@ -1173,7 +1168,7 @@ where reader: Option, warn: Box, - header: HeaderRecord, + header: FileHeader, var_types: VarTypes, state: ReaderState, @@ -1190,7 +1185,7 @@ where /// To read an encrypted system file, wrap `reader` in /// [EncryptedReader](crate::crypto::EncryptedReader). pub fn new(mut reader: R, mut warn: impl FnMut(Warning) + 'a) -> Result { - let header = HeaderRecord::read(&mut reader, &mut warn)?; + let header = FileHeader::read(&mut reader, &mut warn)?; Ok(Self { reader: Some(reader), warn: Box::new(warn), @@ -1202,7 +1197,7 @@ where } /// Returns the header in this reader. - pub fn header(&self) -> &HeaderRecord { + pub fn header(&self) -> &FileHeader { &self.header } @@ -1240,7 +1235,7 @@ where )); } - fn _next(&mut self) -> Option<::Item> { + fn next_inner(&mut self) -> Option<::Item> { match self.0.state { ReaderState::Headers => { let record = loop { @@ -1309,7 +1304,7 @@ where type Item = Result; fn next(&mut self) -> Option { - self._next().inspect(|retval| { + self.next_inner().inspect(|retval| { if retval.is_err() { self.0.state = ReaderState::End; } @@ -1429,7 +1424,7 @@ impl Default for Cases { } impl Cases { - fn new(reader: R, var_types: VarTypes, header: &HeaderRecord) -> Self + fn new(reader: R, var_types: VarTypes, header: &FileHeader) -> Self where R: Read + Seek + 'static, { @@ -1459,8 +1454,8 @@ impl Cases { /// Returns this [Cases] with its notion of variable widths updated from /// `widths`. /// - /// [Headers::decode](super::cooked::Headers::decode) uses this to properly - /// handle very long string variables (see [Cases] for details). + /// [Records::decode](crate::sys::Records::decode) uses this to properly handle + /// very long string variables (see [Cases] for details). pub fn with_widths(self, widths: impl IntoIterator) -> Self { Self { case_vars: widths.into_iter().map(CaseVar::new).collect::>(), diff --git a/rust/pspp/src/sys/raw/records.rs b/rust/pspp/src/sys/raw/records.rs index d1335c23bb..d49437697b 100644 --- a/rust/pspp/src/sys/raw/records.rs +++ b/rust/pspp/src/sys/raw/records.rs @@ -50,7 +50,7 @@ pub enum HeaderWarning { /// A file header record in a system file. #[derive(Clone)] -pub struct HeaderRecord +pub struct FileHeader where S: Debug, { @@ -94,7 +94,7 @@ where pub endian: Endian, } -impl HeaderRecord +impl FileHeader where S: Debug, { @@ -106,7 +106,7 @@ where } } -impl Debug for HeaderRecord +impl Debug for FileHeader where S: Debug, { @@ -127,7 +127,7 @@ where } } -impl HeaderRecord { +impl FileHeader { /// Reads a header record from `r`, reporting any warnings via `warn`. pub fn read(r: &mut R, warn: &mut dyn FnMut(Warning)) -> Result where @@ -166,6 +166,10 @@ impl HeaderRecord { _padding: [u8; 3], } + if &header_bytes[8..20] == b"ENCRYPTEDSAV" { + return Err(ErrorDetails::Encrypted); + } + let be_header = RawHeader::read_be(&mut Cursor::new(&header_bytes)).unwrap(); let le_header = RawHeader::read_le(&mut Cursor::new(&header_bytes)).unwrap(); @@ -209,7 +213,7 @@ impl HeaderRecord { let creation_time = RawString(header.creation_time.into()); let file_label = RawString(header.file_label.into()); - Ok(HeaderRecord { + Ok(FileHeader { magic, layout_code: header.layout_code, nominal_case_size, @@ -226,12 +230,12 @@ impl HeaderRecord { } /// Decodes this record with `decoder` and returns the decoded version. - pub fn decode(self, decoder: &mut Decoder) -> HeaderRecord { + pub fn decode(self, decoder: &mut Decoder) -> FileHeader { let eye_catcher = decoder.decode(&self.eye_catcher).to_string(); let file_label = decoder.decode(&self.file_label).to_string(); let creation_date = decoder.decode(&self.creation_date).to_string(); let creation_time = decoder.decode(&self.creation_time).to_string(); - HeaderRecord { + FileHeader { eye_catcher, weight_index: self.weight_index, n_cases: self.n_cases, diff --git a/rust/pspp/src/sys/test.rs b/rust/pspp/src/sys/test.rs index 3c4e9e97d9..48e95a0f6b 100644 --- a/rust/pspp/src/sys/test.rs +++ b/rust/pspp/src/sys/test.rs @@ -29,8 +29,8 @@ use crate::{ Details, Item, Text, }, sys::{ - cooked::{Headers, SystemFile}, - raw::{infer_encoding, Decoder, Reader}, + cooked::ReaderOptions, + raw::{self, ErrorDetails}, sack::sack, }, }; @@ -551,6 +551,19 @@ fn encrypted_file() { test_encrypted_sysfile("test-encrypted.sav", "pspp"); } +#[test] +fn encrypted_file_without_password() { + let error = ReaderOptions::new() + .open_file("src/crypto/testdata/test-encrypted.sav", |_| { + panic!(); + }) + .unwrap_err(); + assert!(matches!( + error.downcast::().unwrap().details, + ErrorDetails::Encrypted + )); +} + fn test_raw_sysfile(name: &str) { let input_filename = Path::new(env!("CARGO_MANIFEST_DIR")) .join("src/sys/testdata") @@ -602,27 +615,9 @@ where R: Read + Seek + 'static, { let mut warnings = Vec::new(); - let mut reader = Reader::new(sysfile, |warning| warnings.push(warning)).unwrap(); - let output = match reader.records().collect::, _>>() { - Ok(records) => { - let header = reader.header().clone(); - let cases = reader.cases(); - let encoding = infer_encoding(&records, |warning| warnings.push(warning)).unwrap(); - let mut decoder = Decoder::new(encoding, |warning| warnings.push(warning)); - let header = header.decode(&mut decoder); - let decoded_records = records - .into_iter() - .map(|record| record.decode(&mut decoder)) - .collect::>(); - drop(decoder); - - let mut errors = Vec::new(); - let headers = Headers::new(decoded_records, |e| errors.push(e)).unwrap(); - let SystemFile { - dictionary, - metadata, - cases, - } = headers.decode(header, cases, encoding, |e| errors.push(e)); + let output = match ReaderOptions::new().open_reader(sysfile, |warning| warnings.push(warning)) { + Ok(system_file) => { + let (dictionary, metadata, cases) = system_file.into_parts(); let (group, data) = metadata.to_pivot_rows(); let metadata_table = PivotTable::new([(Axis3::Y, Dimension::new(group))]).with_data( data.into_iter() @@ -643,11 +638,6 @@ where .into_iter() .map(|warning| Arc::new(Item::from(Text::new_log(warning.to_string())))), ); - output.extend( - errors - .into_iter() - .map(|error| Arc::new(Item::from(Text::new_log(error.to_string())))), - ); output.push(Arc::new(metadata_table.into())); output.push(Arc::new(dictionary_table.into())); output.push(Arc::new( -- 2.30.2