Merge pull request #225 from yuankunzhang/result-based-error-handling

refactor: replace Option-based error handling with Result-based error handling

Author: cakebaker

src/items/builder.rs

@@ -3,7 +3,7 @@
 
 use jiff::{civil, Span, Zoned};
 
-use super::{date, epoch, relative, time, timezone, weekday, year};
+use super::{date, epoch, error, relative, time, timezone, weekday, year};
 
 /// The builder is used to construct a DateTime object from various components.
 /// The parser creates a `DateTimeBuilder` object with the parsed components,
@@ -148,16 +148,12 @@ impl DateTimeBuilder {
         self.set_time(time)
     }
 
-    pub(super) fn build(self) -> Option<Zoned> {
+    pub(super) fn build(self) -> Result<Zoned, error::Error> {
         let base = self.base.unwrap_or(Zoned::now());
 
         // If a timestamp is set, we use it to build the `Zoned` object.
         if let Some(ts) = self.timestamp {
-            return Some(
-                jiff::Timestamp::try_from(ts)
-                    .ok()?
-                    .to_zoned(base.offset().to_time_zone()),
-            );
+            return Ok(jiff::Timestamp::try_from(ts)?.to_zoned(base.offset().to_time_zone()));
         }
 
         // If any of the following items are set, we truncate the time portion
@@ -170,30 +166,30 @@ impl DateTimeBuilder {
         {
             base
         } else {
-            base.with().time(civil::time(0, 0, 0, 0)).build().ok()?
+            base.with().time(civil::time(0, 0, 0, 0)).build()?
         };
 
         if let Some(date) = self.date {
             let d: civil::Date = if date.year.is_some() {
-                date.try_into().ok()?
+                date.try_into()?
             } else {
-                date.with_year(dt.date().year() as u16).try_into().ok()?
+                date.with_year(dt.date().year() as u16).try_into()?
             };
-            dt = dt.with().date(d).build().ok()?;
+            dt = dt.with().date(d).build()?;
         }
 
         if let Some(time) = self.time.clone() {
             if let Some(offset) = &time.offset {
-                dt = dt.datetime().to_zoned(offset.try_into().ok()?).ok()?;
+                dt = dt.datetime().to_zoned(offset.try_into()?)?;
             }
 
-            let t: civil::Time = time.try_into().ok()?;
-            dt = dt.with().time(t).build().ok()?;
+            let t: civil::Time = time.try_into()?;
+            dt = dt.with().time(t).build()?;
         }
 
         if let Some(weekday::Weekday { offset, day }) = self.weekday {
             if self.time.is_none() {
-                dt = dt.with().time(civil::time(0, 0, 0, 0)).build().ok()?;
+                dt = dt.with().time(civil::time(0, 0, 0, 0)).build()?;
             }
 
             let mut offset = offset;
@@ -228,29 +224,27 @@ impl DateTimeBuilder {
             let delta = (day.since(civil::Weekday::Monday) as i32
                 - dt.date().weekday().since(civil::Weekday::Monday) as i32)
                 .rem_euclid(7)
-                + offset.checked_mul(7)?;
+                + offset.checked_mul(7).ok_or("multiplication overflow")?;
 
-            dt = dt.checked_add(Span::new().try_days(delta).ok()?).ok()?;
+            dt = dt.checked_add(Span::new().try_days(delta)?)?;
         }
 
         for rel in self.relative {
-            dt = dt
-                .checked_add::<Span>(if let relative::Relative::Months(x) = rel {
-                    // *NOTE* This is done in this way to conform to GNU behavior.
-                    let days = dt.date().last_of_month().day() as i32;
-                    Span::new().try_days(days.checked_mul(x)?).ok()?
-                } else {
-                    rel.try_into().ok()?
-                })
-                .ok()?;
+            dt = dt.checked_add::<Span>(if let relative::Relative::Months(x) = rel {
+                // *NOTE* This is done in this way to conform to GNU behavior.
+                let days = dt.date().last_of_month().day() as i32;
+                Span::new().try_days(days.checked_mul(x).ok_or("multiplication overflow")?)?
+            } else {
+                rel.try_into()?
+            })?;
         }
 
         if let Some(offset) = self.timezone {
             let (offset, hour_adjustment) = offset.normalize();
-            dt = dt.checked_add(Span::new().hours(hour_adjustment)).ok()?;
-            dt = dt.datetime().to_zoned((&offset).try_into().ok()?).ok()?;
+            dt = dt.checked_add(Span::new().hours(hour_adjustment))?;
+            dt = dt.datetime().to_zoned((&offset).try_into()?)?;
         }
 
-        Some(dt)
+        Ok(dt)
     }
 }

src/items/error.rs

@@ -0,0 +1,38 @@
+use std::fmt;
+
+use winnow::error::{ContextError, ErrMode};
+
+#[derive(Debug)]
+pub(crate) enum Error {
+    ParseError(String),
+}
+
+impl std::error::Error for Error {}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Error::ParseError(reason) => {
+                write!(f, "{reason}")
+            }
+        }
+    }
+}
+
+impl From<&'static str> for Error {
+    fn from(reason: &'static str) -> Self {
+        Error::ParseError(reason.to_owned())
+    }
+}
+
+impl From<ErrMode<ContextError>> for Error {
+    fn from(err: ErrMode<ContextError>) -> Self {
+        Error::ParseError(err.to_string())
+    }
+}
+
+impl From<jiff::Error> for Error {
+    fn from(err: jiff::Error) -> Self {
+        Error::ParseError(err.to_string())
+    }
+}

