diff --git a/CHANGELOG.md b/CHANGELOG.md index dfd5f26..a667570 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # CHANGELOG +0.2.0 (TBD) +=========== + +**BREAKING CHANGES**: + +* [#36](https://github.com/BurntSushi/jiff/issues/36): +Turn panics during `Timestamp::saturing_add` into errors. Callers adding +spans that are known to contain units of hours or smaller are guaranteed that +this will not panic. + + 0.1.29 (TBD) ============ TODO diff --git a/COMPARE.md b/COMPARE.md index ebc50a5..ea895d2 100644 --- a/COMPARE.md +++ b/COMPARE.md @@ -1093,7 +1093,7 @@ use jiff::{Timestamp, ToSpan}; fn main() -> anyhow::Result<()> { let ts = Timestamp::MAX; assert!(ts.checked_add(1.day()).is_err()); - assert_eq!(ts.saturating_add(1.hour()), ts); + assert_eq!(ts.saturating_add(1.hour())?, ts); Ok(()) } diff --git a/src/timestamp.rs b/src/timestamp.rs index 0f29765..150b9d0 100644 --- a/src/timestamp.rs +++ b/src/timestamp.rs @@ -1352,12 +1352,10 @@ impl Timestamp { /// result saturates on overflow. That is, instead of overflow, either /// [`Timestamp::MIN`] or [`Timestamp::MAX`] is returned. /// - /// # Panics + /// # Errors /// - /// This panics if the given `Span` contains any non-zero units greater - /// than hours. In `jiff 0.2`, this panic will be changed to an error. It - /// panics in `jiff 0.1` to avoid giving incorrect results. (It was an - /// oversight to make this method infallible.) + /// This returns an error if the given `Span` contains any non-zero units + /// greater than hours. /// /// # Example /// @@ -1368,51 +1366,45 @@ impl Timestamp { /// /// assert_eq!( /// Timestamp::MAX, - /// Timestamp::MAX.saturating_add(1.nanosecond()), + /// Timestamp::MAX.saturating_add(1.nanosecond())?, /// ); /// assert_eq!( /// Timestamp::MIN, - /// Timestamp::MIN.saturating_add(-1.nanosecond()), + /// Timestamp::MIN.saturating_add(-1.nanosecond())?, /// ); /// assert_eq!( /// Timestamp::MAX, - /// Timestamp::UNIX_EPOCH.saturating_add(SignedDuration::MAX), + /// Timestamp::UNIX_EPOCH.saturating_add(SignedDuration::MAX)?, /// ); /// assert_eq!( /// Timestamp::MIN, - /// Timestamp::UNIX_EPOCH.saturating_add(SignedDuration::MIN), + /// Timestamp::UNIX_EPOCH.saturating_add(SignedDuration::MIN)?, /// ); /// assert_eq!( /// Timestamp::MAX, - /// Timestamp::UNIX_EPOCH.saturating_add(std::time::Duration::MAX), + /// Timestamp::UNIX_EPOCH.saturating_add(std::time::Duration::MAX)?, /// ); + /// + /// # Ok::<(), Box>(()) /// ``` #[inline] pub fn saturating_add>( self, duration: A, - ) -> Timestamp { + ) -> Result { let duration: TimestampArithmetic = duration.into(); - match duration.saturating_add(self) { - Ok(ts) => ts, - Err(err) => { - // FIXME: This should be an actual error. - // We'll do that in jiff 0.2. - panic!( - "saturating Timestamp arithmetic \ - requires only time units: {err}" - ) - } - } + duration.saturating_add(self).context( + "saturating `Timestamp` arithmetic requires only time units", + ) } /// This routine is identical to [`Timestamp::saturating_add`] with the /// span parameter negated. /// - /// # Panics + /// # Errors /// - /// This routine panics under the same conditions as - /// [`Timestamp::saturating_add`]. + /// This returns an error if the given `Span` contains any non-zero units + /// greater than hours. /// /// # Example /// @@ -1423,33 +1415,35 @@ impl Timestamp { /// /// assert_eq!( /// Timestamp::MIN, - /// Timestamp::MIN.saturating_sub(1.nanosecond()), + /// Timestamp::MIN.saturating_sub(1.nanosecond())?, /// ); /// assert_eq!( /// Timestamp::MAX, - /// Timestamp::MAX.saturating_sub(-1.nanosecond()), + /// Timestamp::MAX.saturating_sub(-1.nanosecond())?, /// ); /// assert_eq!( /// Timestamp::MIN, - /// Timestamp::UNIX_EPOCH.saturating_sub(SignedDuration::MAX), + /// Timestamp::UNIX_EPOCH.saturating_sub(SignedDuration::MAX)?, /// ); /// assert_eq!( /// Timestamp::MAX, - /// Timestamp::UNIX_EPOCH.saturating_sub(SignedDuration::MIN), + /// Timestamp::UNIX_EPOCH.saturating_sub(SignedDuration::MIN)?, /// ); /// assert_eq!( /// Timestamp::MIN, - /// Timestamp::UNIX_EPOCH.saturating_sub(std::time::Duration::MAX), + /// Timestamp::UNIX_EPOCH.saturating_sub(std::time::Duration::MAX)?, /// ); + /// + /// # Ok::<(), Box>(()) /// ``` #[inline] pub fn saturating_sub>( self, duration: A, - ) -> Timestamp { + ) -> Result { let duration: TimestampArithmetic = duration.into(); let Ok(duration) = duration.checked_neg() else { - return Timestamp::MIN; + return Ok(Timestamp::MIN); }; self.saturating_add(duration) } @@ -3701,15 +3695,19 @@ mod tests { } #[test] - #[should_panic] fn timestamp_saturating_add() { - Timestamp::MIN.saturating_add(Span::new().days(1)); + insta::assert_snapshot!( + Timestamp::MIN.saturating_add(Span::new().days(1)).unwrap_err(), + @"saturating `Timestamp` arithmetic requires only time units: operation can only be performed with units of hours or smaller, but found non-zero day units (operations on `Timestamp`, `tz::Offset` and `civil::Time` don't support calendar units in a `Span`)", + ) } #[test] - #[should_panic] fn timestamp_saturating_sub() { - Timestamp::MAX.saturating_sub(Span::new().days(1)); + insta::assert_snapshot!( + Timestamp::MAX.saturating_sub(Span::new().days(1)).unwrap_err(), + @"saturating `Timestamp` arithmetic requires only time units: operation can only be performed with units of hours or smaller, but found non-zero day units (operations on `Timestamp`, `tz::Offset` and `civil::Time` don't support calendar units in a `Span`)", + ) } quickcheck::quickcheck! {