refactor: enhance error handling, validation in parsing methods

Author: nyurik

Cargo.toml

@@ -24,7 +24,7 @@ serde = ["dep:serde"]
 with-serde = ["serde"]
 
 [dependencies]
-can-dbc-pest = { version = "0.5.0" }
+can-dbc-pest = { version = "0.5.1" }
 encoding_rs = { version = "0.8", optional = true }
 serde = { version = "1.0", features = ["derive"], optional = true }
 thiserror = "2.0.17"

src/ast/access_node.rs

@@ -1,6 +1,6 @@
 use can_dbc_pest::{Pair, Rule};
 
-use crate::DbcError;
+use crate::parser::{validated, DbcError};
 
 // TODO: consider merging with Transmitter
 
@@ -15,16 +15,11 @@ impl TryFrom<Pair<'_, Rule>> for AccessNode {
     type Error = DbcError;
 
     fn try_from(pair: Pair<'_, Rule>) -> Result<Self, Self::Error> {
-        match pair.as_rule() {
-            Rule::node_name => {
-                let name = pair.as_str();
-                Ok(if name == "VECTOR__XXX" {
-                    Self::VectorXXX
-                } else {
-                    Self::Name(name.to_string())
-                })
-            }
-            _ => Err(Self::Error::ParseError),
-        }
+        let value = validated(pair, Rule::node_name)?.as_str();
+        Ok(if value == "VECTOR__XXX" {
+            Self::VectorXXX
+        } else {
+            Self::Name(value.to_string())
+        })
     }
 }

src/ast/access_type.rs

@@ -1,6 +1,6 @@
 use can_dbc_pest::{Pair, Rule};
 
-use crate::parser::{parse_uint, single_rule};
+use crate::parser::{parse_uint, single_inner, validated};
 use crate::DbcError;
 
 #[derive(Copy, Clone, Debug, PartialEq)]
@@ -16,18 +16,18 @@ impl TryFrom<Pair<'_, Rule>> for AccessType {
     type Error = DbcError;
 
     fn try_from(pair: Pair<'_, Rule>) -> Result<Self, Self::Error> {
-        match pair.as_rule() {
-            Rule::access_type => {
-                match parse_uint(single_rule(pair, Rule::uint)?)? {
-                    0 => Ok(Self::DummyNodeVector0),
-                    1 => Ok(Self::DummyNodeVector1),
-                    2 => Ok(Self::DummyNodeVector2),
-                    3 => Ok(Self::DummyNodeVector3),
-                    // FIXME: is this correct?
-                    _ => Ok(AccessType::DummyNodeVector0),
-                }
-            }
-            _ => Err(Self::Error::ParseError),
-        }
+        let value = parse_uint(single_inner(
+            validated(pair, Rule::access_type)?,
+            Rule::uint,
+        )?)?;
+
+        Ok(match value {
+            0 => Self::DummyNodeVector0,
+            1 => Self::DummyNodeVector1,
+            2 => Self::DummyNodeVector2,
+            3 => Self::DummyNodeVector3,
+            // FIXME: is this correct?
+            _ => AccessType::DummyNodeVector0,
+        })
     }
 }

src/ast/attribute_default.rs

@@ -1,7 +1,7 @@
 use can_dbc_pest::{Pair, Rule};
 
 use crate::ast::AttributeValue;
-use crate::parser::{expect_empty, next_rule, parse_str, DbcError, DbcResult};
+use crate::parser::{expect_empty, inner_str, next, next_rule, DbcError};
 
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
@@ -10,12 +10,13 @@ pub struct AttributeDefault {
     pub value: AttributeValue,
 }
 
