From f33dfa9b83e9fa32d95d9ad2a70beadce5a34ece Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sun, 13 Jul 2025 13:44:30 -0700 Subject: [PATCH] get rid of TBD errors --- rust/pspp/src/sys/cooked.rs | 67 +++++++++++++++++++++----------- rust/pspp/src/sys/raw.rs | 9 +++-- rust/pspp/src/sys/raw/records.rs | 4 +- 3 files changed, 52 insertions(+), 28 deletions(-) diff --git a/rust/pspp/src/sys/cooked.rs b/rust/pspp/src/sys/cooked.rs index 2c87f74322..972741b35d 100644 --- a/rust/pspp/src/sys/cooked.rs +++ b/rust/pspp/src/sys/cooked.rs @@ -191,12 +191,18 @@ pub enum Error { #[error("Ignoring long string value label for numeric variable {0}.")] LongStringValueLabelNumericVariable(Identifier), + #[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), + #[error("Unknown short name {0} in long variable name record.")] + UnknownShortName(Identifier), + #[error("Invalid name in long variable name record. {0}")] InvalidLongName(IdError), @@ -206,6 +212,9 @@ pub enum Error { #[error("Invalid variable name in very long string record. {0}")] InvalidLongStringName(IdError), + #[error("Very long string entry for unknown variable {0}.")] + UnknownVeryLongString(Identifier), + #[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 }, @@ -268,6 +277,9 @@ pub enum Error { #[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 }, + #[error("Long string missing values for variable {0} are too wide.")] + MissingValuesTooWide(Identifier), + #[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 { offset: u64, @@ -303,8 +315,10 @@ pub enum Error { variable: Identifier, }, - #[error("Details TBD (cooked)")] - TBD, + #[error( + "Dictionary has {expected} variables but {actual} variable display entries are present." + )] + WrongNumberOfVarDisplay { expected: usize, actual: usize }, } #[derive(Clone, Debug)] @@ -737,19 +751,22 @@ impl Headers { } if let Some(display) = &self.var_display { - for (index, display) in display.0.iter().enumerate() { - if let Some(variable) = dictionary.variables.get_index_mut2(index) { - if let Some(width) = display.width { - variable.display_width = width; - } - if let Some(alignment) = display.alignment { - variable.alignment = alignment; - } - if let Some(measure) = display.measure { - variable.measure = Some(measure); - } - } else { - warn(dbg!(Error::TBD)); + if display.0.len() != dictionary.variables.len() { + warn(Error::WrongNumberOfVarDisplay { + expected: dictionary.variables.len(), + actual: display.0.len(), + }); + } + for (display, index) in display.0.iter().zip(0..dictionary.variables.len()) { + let variable = dictionary.variables.get_index_mut2(index).unwrap(); + if let Some(width) = display.width { + variable.display_width = width; + } + if let Some(alignment) = display.alignment { + variable.alignment = alignment; + } + if let Some(measure) = display.measure { + variable.measure = Some(measure); } } } @@ -774,26 +791,26 @@ impl Headers { .flat_map(|record| record.0.into_iter()) { let Some(index) = dictionary.variables.get_index_of(&record.short_name.0) else { - warn(dbg!(Error::TBD)); + warn(Error::UnknownVeryLongString(record.short_name.clone())); continue; }; let width = VarWidth::String(record.length); let n_segments = width.n_segments(); if n_segments == 1 { - warn(dbg!(Error::ShortVeryLongString { + warn(Error::ShortVeryLongString { short_name: record.short_name.clone(), width: record.length - })); + }); continue; } if index + n_segments > dictionary.variables.len() { - warn(dbg!(Error::VeryLongStringOverflow { + warn(Error::VeryLongStringOverflow { short_name: record.short_name.clone(), width: record.length, index, n_segments, len: dictionary.variables.len() - })); + }); continue; } let mut short_names = Vec::with_capacity(n_segments); @@ -855,7 +872,7 @@ impl Headers { .unwrap() .short_names = vec![short_name]; } else { - warn(dbg!(Error::TBD)); + warn(Error::UnknownShortName(short_name.clone())); } } } @@ -871,7 +888,9 @@ impl Headers { { variable.attributes.append(&mut attr_set.attributes); } else { - warn(dbg!(Error::TBD)); + warn(Error::UnknownAttributeVariableName( + attr_set.long_var_name.clone(), + )); } } @@ -947,7 +966,9 @@ impl Headers { .collect::>(); match MissingValues::new(values, None) { Ok(missing_values) => variable.missing_values = missing_values, - Err(MissingValuesError::TooWide) => warn(dbg!(Error::TBD)), + Err(MissingValuesError::TooWide) => { + warn(Error::MissingValuesTooWide(record.var_name.clone())) + } Err(MissingValuesError::TooMany) | Err(MissingValuesError::MixedTypes) => { unreachable!() } diff --git a/rust/pspp/src/sys/raw.rs b/rust/pspp/src/sys/raw.rs index bc599c5af7..0623d374e2 100644 --- a/rust/pspp/src/sys/raw.rs +++ b/rust/pspp/src/sys/raw.rs @@ -432,6 +432,9 @@ pub enum WarningDetails { #[error("Attribute for {name}[{}] missing quotations.", index + 1)] AttributeMissingQuotes { name: Identifier, index: usize }, + #[error("Variable attribute missing `:`.")] + VariableAttributeMissingColon, + #[error("Duplicate attributes for variable {variable}: {}.", attributes.iter().join(", "))] DuplicateVariableAttributes { variable: Identifier, @@ -441,6 +444,9 @@ pub enum WarningDetails { #[error("Duplicate dataset attributes with names: {}.", attributes.iter().join(", "))] DuplicateFileAttributes { attributes: Vec }, + #[error("File attributes record contains trailing garbage.")] + FileAttributesTrailingGarbage, + #[error("Compression bias is {0} instead of the usual values of 0 or 100.")] UnexpectedBias(f64), @@ -461,9 +467,6 @@ pub enum WarningDetails { actual: u32, max_expected: u32, }, - - #[error("Details TBD (raw)")] - TBD, } impl From for WarningDetails { diff --git a/rust/pspp/src/sys/raw/records.rs b/rust/pspp/src/sys/raw/records.rs index bf7e7cdfc8..996762be49 100644 --- a/rust/pspp/src/sys/raw/records.rs +++ b/rust/pspp/src/sys/raw/records.rs @@ -1541,7 +1541,7 @@ impl RawFileAttributesRecord { if !rest.is_empty() { decoder.warn(Warning::new( Some(self.0.offsets.clone()), - WarningDetails::TBD, + WarningDetails::FileAttributesTrailingGarbage, )); } FileAttributesRecord(set) @@ -1564,7 +1564,7 @@ impl VarAttributes { input: &'a str, ) -> Result<(VarAttributes, &'a str), WarningDetails> { let Some((long_var_name, rest)) = input.split_once(':') else { - return Err(dbg!(WarningDetails::TBD)); + return Err(WarningDetails::VariableAttributeMissingColon); }; let long_var_name = decoder .new_identifier(long_var_name) -- 2.30.2