Merge pull request #30 from mathstuf/error-cleanupHEADmaster

Error cleanup

7 files changed, 112 insertions, 85 deletions

diff --git a/Cargo.toml b/Cargo.toml
index 8f341bf..1f22f6b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -14,7 +14,7 @@ keywords = ["vobject", "icalendar", "calendar", "contacts"]
 
 [dependencies]
 chrono      = { version = "0.4", optional = true }
-failure = "0.1"
+thiserror = "1.0"
 
 [features]
 default         = []
diff --git a/src/component.rs b/src/component.rs
index 8330d18..f6d9ac0 100644
--- a/src/component.rs
+++ b/src/component.rs
@@ -2,7 +2,7 @@ use std::str::FromStr;
 use std::collections::BTreeMap;
 
 use property::Property;
-use parser::Parser;
+use parser::{Parser, ParseErrorReason};
 
 use error::*;
 
@@ -69,29 +69,28 @@ impl Component {
 }
 
 impl FromStr for Component {
-    type Err = VObjectErrorKind;
+    type Err = VObjectError;
 
     /// Same as `vobject::parse_component`
-    fn from_str(s: &str) -> Result<Component> {
+    fn from_str(s: &str) -> VObjectResult<Component> {
         parse_component(s)
     }
 }
 
 /// Parse exactly one component. Trailing data generates errors.
-pub fn parse_component(s: &str) -> Result<Component> {
-    let (rv, new_s) = try!(read_component(s));
+pub fn parse_component(s: &str) -> VObjectResult<Component> {
+    let (rv, new_s) = read_component(s)?;
     if !new_s.is_empty() {
-        let s = format!("Trailing data: `{}`", new_s);
-        return Err(VObjectErrorKind::ParserError(s));
+        return Err(ParseErrorReason::TrailingData(new_s.into()).into());
     }
 
     Ok(rv)
 }
 
 /// Parse one component and return the rest of the string.
-pub fn read_component(s: &str) -> Result<(Component, &str)> {
+pub fn read_component(s: &str) -> VObjectResult<(Component, &str)> {
     let mut parser = Parser::new(s);
-    let rv = try!(parser.consume_component());
+    let rv = parser.consume_component()?;
     let new_s = if parser.eof() {
         ""
     } else {
diff --git a/src/error.rs b/src/error.rs
index b9c7b3a..8b01844 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,17 +1,27 @@
-#[derive(Debug, Clone, Eq, PartialEq, Fail)]
-pub enum VObjectErrorKind {
-    #[fail(display = "Parser error: {}", _0)]
-    ParserError(String),
+use thiserror::Error;
 
-    #[fail(display = "Not a Vcard")]
+use ::parser::ParseErrorReason;
+
+#[derive(Debug, Clone, Error)]
+pub enum VObjectError {
+    #[error("failed to parse: {}", source)]
+    Parse {
+        #[from]
+        source: ParseErrorReason,
+    },
+
+    #[error("Not a Vcard")]
     NotAVCard,
 
-    #[fail(display = "Not a Icalendar: {}", _0)]
+    #[error("Not a Icalendar: {}", _0)]
     NotAnICalendar(String),
 
     #[cfg(feature = "timeconversions")]
-    #[fail(display = "{}", _0)]
-    ChronoError(::chrono::format::ParseError),
+    #[error("failed to parse time")]
+    ChronoError {
+        #[from]
+        source: chrono::format::ParseError,
+    },
 }
 
-pub type Result<T> = ::std::result::Result<T, VObjectErrorKind>;
+pub(crate) type VObjectResult<T> = Result<T, VObjectError>;
diff --git a/src/icalendar.rs b/src/icalendar.rs
index c49e193..4da4802 100644
--- a/src/icalendar.rs
+++ b/src/icalendar.rs
@@ -1,4 +1,3 @@
-use std::result::Result as RResult;
 use std::collections::BTreeMap;
 
 use component::Component;
@@ -23,9 +22,9 @@ impl ICalendar {
     /// Returns an error if the parsed text is not a ICalendar (that means that an error is
     /// returned also if this is a valid Vcard!)
     ///
-    pub fn build(s: &str) -> Result<ICalendar> {
+    pub fn build(s: &str) -> VObjectResult<ICalendar> {
         let c = parse_component(s)?;
-        Self::from_component(c).map_err(|_| VObjectErrorKind::NotAnICalendar(s.to_owned()))
+        Self::from_component(c).map_err(|_| VObjectError::NotAnICalendar(s.to_owned()))
     }
 
     pub fn empty() -> ICalendar {
@@ -45,7 +44,7 @@ impl ICalendar {
     }
 
     /// Wrap a Component into a Vcard object, or don't do it if the Component is not a Vcard.
-    pub fn from_component(c: Component)-> RResult<ICalendar, Component> {
+    pub fn from_component(c: Component)-> Result<ICalendar, Component> {
         if c.name == "VCALENDAR" {
             Ok(ICalendar(c))
         } else {
@@ -99,7 +98,7 @@ impl<'a> EventIterator<'a> {
 }
 
 impl<'a> Iterator for EventIterator<'a> {
-    type Item = RResult<Event<'a>, &'a Component>;
+    type Item = Result<Event<'a>, &'a Component>;
 
     fn next(&mut self) -> Option<Self::Item> {
         self.0.next().map(Event::from_component)
@@ -111,7 +110,7 @@ impl<'a> Iterator for EventIterator<'a> {
 pub struct Event<'a>(&'a Component);
 
 impl<'a> Event<'a> {
-    fn from_component(c: &'a Component) -> RResult<Event<'a>, &'a Component> {
+    fn from_component(c: &'a Component) -> Result<Event<'a>, &'a Component> {
         if c.name == "VEVENT" {
             Ok(Event(c))
         } else {
@@ -160,19 +159,18 @@ pub enum Time {
 
 #[cfg(feature = "timeconversions")]
 pub trait AsDateTime {
-    fn as_datetime(&self) -> Result<Time>;
+    fn as_datetime(&self) -> VObjectResult<Time>;
 }
 
 #[cfg(feature = "timeconversions")]
 impl AsDateTime for Dtend {
 
-    fn as_datetime(&self) -> Result<Time> {
-        match NaiveDateTime::parse_from_str(&self.0, DATE_TIME_FMT) {
-            Ok(dt) => Ok(Time::DateTime(dt)),
+    fn as_datetime(&self) -> VObjectResult<Time> {
+        Ok(match NaiveDateTime::parse_from_str(&self.0, DATE_TIME_FMT) {
+            Ok(dt) => Time::DateTime(dt),
             Err(_) => NaiveDate::parse_from_str(&self.0, DATE_FMT)
-                .map(Time::Date)
-                .map_err(VObjectErrorKind::ChronoError),
-        }
+                .map(Time::Date)?,
+        })
     }
 
 }
@@ -180,13 +178,12 @@ impl AsDateTime for Dtend {
 #[cfg(feature = "timeconversions")]
 impl AsDateTime for Dtstart {
 
-    fn as_datetime(&self) -> Result<Time> {
-        match NaiveDateTime::parse_from_str(&self.0, DATE_TIME_FMT) {
-            Ok(dt) => Ok(Time::DateTime(dt)),
+    fn as_datetime(&self) -> VObjectResult<Time> {
+        Ok(match NaiveDateTime::parse_from_str(&self.0, DATE_TIME_FMT) {
+            Ok(dt) => Time::DateTime(dt),
             Err(_) => NaiveDate::parse_from_str(&self.0, DATE_FMT)
-                .map(Time::Date)
-                .map_err(VObjectErrorKind::ChronoError),
-        }
+                .map(Time::Date)?,
+        })
     }
 
 }
@@ -194,13 +191,12 @@ impl AsDateTime for Dtstart {
 #[cfg(feature = "timeconversions")]
 impl AsDateTime for Dtstamp {
 
-    fn as_datetime(&self) -> Result<Time> {
-        match NaiveDateTime::parse_from_str(&self.0, DATE_TIME_FMT) {
-            Ok(dt) => Ok(Time::DateTime(dt)),
+    fn as_datetime(&self) -> VObjectResult<Time> {
+        Ok(match NaiveDateTime::parse_from_str(&self.0, DATE_TIME_FMT) {
+            Ok(dt) => Time::DateTime(dt),
             Err(_) => NaiveDate::parse_from_str(&self.0, DATE_FMT)
-                .map(Time::Date)
-                .map_err(VObjectErrorKind::ChronoError),
-        }
+                .map(Time::Date)?,
+        })
     }
 
 }
diff --git a/src/lib.rs b/src/lib.rs
index 1aadb8e..4bd1193 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,9 +1,8 @@
-#[macro_use]
-extern crate failure;
-
 #[cfg(feature = "timeconversions")]
 extern crate chrono;
 
+extern crate thiserror;
+
 #[macro_use] pub mod param;
 #[macro_use] mod util;
 
diff --git a/src/parser.rs b/src/parser.rs
index 808bc62..8cba634 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -1,8 +1,32 @@
 use std::collections::BTreeMap;
+use std::fmt;
+
+use thiserror::Error;
 
 use component::Component;
 use property::Property;
-use error::*;
+
+#[derive(Debug, Clone, Error)]
+pub enum ParseErrorReason {
+    #[error("trailing data: {}", _0)]
+    TrailingData(String),
+    #[error("expected {}, found EOL", _0)]
+    UnexpectedEol(char),
+    #[error("expected {}, found {}", _0, _1)]
+    UnexpectedChar(char, char),
+    #[error("expected EOL")]
+    ExpectedEol,
+    #[error("no property name found")]
+    NoPropertyName,
+    #[error("no parameter name found")]
+    NoParameterName,
+    #[error("expected BEGIN tag")]
+    ExpectedBegin,
+    #[error("mismatched tags: BEGIN:{} vs END:{}", _0, _1)]
+    MismatchedTag(String, String),
+}
+
+type ParseResult<T> = Result<T, ParseErrorReason>;
 
 pub struct Parser<'s> {
     pub input: &'s str,
@@ -55,16 +79,16 @@ impl<'s> Parser<'s> {
         self.pos >= self.input.len()
     }
 
-    fn assert_char(&self, c: char) -> Result<()> {
+    fn assert_char(&self, c: char) -> ParseResult<()> {
         let real_c = match self.peek() {
             Some((x, _)) => x,
             None => {
-                return Err(VObjectErrorKind::ParserError(format!("Expected {}, found EOL", c)))
+                return Err(ParseErrorReason::UnexpectedEol(c))
            }
         };
 
         if real_c != c {
-            return Err(VObjectErrorKind::ParserError(format!("Expected {}, found {}", c, real_c)))
+            return Err(ParseErrorReason::UnexpectedChar(c, real_c))
         };
 
         Ok(())
@@ -86,7 +110,7 @@ impl<'s> Parser<'s> {
         }
     }
 
-    fn consume_eol(&mut self) -> Result<()> {
+    fn consume_eol(&mut self) -> ParseResult<()> {
         let start_pos = self.pos;
 
         let consumed = match self.consume_char() {
@@ -102,13 +126,13 @@ impl<'s> Parser<'s> {
             Ok(())
         } else {
             self.pos = start_pos;
-            return Err(VObjectErrorKind::ParserError("Expected EOL.".to_owned()))
+            return Err(ParseErrorReason::ExpectedEol)
         }
     }
 
-    fn sloppy_terminate_line(&mut self) -> Result<()> {
+    fn sloppy_terminate_line(&mut self) -> ParseResult<()> {
         if !self.eof() {
-            try!(self.consume_eol());
+            self.consume_eol()?;
             while let Ok(_) = self.consume_eol() {}
         };
 
@@ -150,15 +174,15 @@ impl<'s> Parser<'s> {
         res
     }
 
-    pub fn consume_property(&mut self) -> Result<Property> {
+    pub fn consume_property(&mut self) -> ParseResult<Property> {
         let group = self.consume_property_group().ok();
-        let name = try!(self.consume_property_name());
+        let name = self.consume_property_name()?;
         let params = self.consume_params();
 
-        try!(self.assert_char(':'));
+        self.assert_char(':')?;
         self.consume_char();
 
-        let value = try!(self.consume_property_value());
+        let value = self.consume_property_value()?;
 
         Ok(Property {
             name: name,
@@ -168,16 +192,16 @@ impl<'s> Parser<'s> {
         })
     }
 
-    fn consume_property_name(&mut self) -> Result<String> {
+    fn consume_property_name(&mut self) -> ParseResult<String> {
         let rv = self.consume_while(|x| x == '-' || x.is_alphanumeric());
         if rv.is_empty() {
-            Err(VObjectErrorKind::ParserError("No property name found.".to_owned()))
+            Err(ParseErrorReason::NoPropertyName)
         } else {
             Ok(rv)
         }
     }
 
-    fn consume_property_group(&mut self) -> Result<String> {
+    fn consume_property_group(&mut self) -> ParseResult<String> {
         let start_pos = self.pos;
         let name = self.consume_property_name();
 
@@ -196,18 +220,18 @@ impl<'s> Parser<'s> {
         e
     }
 
-    fn consume_property_value(&mut self) -> Result<String> {
+    fn consume_property_value(&mut self) -> ParseResult<String> {
         let rv = self.consume_while(|x| x != '\r' && x != '\n');
-        try!(self.sloppy_terminate_line());
+        self.sloppy_terminate_line()?;
         Ok(rv)
     }
 
-    fn consume_param_name(&mut self) -> Result<String> {
+    fn consume_param_name(&mut self) -> ParseResult<String> {
         self.consume_property_name()
-            .map_err(|e| VObjectErrorKind::ParserError(format!("No param name found: {}", e)))
+            .map_err(|_| ParseErrorReason::NoParameterName)
     }
 
-    fn consume_param_value(&mut self) -> Result<String> {
+    fn consume_param_value(&mut self) -> ParseResult<String> {
         let qsafe = |x| {
             x != '"' &&
             x != '\r' &&
@@ -218,7 +242,7 @@ impl<'s> Parser<'s> {
 
         if self.consume_only_char('"') {
             let rv = self.consume_while(qsafe);
-            try!(self.assert_char('"'));
+            self.assert_char('"')?;
             self.consume_char();
             Ok(rv)
         } else {
@@ -226,8 +250,8 @@ impl<'s> Parser<'s> {
         }
     }
 
-    fn consume_param(&mut self) -> Result<(String, String)> {
-        let name = try!(self.consume_param_name());
+    fn consume_param(&mut self) -> ParseResult<(String, String)> {
+        let name = self.consume_param_name()?;
         let start_pos = self.pos;
         let value = if self.consume_only_char('=') {
             match self.consume_param_value() {
@@ -252,12 +276,12 @@ impl<'s> Parser<'s> {
         rv
     }
 
-    pub fn consume_component(&mut self) -> Result<Component> {
+    pub fn consume_component(&mut self) -> ParseResult<Component> {
         let start_pos = self.pos;
-        let mut property = try!(self.consume_property());
+        let mut property = self.consume_property()?;
         if property.name != "BEGIN" {
             self.pos = start_pos;
-            return Err(VObjectErrorKind::ParserError("Expected BEGIN tag.".to_owned()));
+            return Err(ParseErrorReason::ExpectedBegin);
         };
 
         // Create a component with the name of the BEGIN tag's value
@@ -265,17 +289,14 @@ impl<'s> Parser<'s> {
 
         loop {
             let previous_pos = self.pos;
-            property = try!(self.consume_property());
+            property = self.consume_property()?;
             if property.name == "BEGIN" {
                 self.pos = previous_pos;
-                component.subcomponents.push(try!(self.consume_component()));
+                component.subcomponents.push(self.consume_component()?);
             } else if property.name == "END" {
                 if property.raw_value != component.name {
                     self.pos = start_pos;
-                    let s = format!("Mismatched tags: BEGIN:{} vs END:{}",
-                                    component.name,
-                                    property.raw_value);
-                    return Err(VObjectErrorKind::ParserError(s));
+                    return Err(ParseErrorReason::MismatchedTag(component.name, property.raw_value));
                 }
 
                 break;
@@ -290,7 +311,6 @@ impl<'s> Parser<'s> {
 
 #[cfg(test)]
 mod tests {
-    use error::*;
     use super::Parser;
 
     #[test]
@@ -348,7 +368,7 @@ mod tests {
         // Test for infinite loops as well
         use std::sync::mpsc::{channel, RecvTimeoutError};
         use std::time::Duration;
-        use error::VObjectErrorKind;
+        use super::ParseErrorReason;
         let mut p = Parser {input: "BEGIN:a\nBEGIN:b\nEND:a", pos: 0};
 
         let (tx, rx) = channel();
@@ -358,7 +378,10 @@ mod tests {
             Err(RecvTimeoutError::Timeout) => assert!(false),
             Ok(Err(e)) => {
                 match e {
-                    VObjectErrorKind::ParserError { .. } => assert!(true),
+                    ParseErrorReason::MismatchedTag(begin, end) => {
+                        assert_eq!(begin, "b");
+                        assert_eq!(end, "a");
+                    },
                     _ => assert!(false),
                 }
             },
diff --git a/src/vcard.rs b/src/vcard.rs
index 5a4d980..ec750fa 100644
--- a/src/vcard.rs
+++ b/src/vcard.rs
@@ -22,10 +22,10 @@ impl Vcard {
     /// Returns an error if the parsed text is not a Vcard (that means that an error is returned
     /// also if this is a valid icalendar!)
     ///
-    pub fn build(s: &str) -> Result<Vcard> {
+    pub fn build(s: &str) -> VObjectResult<Vcard> {
         parse_component(s)
             .and_then(|c| {
-                Self::from_component(c).map_err(|_| VObjectErrorKind::NotAVCard)
+                Self::from_component(c).map_err(|_| VObjectError::NotAVCard)
             })
     }
 
@@ -154,7 +154,7 @@ impl VcardBuilder {
         }
     }
 
-    pub fn build(self) -> Result<Vcard> {
+    pub fn build(self) -> VObjectResult<Vcard> {
         let mut v = Vcard::default();
         v.set_properties(self.properties);
         Ok(v)