src/items/mod.rs

@@ -44,7 +44,8 @@ mod builder;
 mod ordinal;
 mod primitive;
 
-use builder::DateTimeBuilder;
+pub(crate) mod error;
+
 use jiff::Zoned;
 use primitive::space;
 use winnow::{
@@ -54,7 +55,8 @@ use winnow::{
     ModalResult, Parser,
 };
 
-use crate::ParseDateTimeError;
+use builder::DateTimeBuilder;
+use error::Error;
 
 #[derive(PartialEq, Debug)]
 enum Item {
@@ -69,40 +71,23 @@ enum Item {
 }
 
 /// Parse a date and time string based on a specific date.
-pub(crate) fn parse_at_date<S: AsRef<str> + Clone>(
-    base: Zoned,
-    input: S,
-) -> Result<Zoned, ParseDateTimeError> {
+pub(crate) fn parse_at_date<S: AsRef<str> + Clone>(base: Zoned, input: S) -> Result<Zoned, Error> {
     let input = input.as_ref().to_ascii_lowercase();
     match parse(&mut input.as_str()) {
-        Ok(builder) => at_date(builder, base),
-        Err(_) => Err(ParseDateTimeError::InvalidInput),
+        Ok(builder) => builder.set_base(base).build(),
+        Err(e) => Err(e.into()),
     }
 }
 
 /// Parse a date and time string based on the current local time.
-pub(crate) fn parse_at_local<S: AsRef<str> + Clone>(input: S) -> Result<Zoned, ParseDateTimeError> {
+pub(crate) fn parse_at_local<S: AsRef<str> + Clone>(input: S) -> Result<Zoned, Error> {
     let input = input.as_ref().to_ascii_lowercase();
     match parse(&mut input.as_str()) {
-        Ok(builder) => at_local(builder),
-        Err(_) => Err(ParseDateTimeError::InvalidInput),
+        Ok(builder) => builder.build(),
+        Err(e) => Err(e.into()),
     }
 }
 
-/// Build a `Zoned` object from a `DateTimeBuilder` and a base `Zoned` object.
-fn at_date(builder: DateTimeBuilder, base: Zoned) -> Result<Zoned, ParseDateTimeError> {
-    builder
-        .set_base(base)
-        .build()
-        .ok_or(ParseDateTimeError::InvalidInput)
-}
-
-/// Build a `Zoned` object from a `DateTimeBuilder` and a default `Zoned` object
-/// (the current time in the local timezone).
-fn at_local(builder: DateTimeBuilder) -> Result<Zoned, ParseDateTimeError> {
-    builder.build().ok_or(ParseDateTimeError::InvalidInput)
-}
-
 /// Parse a date and time string.
 ///
 /// Grammar:
@@ -310,10 +295,14 @@ fn expect_error(input: &mut &str, reason: &'static str) -> ErrMode<ContextError>
 mod tests {
     use jiff::{civil::DateTime, tz::TimeZone, ToSpan, Zoned};
 
-    use super::{at_date, parse, DateTimeBuilder};
+    use super::{parse, DateTimeBuilder};
+
+    fn at_date(builder: DateTimeBuilder, base: Zoned) -> Zoned {
+        builder.set_base(base).build().unwrap()
+    }
 
     fn at_utc(builder: DateTimeBuilder) -> Zoned {
-        at_date(builder, Zoned::now().with_time_zone(TimeZone::UTC)).unwrap()
+        at_date(builder, Zoned::now().with_time_zone(TimeZone::UTC))
     }
 
     fn test_eq_fmt(fmt: &str, input: &str) -> String {
@@ -479,43 +468,40 @@ mod tests {
             .unwrap();
 
         assert_eq!(
-            at_date(parse(&mut "last wed").unwrap(), now.clone()).unwrap(),
+            at_date(parse(&mut "last wed").unwrap(), now.clone()),
             now.checked_sub(7.days()).unwrap()
         );
+        assert_eq!(at_date(parse(&mut "this wed").unwrap(), now.clone()), now);
         assert_eq!(
-            at_date(parse(&mut "this wed").unwrap(), now.clone()).unwrap(),
-            now
-        );
-        assert_eq!(
-            at_date(parse(&mut "next wed").unwrap(), now.clone()).unwrap(),
+            at_date(parse(&mut "next wed").unwrap(), now.clone()),
             now.checked_add(7.days()).unwrap()
         );
         assert_eq!(
-            at_date(parse(&mut "last thu").unwrap(), now.clone()).unwrap(),
+            at_date(parse(&mut "last thu").unwrap(), now.clone()),
             now.checked_sub(6.days()).unwrap()
         );
         assert_eq!(
-            at_date(parse(&mut "this thu").unwrap(), now.clone()).unwrap(),
+            at_date(parse(&mut "this thu").unwrap(), now.clone()),
             now.checked_add(1.days()).unwrap()
         );
         assert_eq!(
-            at_date(parse(&mut "next thu").unwrap(), now.clone()).unwrap(),
+            at_date(parse(&mut "next thu").unwrap(), now.clone()),
             now.checked_add(1.days()).unwrap()
         );
         assert_eq!(
-            at_date(parse(&mut "1 wed").unwrap(), now.clone()).unwrap(),
+            at_date(parse(&mut "1 wed").unwrap(), now.clone()),
             now.checked_add(7.days()).unwrap()
         );
         assert_eq!(
-            at_date(parse(&mut "1 thu").unwrap(), now.clone()).unwrap(),
+            at_date(parse(&mut "1 thu").unwrap(), now.clone()),
             now.checked_add(1.days()).unwrap()
         );
         assert_eq!(
-            at_date(parse(&mut "2 wed").unwrap(), now.clone()).unwrap(),
+            at_date(parse(&mut "2 wed").unwrap(), now.clone()),
             now.checked_add(14.days()).unwrap()
         );
         assert_eq!(
-            at_date(parse(&mut "2 thu").unwrap(), now.clone()).unwrap(),
+            at_date(parse(&mut "2 thu").unwrap(), now.clone()),
             now.checked_add(8.days()).unwrap()
         );
     }
@@ -524,30 +510,30 @@ mod tests {
     fn relative_date_time() {
         let now = Zoned::now().with_time_zone(TimeZone::UTC);
 
-        let result = at_date(parse(&mut "2 days ago").unwrap(), now.clone()).unwrap();
+        let result = at_date(parse(&mut "2 days ago").unwrap(), now.clone());
         assert_eq!(result, now.checked_sub(2.days()).unwrap());
         assert_eq!(result.hour(), now.hour());
         assert_eq!(result.minute(), now.minute());
         assert_eq!(result.second(), now.second());
 
-        let result = at_date(parse(&mut "2 days 3 days ago").unwrap(), now.clone()).unwrap();
+        let result = at_date(parse(&mut "2 days 3 days ago").unwrap(), now.clone());
         assert_eq!(result, now.checked_sub(1.days()).unwrap());
         assert_eq!(result.hour(), now.hour());
         assert_eq!(result.minute(), now.minute());
         assert_eq!(result.second(), now.second());
 
-        let result = at_date(parse(&mut "2025-01-01 2 days ago").unwrap(), now.clone()).unwrap();
+        let result = at_date(parse(&mut "2025-01-01 2 days ago").unwrap(), now.clone());
         assert_eq!(result.hour(), 0);
         assert_eq!(result.minute(), 0);
         assert_eq!(result.second(), 0);
 
-        let result = at_date(parse(&mut "3 weeks").unwrap(), now.clone()).unwrap();
+        let result = at_date(parse(&mut "3 weeks").unwrap(), now.clone());
         assert_eq!(result, now.checked_add(21.days()).unwrap());
         assert_eq!(result.hour(), now.hour());
         assert_eq!(result.minute(), now.minute());
         assert_eq!(result.second(), now.second());
 
-        let result = at_date(parse(&mut "2025-01-01 3 weeks").unwrap(), now).unwrap();
+        let result = at_date(parse(&mut "2025-01-01 3 weeks").unwrap(), now);
         assert_eq!(result.hour(), 0);
         assert_eq!(result.minute(), 0);
         assert_eq!(result.second(), 0);
@@ -558,23 +544,23 @@ mod tests {
         let now = Zoned::now().with_time_zone(TimeZone::UTC);
 
         // Pure number as year.
-        let result = at_date(parse(&mut "jul 18 12:30 2025").unwrap(), now.clone()).unwrap();
+        let result = at_date(parse(&mut "jul 18 12:30 2025").unwrap(), now.clone());
         assert_eq!(result.year(), 2025);
 
         // Pure number as time.
-        let result = at_date(parse(&mut "1230").unwrap(), now.clone()).unwrap();
+        let result = at_date(parse(&mut "1230").unwrap(), now.clone());
         assert_eq!(result.hour(), 12);
         assert_eq!(result.minute(), 30);
 
-        let result = at_date(parse(&mut "123").unwrap(), now.clone()).unwrap();
+        let result = at_date(parse(&mut "123").unwrap(), now.clone());
         assert_eq!(result.hour(), 1);
         assert_eq!(result.minute(), 23);
 
-        let result = at_date(parse(&mut "12").unwrap(), now.clone()).unwrap();
+        let result = at_date(parse(&mut "12").unwrap(), now.clone());
         assert_eq!(result.hour(), 12);
         assert_eq!(result.minute(), 0);
 
-        let result = at_date(parse(&mut "1").unwrap(), now.clone()).unwrap();
+        let result = at_date(parse(&mut "1").unwrap(), now.clone());
         assert_eq!(result.hour(), 1);
         assert_eq!(result.minute(), 0);
     }

src/items/pure.rs

@@ -3,7 +3,7 @@
 
 //! Parse a pure number string.
 //!
-//! The GNU docs say:
+//! From the GNU docs:
 //!
 //! > The precise interpretation of a pure decimal number depends on the
 //! > context in the date string.
@@ -22,12 +22,16 @@
 //! > number in the date string, but no relative item, then the number
 //! > overrides the year.
 
-use winnow::{stream::AsChar, token::take_while, ModalResult, Parser};
+use winnow::{ModalResult, Parser};
 
-use super::primitive::s;
+use super::primitive::{dec_uint_str, s};
 
+/// Parse a pure number string and return it as an owned `String`. We return a
+/// `String` here because the interpretation of the number depends on the
+/// parsing context in which it appears. The interpretation is deferred to the
+/// result building phase.
 pub(super) fn parse(input: &mut &str) -> ModalResult<String> {
-    s(take_while(1.., AsChar::is_dec_digit))
+    s(dec_uint_str)
         .map(|s: &str| s.to_owned())
         .parse_next(input)
 }