feat!: Use gix-error in gix-date

This will make for easier introspection for users of these errors.

Author: Byron

Cargo.lock

@@ -15,7 +15,7 @@ mod _ref {
         }
 
         /// Try to parse the timestamp and create an owned instance from this shared one.
-        pub fn to_owned(&self) -> Result<Signature, gix_date::parse::Error> {
+        pub fn to_owned(&self) -> Result<Signature, gix_date::Error> {
             Ok(Signature {
                 name: self.name.to_owned(),
                 email: self.email.to_owned(),
@@ -58,7 +58,7 @@ mod _ref {
 
         /// Parse the `time` field for access to the passed time since unix epoch, and the time offset.
         /// The format is expected to be [raw](gix_date::parse_header()).
-        pub fn time(&self) -> Result<gix_date::Time, gix_date::parse::Error> {
+        pub fn time(&self) -> Result<gix_date::Time, gix_date::Error> {
             self.time.parse()
         }
     }

gix-actor/src/signature/mod.rs

@@ -19,11 +19,11 @@ doctest = false
 serde = ["dep:serde", "bstr/serde"]
 
 [dependencies]
+gix-error = { version = "0.0.0", path = "../gix-error" }
 bstr = { version = "1.12.0", default-features = false, features = ["std"] }
 serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] }
 itoa = "1.0.17"
 jiff = "0.2.17"
-thiserror = "2.0.17"
 # TODO: used for quick and easy `TimeBacking: std::io::Write` implementation, but could make that `Copy`
 #       and remove this dep with custom impl
 smallvec = { version = "1.15.1", features = ["write"] }

gix-date/Cargo.toml

@@ -15,6 +15,8 @@ pub mod time;
 pub mod parse;
 pub use parse::function::{parse, parse_header};
 
+pub use gix_error::ParseError as Error;
+
 /// A timestamp with timezone.
 #[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]

gix-date/src/lib.rs

@@ -5,10 +5,11 @@ use jiff::{civil::Date, fmt::rfc2822, tz::TimeZone, Zoned};
 use crate::parse::git::parse_git_date_format;
 use crate::parse::raw::parse_raw;
 use crate::{
-    parse::{relative, Error},
+    parse::relative,
     time::format::{DEFAULT, GITOXIDE, ISO8601, ISO8601_STRICT, SHORT},
-    OffsetInSeconds, SecondsSinceUnixEpoch, Time,
+    Error, OffsetInSeconds, SecondsSinceUnixEpoch, Time,
 };
+use gix_error::{Result, ResultExt};
 
 /// Parse `input` as any time that Git can parse when inputting a date.
 ///
@@ -75,7 +76,7 @@ pub fn parse(input: &str, now: Option<SystemTime>) -> Result<Time, Error> {
     Ok(if let Ok(val) = Date::strptime(SHORT.0, input) {
         let val = val
             .to_zoned(TimeZone::UTC)
-            .map_err(|_| Error::InvalidDateString { input: input.into() })?;
+            .or_raise(|| Error::new_with_input("Timezone conversion failed", input))?;
         Time::new(val.timestamp().as_second(), val.offset().seconds())
     } else if let Ok(val) = rfc2822_relaxed(input) {
         Time::new(val.timestamp().as_second(), val.offset().seconds())
@@ -97,7 +98,7 @@ pub fn parse(input: &str, now: Option<SystemTime>) -> Result<Time, Error> {
         // Format::Raw
         val
     } else {
-        return Err(Error::InvalidDateString { input: input.into() });
+        return Err(Error::new_with_input("Unknown date format", input))?;
     })
 }
 
@@ -168,15 +169,15 @@ pub fn parse_header(input: &str) -> Option<Time> {
 /// whose weekdays are inconsistent with the date. While the day-of-week
 /// still must be parsed, it is otherwise ignored. This seems to be
 /// consistent with how `git` behaves.
-fn strptime_relaxed(fmt: &str, input: &str) -> Result<Zoned, jiff::Error> {
+fn strptime_relaxed(fmt: &str, input: &str) -> std::result::Result<Zoned, jiff::Error> {
     let mut tm = jiff::fmt::strtime::parse(fmt, input)?;
     tm.set_weekday(None);
     tm.to_zoned()
 }
 
 /// This is just like strptime_relaxed, except for RFC 2822 parsing.
 /// Namely, it permits the weekday to be inconsistent with the date.
-fn rfc2822_relaxed(input: &str) -> Result<Zoned, jiff::Error> {
+fn rfc2822_relaxed(input: &str) -> std::result::Result<Zoned, jiff::Error> {
     static P: rfc2822::DateTimeParser = rfc2822::DateTimeParser::new().relaxed_weekday(true);
     P.parse_zoned(input)
 }

gix-date/src/parse/function.rs

@@ -1,22 +1,8 @@
 use std::str::FromStr;
 
+use crate::{Error, Time};
 use smallvec::SmallVec;
 
-use crate::Time;
-
-#[derive(thiserror::Error, Debug, Clone)]
-#[allow(missing_docs)]
-pub enum Error {
-    #[error("Could not convert a duration into a date")]
-    RelativeTimeConversion,
-    #[error("Date string can not be parsed")]
-    InvalidDateString { input: String },
-    #[error("The heat-death of the universe happens before this date")]
-    InvalidDate(#[from] std::num::TryFromIntError),
-    #[error("Current time is missing but required to handle relative dates.")]
-    MissingCurrentTime,
-}
-
 /// A container for just enough bytes to hold the largest-possible [`time`](Time) instance.
 /// It's used in conjunction with
 #[derive(Default, Clone)]
@@ -58,7 +44,7 @@ impl FromStr for Time {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        crate::parse_header(s).ok_or_else(|| Error::InvalidDateString { input: s.into() })
+        crate::parse_header(s).ok_or_else(|| Error::new_with_input("invalid time", s))
     }
 }
 

gix-date/src/parse/mod.rs

@@ -1,9 +1,9 @@
 use std::{str::FromStr, time::SystemTime};
 
+use crate::Error;
+use gix_error::{ensure, ParseError, Result, ResultExt};
 use jiff::{tz::TimeZone, Span, Timestamp, Zoned};
 
-use crate::parse::Error;
-
 fn parse_inner(input: &str) -> Option<Result<Span, Error>> {
     let mut split = input.split_whitespace();
     let units = i64::from_str(split.next()?).ok()?;
@@ -15,26 +15,24 @@ fn parse_inner(input: &str) -> Option<Result<Span, Error>> {
 }
 
 pub fn parse(input: &str, now: Option<SystemTime>) -> Option<Result<Zoned, Error>> {
-    parse_inner(input).map(|result| {
+    parse_inner(input).map(|result| -> Result<Zoned, Error> {
         let span = result?;
         // This was an error case in a previous version of this code, where
         // it would fail when converting from a negative signed integer
         // to an unsigned integer. This preserves that failure case even
         // though the code below handles it okay.
-        if span.is_negative() {
-            return Err(Error::RelativeTimeConversion);
-        }
-        now.ok_or(Error::MissingCurrentTime).and_then(|now| {
-            let ts = Timestamp::try_from(now).map_err(|_| Error::RelativeTimeConversion)?;
-            // N.B. This matches the behavior of this code when it was
-            // written with `time`, but we might consider using the system
-            // time zone here. If we did, then it would implement "1 day
-            // ago" correctly, even when it crosses DST transitions. Since
-            // we're in the UTC time zone here, which has no DST, 1 day is
-            // in practice always 24 hours. ---AG
-            let zdt = ts.to_zoned(TimeZone::UTC);
-            zdt.checked_sub(span).map_err(|_| Error::RelativeTimeConversion)
-        })
+        ensure!(!span.is_negative(), ParseError::new(""));
+        let now = now.ok_or(ParseError::new("Missing current time"))?;
+        let ts: Timestamp = Timestamp::try_from(now).or_raise(|| Error::new("Could not convert current time"))?;
+        // N.B. This matches the behavior of this code when it was
+        // written with `time`, but we might consider using the system
+        // time zone here. If we did, then it would implement "1 day
+        // ago" correctly, even when it crosses DST transitions. Since
+        // we're in the UTC time zone here, which has no DST, 1 day is
+        // in practice always 24 hours. ---AG
+        let zdt = ts.to_zoned(TimeZone::UTC);
+        zdt.checked_sub(span)
+            .or_raise(|| Error::new(format!("Failed to subtract {zdt} from {span}")))
     })
 }
 
@@ -51,7 +49,7 @@ fn span(period: &str, units: i64) -> Option<Result<Span, Error>> {
         // Ignore values you don't know, assume seconds then (so does git)
         _anything => Span::new().try_seconds(units),
     };
-    Some(result.map_err(|_| Error::RelativeTimeConversion))
+    Some(result.or_raise(|| Error::new(format!("Couldn't parse span from '{period} {units}'"))))
 }
 
 #[cfg(test)]

gix-date/src/parse/relative.rs

@@ -63,7 +63,7 @@ fn git_rfc2822() {
 }
 
 #[test]
-fn raw() -> gix_testtools::Result {
+fn raw() -> gix_error::Result<(), gix_date::Error> {
     assert_eq!(
         gix_date::parse("1660874655 +0800", None)?,
         Time {
@@ -177,10 +177,10 @@ fn git_default() {
 
 #[test]
 fn invalid_dates_can_be_produced_without_current_time() {
-    assert!(matches!(
-        gix_date::parse("foobar", None).unwrap_err(),
-        gix_date::parse::Error::InvalidDateString { input } if input == "foobar"
-    ));
+    assert_eq!(
+        gix_date::parse("foobar", None).unwrap_err().to_string(),
+        "Unknown date format: foobar"
+    );
 }
 
 /// Tests for compact ISO8601 formats (YYYYMMDDTHHMMSS variants)

gix-date/tests/time/parse/mod.rs

@@ -10,10 +10,12 @@ fn large_offsets() {
 
 #[test]
 fn large_offsets_do_not_panic() {
-    assert!(matches!(
-        gix_date::parse("9999999999 weeks ago", Some(std::time::UNIX_EPOCH)),
-        Err(gix_date::parse::Error::RelativeTimeConversion)
-    ));
+    assert_eq!(
+        gix_date::parse("9999999999 weeks ago", Some(std::time::UNIX_EPOCH))
+            .unwrap_err()
+            .to_string(),
+        "Couldn't parse span from 'week 9999999999'"
+    );
 }
 
 #[test]

gix-date/tests/time/parse/relative.rs

@@ -15,5 +15,6 @@ rust-version = "1.82"
 doctest = false
 
 [dependencies]
-exn = "0.2.1"
+bstr = { version = "1.12.0", default-features = false }
+exn = { version = "0.2.1", default-features = false }