-impl AttributeDefault {
-    /// Parse attribute default: `BA_DEF_DEF_ attribute_name default_value;`
-    pub(crate) fn parse(pair: Pair<Rule>) -> DbcResult<Self> {
+impl TryFrom<Pair<'_, Rule>> for AttributeDefault {
+    type Error = DbcError;
+
+    fn try_from(pair: Pair<'_, Rule>) -> Result<Self, Self::Error> {
         let mut pairs = pair.into_inner();
-        let name = parse_str(next_rule(&mut pairs, Rule::attribute_name)?);
-        let value = pairs.next().ok_or(DbcError::ParseError)?.try_into()?;
+        let name = inner_str(next_rule(&mut pairs, Rule::attribute_name)?);
+        let value = next(&mut pairs)?.try_into()?;
         expect_empty(&mut pairs)?;
 
         Ok(Self { name, value })

src/ast/attribute_definition.rs

@@ -1,6 +1,6 @@
 use can_dbc_pest::{Pair, Rule};
 
-use crate::parser::DbcResult;
+use crate::parser::DbcError;
 
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
@@ -16,15 +16,21 @@ pub enum AttributeDefinition {
     Plain(String),
 }
 
-impl AttributeDefinition {
+impl TryFrom<Pair<'_, Rule>> for AttributeDefinition {
+    type Error = DbcError;
+
     /// Parse attribute definition: `BA_DEF_ [object_type] attribute_name attribute_type [min max];`
-    pub(crate) fn parse(pair: Pair<Rule>) -> DbcResult<Self> {
+    fn try_from(pair: Pair<'_, Rule>) -> Result<Self, Self::Error> {
+        let mut pairs = pair.into_inner();
         let mut definition_string = String::new();
         let mut object_type = "";
 
-        for pairs in pair.into_inner() {
-            match pairs.as_rule() {
-                Rule::object_type => object_type = pairs.as_str(),
+        // Process all pairs
+        while let Some(pair) = pairs.next() {
+            match pair.as_rule() {
+                Rule::object_type => {
+                    object_type = pair.as_str();
+                }
                 Rule::attribute_name
                 | Rule::attribute_type_int
                 | Rule::attribute_type_hex
@@ -34,9 +40,9 @@ impl AttributeDefinition {
                     if !definition_string.is_empty() {
                         definition_string.push(' ');
                     }
-                    definition_string.push_str(pairs.as_str());
+                    definition_string.push_str(pair.as_str());
                 }
-                _ => panic!("Unexpected rule: {:?}", pairs.as_rule()),
+                _ => return Err(DbcError::ParseError),
             }
         }
 

src/ast/attribute_value.rs

@@ -1,7 +1,6 @@
 use can_dbc_pest::{Pair, Rule};
 
-use crate::parser::{parse_float, parse_str};
-use crate::DbcError;
+use crate::parser::{inner_str, parse_float, DbcError};
 
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
@@ -17,7 +16,7 @@ impl TryFrom<Pair<'_, Rule>> for AttributeValue {
 
     fn try_from(pair: Pair<'_, Rule>) -> Result<Self, Self::Error> {
         match pair.as_rule() {
-            Rule::quoted_str => Ok(Self::String(parse_str(pair))),
+            Rule::quoted_str => Ok(Self::String(inner_str(pair))),
             Rule::number => Ok(Self::Double(parse_float(pair)?)),
             // FIXME: Add u64 and i64 parsing
             _ => Err(Self::Error::ParseError),

src/ast/attribute_value_for_object.rs

@@ -2,7 +2,7 @@ use can_dbc_pest::{Pair, Rule};
 
 use crate::ast::AttributeValuedForObjectType;
 use crate::parser::{
-    expect_empty, next_rule, next_string, parse_float, parse_str, single_rule, DbcResult,
+    expect_empty, inner_str, next_rule, next_string, parse_float, single_inner, DbcError,
 };
 use crate::{AttributeValue, MessageId};
 
@@ -13,9 +13,11 @@ pub struct AttributeValueForObject {
     pub value: AttributeValuedForObjectType,
 }
 
-impl AttributeValueForObject {
+impl TryFrom<Pair<'_, Rule>> for AttributeValueForObject {
+    type Error = DbcError;
+
     /// Parse attribute value: `BA_ attribute_name [object_type] object_name value;`
-    pub(crate) fn parse(pair: Pair<Rule>) -> DbcResult<Self> {
+    fn try_from(pair: Pair<'_, Rule>) -> Result<Self, Self::Error> {
         let mut name = String::new();
         let mut object_type = None;
         let mut message_id: Option<MessageId> = None;
@@ -26,21 +28,21 @@ impl AttributeValueForObject {
 
         for pairs in pair.into_inner() {
             match pairs.as_rule() {
-                Rule::attribute_name => name = parse_str(pairs),
+                Rule::attribute_name => name = inner_str(pairs),
                 // num_str_value is a silent rule, so we get quoted_str or number directly
-                Rule::quoted_str => value = Some(AttributeValue::String(parse_str(pairs))),
+                Rule::quoted_str => value = Some(AttributeValue::String(inner_str(pairs))),
                 Rule::number => value = Some(AttributeValue::Double(parse_float(pairs)?)),
                 Rule::node_var => {
                     object_type = Some(pairs.as_rule());
                     // Parse the node name from the inner pairs
                     // node_var contains: node_literal ~ node_name
                     // node_literal is silent (_), so we get node_name directly
-                    node_name = Some(single_rule(pairs, Rule::node_name)?.as_str().to_string());
+                    node_name = Some(single_inner(pairs, Rule::node_name)?.as_str().to_string());
                 }
                 Rule::msg_var => {
                     object_type = Some(pairs.as_rule());
                     // Parse the message ID from the inner pairs
-                    message_id = Some(single_rule(pairs, Rule::message_id)?.try_into()?);
+                    message_id = Some(single_inner(pairs, Rule::message_id)?.try_into()?);
                 }
                 Rule::signal_var => {
                     object_type = Some(pairs.as_rule());
@@ -55,7 +57,7 @@ impl AttributeValueForObject {
                     // Parse the environment variable name from the inner pairs
                     // env_var contains: env_literal ~ env_var_name
                     // env_literal is silent (_), so we get env_var_name directly
-                    let v = single_rule(pairs, Rule::env_var_name)?;
+                    let v = single_inner(pairs, Rule::env_var_name)?;
                     env_var_name = Some(v.as_str().to_string());
                 }
                 other => panic!("What is this? {other:?}"),

src/ast/baudrate.rs

@@ -1,19 +1,17 @@
 use can_dbc_pest::{Pair, Rule};
 
-use crate::parser::DbcResult;
+use crate::parser::DbcError;
 
 /// Baudrate of network in KBit/s
 #[derive(Copy, Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Baudrate(pub u64);
 
-impl Baudrate {
+impl TryFrom<Pair<'_, Rule>> for Baudrate {
+    type Error = DbcError;
+
     /// Parse bit timing: `BS_: [baud_rate : BTR1 , BTR2 ]`
-    pub(crate) fn parse(pair: Pair<Rule>) -> DbcResult<Vec<Self>> {
-        let pairs = pair.into_inner();
-        if pairs.len() == 0 {
-            return Ok(vec![]);
-        }
+    fn try_from(_pair: Pair<'_, Rule>) -> Result<Self, Self::Error> {
         todo!("Bit timing parsing not implemented yet");
     }
 }

src/ast/comment.rs

@@ -1,7 +1,7 @@
 use can_dbc_pest::{Pair, Rule};
 
 use crate::ast::MessageId;
-use crate::parser::{parse_str, parse_uint, single_rule, DbcResult};
+use crate::parser::{inner_str, parse_uint, single_inner, DbcError};
 
 /// Object comments
 #[derive(Clone, Debug, PartialEq)]
@@ -29,9 +29,11 @@ pub enum Comment {
     },
 }
 
-impl Comment {
+impl TryFrom<Pair<'_, Rule>> for Comment {
+    type Error = DbcError;
+
     /// Parse comment: `CM_ [BU_|BO_|SG_|EV_] object_name "comment_text";`
-    pub(crate) fn parse(pair: Pair<Rule>) -> DbcResult<Option<Self>> {
+    fn try_from(pair: Pair<'_, Rule>) -> Result<Self, Self::Error> {
         let mut comment = String::new();
         let mut message_id = None;
         let mut signal_name = None;
@@ -40,16 +42,19 @@ impl Comment {
 
         for pairs in pair.into_inner() {
             match pairs.as_rule() {
-                Rule::quoted_str => comment = parse_str(pairs),
+                Rule::quoted_str => comment = inner_str(pairs),
                 Rule::msg_var => {
-                    message_id = Some(parse_uint(single_rule(pairs, Rule::message_id)?)? as u32);
+                    message_id = Some(parse_uint(single_inner(pairs, Rule::message_id)?)? as u32);
                 }
                 Rule::node_var => {
-                    node_name = Some(single_rule(pairs, Rule::node_name)?.as_str().to_string());
+                    node_name = Some(single_inner(pairs, Rule::node_name)?.as_str().to_string());
                 }
                 Rule::env_var => {
-                    env_var_name =
-                        Some(single_rule(pairs, Rule::env_var_name)?.as_str().to_string());
+                    env_var_name = Some(
+                        single_inner(pairs, Rule::env_var_name)?
+                            .as_str()
+                            .to_string(),
+                    );
                 }
                 Rule::signal_var => {
                     for sub_pair in pairs.into_inner() {
@@ -85,16 +90,16 @@ impl Comment {
         });
 
         Ok(match (message_id, signal_name, node_name, env_var_name) {
-            (Some(message_id), Some(name), _, _) => Some(Self::Signal {
+            (Some(message_id), Some(name), _, _) => Self::Signal {
                 message_id,
                 name,
                 comment,
-            }),
-            (Some(id), None, _, _) => Some(Self::Message { id, comment }),
-            (_, _, Some(name), _) => Some(Self::Node { name, comment }),
-            (_, _, _, Some(name)) => Some(Self::EnvVar { name, comment }),
-            _ if !comment.is_empty() => Some(Self::Plain { comment }),
-            _ => None,
+            },
+            (Some(id), None, _, _) => Self::Message { id, comment },
+            (_, _, Some(name), _) => Self::Node { name, comment },
+            (_, _, _, Some(name)) => Self::EnvVar { name, comment },
+            _ if !comment.is_empty() => Self::Plain { comment },
+            _ => return Err(DbcError::ParseError),
         })
     }
 }