diff --git a/gsd-parser/src/lib.rs b/gsd-parser/src/lib.rs index 8068bce..466c247 100644 --- a/gsd-parser/src/lib.rs +++ b/gsd-parser/src/lib.rs @@ -559,3 +559,20 @@ pub fn parse_from_file>(file: P) -> GenericStationDescription { Err(e) => panic!("{}", e), } } + +pub fn parse_from_file_with_warnings>( + file: P, +) -> (GenericStationDescription, Vec) { + use std::io::Read; + + let mut f = std::fs::File::open(file.as_ref()).expect("TODO"); + let mut source_bytes = Vec::new(); + f.read_to_end(&mut source_bytes).expect("TODO"); + let source = String::from_utf8_lossy(&source_bytes); + + let (res, warnings) = parser::parse_with_warnings(file.as_ref(), &source); + match res { + Ok(gsd) => (gsd, warnings), + Err(e) => panic!("{}", e), + } +} diff --git a/gsd-parser/src/parser.rs b/gsd-parser/src/parser.rs index d1d8ca7..fae53b9 100644 --- a/gsd-parser/src/parser.rs +++ b/gsd-parser/src/parser.rs @@ -89,33 +89,57 @@ pub fn parse( file: &std::path::Path, source: &str, ) -> ParseResult { - parse_inner(source).map_err(|e| e.with_path(&file.to_string_lossy())) + parse_inner(source, &mut Vec::new()).map_err(|e| e.with_path(&file.to_string_lossy())) } -fn parse_inner(source: &str) -> ParseResult { +pub fn parse_with_warnings( + file: &std::path::Path, + source: &str, +) -> ( + ParseResult, + Vec, +) { + let mut warnings = Vec::new(); + let file = file.to_string_lossy(); + let res = parse_inner(source, &mut warnings).map_err(|e| e.with_path(&file)); + for w in warnings.iter_mut() { + *w = w.clone().with_path(&file); + } + (res, warnings) +} + +fn parse_inner( + source: &str, + warnings: &mut Vec, +) -> ParseResult { use pest::Parser; let gsd_pairs = gsd_parser::GsdParser::parse(gsd_parser::Rule::gsd, source)? .next() - .expect("pest grammar wrong?"); + .unwrap(); + + let start_pos = gsd_pairs.as_span().start_pos(); let mut gsd = crate::GenericStationDescription::default(); let mut prm_texts = BTreeMap::new(); let mut user_prm_data_definitions = BTreeMap::new(); let mut legacy_prm = Some(crate::UserPrmData::default()); + let mut modular_station_span = None; + let mut max_modules_span = None; + for statement in gsd_pairs.into_inner() { let statement_span = statement.as_span(); match statement.as_rule() { gsd_parser::Rule::prm_text => { let mut content = statement.into_inner(); - let id: u16 = parse_number(content.next().expect("pest grammar wrong?"))?; + let id: u16 = parse_number(content.next().unwrap())?; let mut values = BTreeMap::new(); for value_pairs in content { assert!(value_pairs.as_rule() == gsd_parser::Rule::prm_text_value); let mut iter = value_pairs.into_inner(); - let number = parse_signed_number(iter.next().expect("pest grammar wrong?"))?; - let value = parse_string_literal(iter.next().expect("pest grammar wrong?")); + let number = parse_signed_number(iter.next().unwrap())?; + let value = parse_string_literal(iter.next().unwrap()); assert!(iter.next().is_none()); values.insert(value, number); } @@ -124,18 +148,15 @@ fn parse_inner(source: &str) -> ParseResult { gsd_parser::Rule::ext_user_prm_data => { let mut content = statement.into_inner(); // TODO: actually u32? - let id: u32 = parse_number(content.next().expect("pest grammar wrong?"))?; - let name = parse_string_literal(content.next().expect("pest grammar wrong?")); + let id: u32 = parse_number(content.next().unwrap())?; + let name = parse_string_literal(content.next().unwrap()); - let data_type_pair = content.next().expect("pest grammar wrong?"); + let data_type_pair = content.next().unwrap(); assert_eq!( data_type_pair.as_rule(), gsd_parser::Rule::prm_data_type_name ); - let data_type_rule = data_type_pair - .into_inner() - .next() - .expect("pest grammar wrong?"); + let data_type_rule = data_type_pair.into_inner().next().unwrap(); let data_type = match data_type_rule.as_rule() { gsd_parser::Rule::identifier => { match data_type_rule.as_str().to_lowercase().as_str() { @@ -149,25 +170,19 @@ fn parse_inner(source: &str) -> ParseResult { } } gsd_parser::Rule::bit => { - let bit = parse_number( - data_type_rule - .into_inner() - .next() - .expect("pest grammar wrong?"), - )?; + let bit = parse_number(data_type_rule.into_inner().next().unwrap())?; crate::UserPrmDataType::Bit(bit) } gsd_parser::Rule::bit_area => { let mut content = data_type_rule.into_inner(); - let first_bit = parse_number(content.next().expect("pest grammar wrong?"))?; - let last_bit = parse_number(content.next().expect("pest grammar wrong?"))?; + let first_bit = parse_number(content.next().unwrap())?; + let last_bit = parse_number(content.next().unwrap())?; crate::UserPrmDataType::BitArea(first_bit, last_bit) } _ => unreachable!(), }; - let default_value = - parse_signed_number(content.next().expect("pest grammar wrong?"))?; + let default_value = parse_signed_number(content.next().unwrap())?; let mut constraint = crate::PrmValueConstraint::Unconstrained; let mut text_ref = None; @@ -178,10 +193,8 @@ fn parse_inner(source: &str) -> ParseResult { match rule.as_rule() { gsd_parser::Rule::prm_data_value_range => { let mut content = rule.into_inner(); - let min_value = - parse_signed_number(content.next().expect("pest grammar wrong?"))?; - let max_value = - parse_signed_number(content.next().expect("pest grammar wrong?"))?; + let min_value = parse_signed_number(content.next().unwrap())?; + let max_value = parse_signed_number(content.next().unwrap())?; constraint = crate::PrmValueConstraint::MinMax(min_value, max_value); } gsd_parser::Rule::prm_data_value_set => { @@ -193,9 +206,7 @@ fn parse_inner(source: &str) -> ParseResult { constraint = crate::PrmValueConstraint::Enum(values); } gsd_parser::Rule::prm_text_ref => { - let text_id = parse_number( - rule.into_inner().next().expect("pest grammar wrong?"), - )?; + let text_id = parse_number(rule.into_inner().next().unwrap())?; text_ref = Some( prm_texts .get(&text_id) @@ -314,8 +325,14 @@ fn parse_inner(source: &str) -> ParseResult { let number = parse_number(pairs.next().unwrap())?; let name = parse_string_literal(pairs.next().unwrap()); - #[allow(unused)] - let find_module = + let default_pair = pairs.next().unwrap(); + let default_span = default_pair.as_span(); + let default_ref = parse_number(default_pair)?; + + let value_pair = pairs.next().unwrap(); + let allowed_modules_span = value_pair.as_span(); + + let mut find_module = |reference: u16, slot_ref: &str, slot_num: u8| @@ -325,16 +342,15 @@ fn parse_inner(source: &str) -> ParseResult { return Some(module.clone()); } } - // TODO: Warning management? - // log::warn!("No module with reference {reference} found for slot {slot_num} (\"{slot_ref}\")"); + warnings.push(parse_error( + format!( + "Slot {slot_num} (\"{slot_ref}\"): Allowed module {reference} does not exist?", + ), + allowed_modules_span, + )); None }; - let default_pair = pairs.next().unwrap(); - let default_span = default_pair.as_span(); - let default_ref = parse_number(default_pair)?; - - let value_pair = pairs.next().unwrap(); let allowed_modules = match value_pair.as_rule() { gsd_parser::Rule::slot_value_range => { let mut pairs = value_pair.into_inner(); @@ -361,14 +377,18 @@ fn parse_inner(source: &str) -> ParseResult { let Some(default) = find_module(default_ref, &name, number) else { return Err(parse_error( format!( - "The default module for slot {number} (\"{name}\") with reference {default_ref} is not available", + "Slot {number} (\"{name}\"): Default module with reference {default_ref} is not available", ), default_span, )); }; if !allowed_modules.contains(&default) { - // TODO: Warning management? - // log::warn!("Default module not part of allowed modules?!"); + warnings.push(parse_error( + format!( + "Slot {number} (\"{name}\"): Default module {default_ref} is not listed in allowed range?", + ), + default_span, + )); } let slot = crate::Slot { @@ -471,8 +491,14 @@ fn parse_inner(source: &str) -> ParseResult { gsd.implementation_type = parse_string_literal(value_pair) } // - "modular_station" => gsd.modular_station = parse_bool(value_pair)?, - "max_module" => gsd.max_modules = parse_number(value_pair)?, + "modular_station" => { + modular_station_span = Some(value_pair.as_span()); + gsd.modular_station = parse_bool(value_pair)? + } + "max_module" => { + max_modules_span = Some(value_pair.as_span()); + gsd.max_modules = parse_number(value_pair)? + } "max_input_len" => gsd.max_input_length = parse_number(value_pair)?, "max_output_len" => gsd.max_output_length = parse_number(value_pair)?, "max_data_len" => gsd.max_data_length = parse_number(value_pair)?, @@ -587,13 +613,37 @@ fn parse_inner(source: &str) -> ParseResult { gsd.user_prm_data = prm; } + // When no Max_Module was set, default to 1 + if max_modules_span.is_none() { + gsd.max_modules = 1; + } + // If this is a compact station, only allow one module if !gsd.modular_station { - if !gsd.max_modules == 1 { - // TODO: Warnings + if gsd.max_modules != 1 { + warnings.push(parse_error( + format!( + "Is a compact station but Max_Module is {} instead of 1", + gsd.max_modules + ), + // Either must be present to hit this situation + max_modules_span.or(modular_station_span).unwrap(), + )); } - if !gsd.available_modules.len() == 1 { - // TODO: Warnings + if gsd.available_modules.len() != 1 { + let message = format!( + "Is a compact station but there are {} modules available instead of 1", + gsd.available_modules.len() + ); + + if let Some(span) = modular_station_span { + warnings.push(parse_error(message, span)); + } else { + warnings.push(pest::error::Error::new_from_pos( + pest::error::ErrorVariant::CustomError { message }, + start_pos, + )); + } } gsd.max_modules = 1; } diff --git a/gsd-parser/tests/data/mock.gsd b/gsd-parser/tests/data/mock.gsd index 52a2ccf..c4ed56c 100644 --- a/gsd-parser/tests/data/mock.gsd +++ b/gsd-parser/tests/data/mock.gsd @@ -154,3 +154,7 @@ Ext_User_Prm_Data_Const(0) = 0x05,0x00,0x00 Ext_User_Prm_Data_Ref(1) = 1 Ext_User_Prm_Data_Ref(1) = 2 EndModule + +SlotDefinition +Slot(1) = "Data Interface" 1 2-3 +EndSlotDefinition diff --git a/gsd-parser/tests/parser_panic.rs b/gsd-parser/tests/parser_panic.rs index 6f66dd4..31718d0 100644 --- a/gsd-parser/tests/parser_panic.rs +++ b/gsd-parser/tests/parser_panic.rs @@ -71,3 +71,16 @@ User_Prm_Data_Len = 3 let path = std::path::PathBuf::from(format!("{}", file!())); println!("{}", gsd_parser::parser::parse(&path, source).unwrap_err()); } + +#[test] +fn parse_missing_slot_default_module() { + let source = r#" +#Profibus_DP +SlotDefinition +Slot(1) = "Process Data Interface" 1 1-3 +EndSlotDefinition +"#; + + let path = std::path::PathBuf::from(format!("{}", file!())); + println!("{}", gsd_parser::parser::parse(&path, source).unwrap_err()); +} diff --git a/gsd-parser/tests/parser_warnings.rs b/gsd-parser/tests/parser_warnings.rs new file mode 100644 index 0000000..4847745 --- /dev/null +++ b/gsd-parser/tests/parser_warnings.rs @@ -0,0 +1,100 @@ +#[test] +fn slot_missing_allowed_module() { + let source = r#" +#Profibus_DP +Module = "Module 1" 0x00 +1 +Ext_Module_Prm_Data_Len = 0 +EndModule +SlotDefinition +Slot(1) = "Process Data Interface" 1 1-3 +EndSlotDefinition +"#; + + let path = std::path::PathBuf::from(format!("{}", file!())); + let (res, warnings) = gsd_parser::parser::parse_with_warnings(&path, source); + res.unwrap(); + for warning in warnings.iter() { + eprintln!("{}", warning); + } + assert_eq!(warnings.len(), 2); +} + +#[test] +fn slot_default_module_not_allowed() { + let source = r#" +#Profibus_DP +Modular_Station = 1 +Module = "Module 1" 0x00 +1 +Ext_Module_Prm_Data_Len = 0 +EndModule +Module = "Module 2" 0x00 +2 +Ext_Module_Prm_Data_Len = 0 +EndModule +Module = "Module 3" 0x00 +3 +Ext_Module_Prm_Data_Len = 0 +EndModule +SlotDefinition +Slot(1) = "Process Data Interface" 1 2-3 +EndSlotDefinition +"#; + + let path = std::path::PathBuf::from(format!("{}", file!())); + let (res, warnings) = gsd_parser::parser::parse_with_warnings(&path, source); + res.unwrap(); + for warning in warnings.iter() { + eprintln!("{}", warning); + } + assert_eq!(warnings.len(), 1); +} + +#[test] +fn compact_station_max_modules_not_1() { + let source = r#" +#Profibus_DP +Modular_Station = 0 +Max_Module = 16 +Module = "Module 1" 0x00 +1 +Ext_Module_Prm_Data_Len = 0 +EndModule +"#; + + let path = std::path::PathBuf::from(format!("{}", file!())); + let (res, warnings) = gsd_parser::parser::parse_with_warnings(&path, source); + res.unwrap(); + for warning in warnings.iter() { + eprintln!("{}", warning); + } + assert_eq!(warnings.len(), 1); +} + +#[test] +fn compact_station_too_many_modules() { + let source = r#" +#Profibus_DP +Module = "Module 1" 0x00 +1 +Ext_Module_Prm_Data_Len = 0 +EndModule +Module = "Module 2" 0x00 +2 +Ext_Module_Prm_Data_Len = 0 +EndModule +Module = "Module 3" 0x00 +3 +Ext_Module_Prm_Data_Len = 0 +EndModule +"#; + + let path = std::path::PathBuf::from(format!("{}", file!())); + let (res, warnings) = gsd_parser::parser::parse_with_warnings(&path, source); + res.unwrap(); + for warning in warnings.iter() { + eprintln!("{}", warning); + } + assert_eq!(warnings.len(), 1); +} diff --git a/gsd-parser/tests/regress.rs b/gsd-parser/tests/regress.rs index 57f69a2..807fd50 100644 --- a/gsd-parser/tests/regress.rs +++ b/gsd-parser/tests/regress.rs @@ -7,6 +7,18 @@ fn regress(#[files("tests/data/*.[gG][sS][dD]")] gsd_file: PathBuf) { insta::assert_debug_snapshot!(name.as_ref(), gsd); } +#[rstest::rstest] +fn regress_warnings(#[files("tests/data/*.[gG][sS][dD]")] gsd_file: PathBuf) { + let name = gsd_file.file_stem().unwrap().to_string_lossy().to_string(); + let (_, warnings) = gsd_parser::parse_from_file_with_warnings(gsd_file); + let warnings_text = warnings + .into_iter() + .map(|w| w.with_path(&format!("{name}.gsd")).to_string()) + .collect::>() + .join("\n"); + insta::assert_snapshot!(format!("{name}-WARNINGS"), warnings_text); +} + #[rstest::rstest] fn regress_prm(#[files("tests/data/*.[gG][sS][dD]")] gsd_file: PathBuf) { let name = gsd_file.file_stem().unwrap().to_string_lossy().to_string(); diff --git a/gsd-parser/tests/snapshots/regress__mock-WARNINGS.snap b/gsd-parser/tests/snapshots/regress__mock-WARNINGS.snap new file mode 100644 index 0000000..67ea7ab --- /dev/null +++ b/gsd-parser/tests/snapshots/regress__mock-WARNINGS.snap @@ -0,0 +1,22 @@ +--- +source: gsd-parser/tests/regress.rs +expression: warnings_text +--- + --> mock.gsd:159:30 + | +159 | Slot(1) = "Data Interface" 1 2-3 + | ^-^ + | + = Slot 1 ("Data Interface"): Allowed module 2 does not exist? + --> mock.gsd:159:30 + | +159 | Slot(1) = "Data Interface" 1 2-3 + | ^-^ + | + = Slot 1 ("Data Interface"): Allowed module 3 does not exist? + --> mock.gsd:159:28 + | +159 | Slot(1) = "Data Interface" 1 2-3 + | ^ + | + = Slot 1 ("Data Interface"): Default module 1 is not listed in allowed range? diff --git a/gsd-parser/tests/snapshots/regress__mock.snap b/gsd-parser/tests/snapshots/regress__mock.snap index 05ccb45..39aff2d 100644 --- a/gsd-parser/tests/snapshots/regress__mock.snap +++ b/gsd-parser/tests/snapshots/regress__mock.snap @@ -132,7 +132,14 @@ GenericStationDescription { }, }, ], - slots: [], + slots: [ + Slot { + name: "Data Interface", + number: 1, + default: "FROBNICATOR 1 byte + 16 word I/O", + allowed_modules: [], + }, + ], user_prm_data: UserPrmData { length: 0, data_const: [ diff --git a/gsdtool/src/main.rs b/gsdtool/src/main.rs index 1de5610..003a4a4 100644 --- a/gsdtool/src/main.rs +++ b/gsdtool/src/main.rs @@ -67,7 +67,17 @@ fn main() { } fn run_config_wizard(args: &ConfigWizardOptions) { - let gsd = gsd_parser::parse_from_file(&args.gsd_path); + let (gsd, warnings) = gsd_parser::parse_from_file_with_warnings(&args.gsd_path); + + if !warnings.is_empty() { + eprintln!("{}", style("Warnings while parsing the GSD file!").yellow()); + + for warning in warnings.into_iter() { + eprintln!("{}", warning); + } + + eprintln!(); + } println!( "{}",