Skip to content

Commit

Permalink
span: remove the PartialEq and Eq trait implementations
Browse files Browse the repository at this point in the history
Fixes #32
  • Loading branch information
BurntSushi committed Feb 2, 2025
1 parent 8032a8a commit 36d80d4
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 44 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
The deprecated `intz` routines on `Zoned`, `Timestamp`, `civil::DateTime` and
`civil::Date` have been removed. You can use `in_tz` instead. This change was
made because many found the name `intz` to be unclear.
* [#32](https://github.com/BurntSushi/jiff/issues/32):
The `PartialEq` and `Eq` trait implementations on `Span` have been removed.
Ideally these shouldn't have been used, but if you do need them, please use
`Span::fieldwise` to create a `SpanFieldwise`, which does have the `PartialEq`
and `Eq` traits implemented. These were removed on `Span` itself because they
made it very easy to commit subtle bugs.
* [#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
Expand Down
2 changes: 1 addition & 1 deletion bench/src/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ fn parse_friendly(c: &mut Criterion) {
c.bench_function(&format!("jiff-span/{NAME}/{kind}"), |b| {
b.iter(|| {
let got: jiff::Span = input.parse().unwrap();
assert_eq!(got, expected);
assert_eq!(got.fieldwise(), expected.fieldwise());
})
});
}
Expand Down
54 changes: 11 additions & 43 deletions src/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,24 +294,22 @@ pub(crate) use span_eq;
///
/// # Comparisons
///
/// While a `Span` currently implements the `PartialEq` and `Eq` traits, these
/// implementations are deprecated and will be removed in `jiff 0.2`. The trait
/// implementations only do comparisons based on the fields in the `Span` and
/// do not take total duration into account. These were deprecated because
/// it made it too easy to introduce bugs. For example, `120.minutes()` and
/// `2.hours()` always correspond to the same total duration, but they have
/// different representations in memory.
/// A `Span` does not implement the `PartialEq` or `Eq` traits. These traits
/// were implemented in an earlier version of Jiff, but they made it too
/// easy to introduce bugs. For example, `120.minutes()` and `2.hours()`
/// always correspond to the same total duration, but they have different
/// representations in memory and so didn't compare equivalent.
///
/// The reason why the `PartialEq` and `Eq` trait implementations do not do
/// comparisons with total duration is because it is fundamentally impossible
/// to do such comparisons without a reference date in all cases.
///
/// However, it is undeniably occasionally useful to do comparisons based on
/// the component fields, so long as such use cases can tolerate two different
/// spans comparing unequal even when their total durations are equivalent. For
/// example, many of the tests in Jiff (including the tests in the documentation)
/// work by comparing a `Span` to an expected result. This is a good demonstration
/// of when fieldwise comparisons are appropriate.
/// However, it is undeniably occasionally useful to do comparisons based
/// on the component fields, so long as such use cases can tolerate two
/// different spans comparing unequal even when their total durations are
/// equivalent. For example, many of the tests in Jiff (including the tests in
/// the documentation) work by comparing a `Span` to an expected result. This
/// is a good demonstration of when fieldwise comparisons are appropriate.
///
/// To do fieldwise comparisons with a span, use the [`Span::fieldwise`]
/// method. This method creates a [`SpanFieldwise`], which is just a `Span`
Expand Down Expand Up @@ -1407,11 +1405,6 @@ impl Span {
/// field values. For example, Jiff itself makes heavy use of fieldwise
/// comparisons for tests.
///
/// **NOTE:** In `jiff 0.1`, the `Span` type itself also implements the
/// `Eq` and `PartialEq` traits. This was considered a bug in the API and
/// these trait implementations have been deprecated and will be removed
/// in `jiff 0.2`.
///
/// # Example: the difference between `SpanFieldwise` and `Span::compare`
///
/// In short, `SpanFieldwise` considers `2 hours` and `120 minutes` to be
Expand Down Expand Up @@ -3330,26 +3323,6 @@ impl Default for Span {
}
}

/// DEPRECATED since `jiff 0.1.25`.
///
/// Please use [`Span::fieldwise`] to create a span that can be compared by its
/// fields.
///
/// This trait implementation will be removed in `jiff 0.2`.
impl Eq for Span {}

/// DEPRECATED since `jiff 0.1.25`.
///
/// Please use [`Span::fieldwise`] to create a span that can be compared by its
/// fields.
///
/// This trait implementation will be removed in `jiff 0.2`.
impl PartialEq for Span {
fn eq(&self, rhs: &Span) -> bool {
self.fieldwise() == rhs.fieldwise()
}
}

impl core::fmt::Debug for Span {
#[inline]
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
Expand Down Expand Up @@ -3857,11 +3830,6 @@ impl quickcheck::Arbitrary for Span {
/// values. For example, Jiff itself makes heavy use of fieldwise comparisons
/// for tests.
///
/// **NOTE:** In `jiff 0.1`, the `Span` type itself also implements the
/// `Eq` and `PartialEq` traits. This was considered a bug in the API and
/// these trait implementations have been deprecated and will be removed
/// in `jiff 0.2`.
///
/// # Construction
///
/// While callers may use `SpanFieldwise(span)` (where `span` has type [`Span`])
Expand Down

0 comments on commit 36d80d4

Please sign in to comment.