From fc15cdf1700d3ac1e30d001a787dc668eb8ca18b Mon Sep 17 00:00:00 2001 From: Matthew Pope Date: Tue, 25 Apr 2023 12:11:58 -0700 Subject: [PATCH] Updates IonData to work with Deref impls and improves docs --- src/element/annotations.rs | 8 +++++ src/element/mod.rs | 9 +++--- src/element/struct.rs | 37 +++++++++++++++------- src/ion_data/ion_eq.rs | 41 +++++++++++------------- src/ion_data/ion_ord.rs | 36 ++++----------------- src/ion_data/mod.rs | 60 ++++++++++++++++++++++++++++++++--- src/types/bool.rs | 45 ++++++++++++++++++++++++++ src/types/float.rs | 65 ++++++++++++++++++++++++++++++++++++++ src/types/mod.rs | 12 ++++++- 9 files changed, 240 insertions(+), 73 deletions(-) create mode 100644 src/types/bool.rs create mode 100644 src/types/float.rs diff --git a/src/element/annotations.rs b/src/element/annotations.rs index 8a5b91bd..67bfabdc 100644 --- a/src/element/annotations.rs +++ b/src/element/annotations.rs @@ -1,5 +1,7 @@ use crate::element::iterators::SymbolsIterator; +use crate::ion_data::IonOrd; use crate::Symbol; +use std::cmp::Ordering; /// An ordered sequence of symbols that convey additional, application-specific information about /// their associated Ion value. @@ -122,3 +124,9 @@ impl<'a> IntoIterator for &'a Annotations { SymbolsIterator::new(self.symbols.as_slice()) } } + +impl IonOrd for Annotations { + fn ion_cmp(&self, other: &Self) -> Ordering { + self.symbols.ion_cmp(&other.symbols) + } +} diff --git a/src/element/mod.rs b/src/element/mod.rs index 2d7fdcdd..cb87f090 100644 --- a/src/element/mod.rs +++ b/src/element/mod.rs @@ -40,6 +40,7 @@ pub use self::bytes::Bytes; pub use annotations::Annotations; pub use lob::{Blob, Clob}; +use crate::types::float; pub use list::List; pub use r#struct::Struct; pub use sequence::Sequence; @@ -50,13 +51,13 @@ impl IonEq for Value { use Value::*; match (self, other) { (Null(this), Null(that)) => this == that, + (Bool(this), Bool(that)) => this == that, (Int(this), Int(that)) => this.ion_eq(that), - (Float(this), Float(that)) => this.ion_eq(that), + (Float(this), Float(that)) => float::Float::ion_eq_f64(this, that), (Decimal(this), Decimal(that)) => this.ion_eq(that), (Timestamp(this), Timestamp(that)) => this.ion_eq(that), (String(this), String(that)) => this.ion_eq(that), (Symbol(this), Symbol(that)) => this.ion_eq(that), - (Bool(this), Bool(that)) => this.ion_eq(that), (Blob(this), Blob(that)) => this.ion_eq(that), (Clob(this), Clob(that)) => this.ion_eq(that), (SExp(this), SExp(that)) => this.ion_eq(that), @@ -95,13 +96,13 @@ impl IonOrd for Value { Ordering::Less } } + Bool(this) => compare!(Bool(that) => this.cmp(that)), Int(this) => compare!(Int(that) => this.ion_cmp(that)), - Float(this) => compare!(Float(that) => this.ion_cmp(that)), + Float(this) => compare!(Float(that) => float::Float::ion_cmp_f64(this, that)), Decimal(this) => compare!(Decimal(that) => this.ion_cmp(that)), Timestamp(this) => compare!(Timestamp(that) => this.ion_cmp(that)), String(this) => compare!(String(that) => this.ion_cmp(that)), Symbol(this) => compare!(Symbol(that) => this.ion_cmp(that)), - Bool(this) => compare!(Bool(that) => this.ion_cmp(that)), Blob(this) => compare!(Blob(that) => this.ion_cmp(that)), Clob(this) => compare!(Clob(that) => this.ion_cmp(that)), SExp(this) => compare!(SExp(that) => this.ion_cmp(that)), diff --git a/src/element/struct.rs b/src/element/struct.rs index ce47015f..b9b3a35d 100644 --- a/src/element/struct.rs +++ b/src/element/struct.rs @@ -234,22 +234,35 @@ impl IonEq for Struct { impl IonOrd for Struct { fn ion_cmp(&self, other: &Self) -> Ordering { - let mut these_fields = self.fields.by_index.clone(); - let mut those_fields = other.fields.by_index.clone(); - these_fields.sort_by(IonOrd::ion_cmp); - those_fields.sort_by(IonOrd::ion_cmp); - IonOrd::ion_cmp(&these_fields, &those_fields) + let mut these_fields = self.fields.by_index.iter().collect::>(); + let mut those_fields = other.fields.by_index.iter().collect::>(); + these_fields.sort_by(ion_cmp_field); + those_fields.sort_by(ion_cmp_field); + + let mut i0 = these_fields.iter(); + let mut i1 = those_fields.iter(); + loop { + match [i0.next(), i1.next()] { + [None, Some(_)] => return Ordering::Less, + [None, None] => return Ordering::Equal, + [Some(_), None] => return Ordering::Greater, + [Some(a), Some(b)] => { + let ord = ion_cmp_field(a, b); + if ord != Ordering::Equal { + return ord; + } + } + } + } } } -impl IonOrd for (Symbol, Element) { - fn ion_cmp(&self, other: &Self) -> Ordering { - let ord = self.0.text().cmp(&other.0.text()); - if !ord.is_eq() { - return ord; - } - IonOrd::ion_cmp(&self.1, &other.1) +fn ion_cmp_field(this: &&(Symbol, Element), that: &&(Symbol, Element)) -> Ordering { + let ord = this.0.ion_cmp(&that.0); + if !ord.is_eq() { + return ord; } + IonOrd::ion_cmp(&this.1, &that.1) } #[cfg(test)] diff --git a/src/ion_data/ion_eq.rs b/src/ion_data/ion_eq.rs index da4eddbd..f60d9d4b 100644 --- a/src/ion_data/ion_eq.rs +++ b/src/ion_data/ion_eq.rs @@ -1,4 +1,4 @@ -use num_traits::Zero; +use std::ops::Deref; /// Determines whether two values are equal according to Ion's definition of equivalence. /// @@ -22,6 +22,7 @@ use num_traits::Zero; /// * `nan` and `nan` are Ion equivalent but not mathematically equivalent. /// * `0.0e` and `-0.0e` are mathematically equivalent but not Ion equivalent. /// * Decimal `0.0` and `-0.0` are mathematically equivalent but not Ion equivalent. +/// * Decimal `0.0` and `0.00` are mathematically equivalent but not Ion equivalent. /// * Timestamps representing the same point in time at different precisions or at different /// timezone offsets are not Ion equivalent. /// @@ -29,32 +30,28 @@ pub trait IonEq { fn ion_eq(&self, other: &Self) -> bool; } -impl IonEq for &T { - fn ion_eq(&self, other: &Self) -> bool { - T::ion_eq(self, other) - } -} +// impl IonEq for &T { +// fn ion_eq(&self, other: &Self) -> bool { +// T::ion_eq(self, other) +// } +// } +// +// impl IonEq for Vec { +// fn ion_eq(&self, other: &Self) -> bool { +// self.as_slice().ion_eq(other.as_slice()) +// } +// } -impl IonEq for bool { +impl IonEq for R +where + ::Target: IonEq, +{ fn ion_eq(&self, other: &Self) -> bool { - self == other - } -} - -impl IonEq for f64 { - fn ion_eq(&self, other: &Self) -> bool { - if self.is_nan() { - return other.is_nan(); - } - if self.is_zero() { - return other.is_zero() && self.is_sign_negative() == other.is_sign_negative(); - } - // For all other values, fall back to mathematical equivalence - self == other + ::Target::ion_eq(self, other) } } -impl IonEq for Vec { +impl IonEq for [T] { fn ion_eq(&self, other: &Self) -> bool { if self.len() != other.len() { return false; diff --git a/src/ion_data/ion_ord.rs b/src/ion_data/ion_ord.rs index 35836cf3..222c7c3d 100644 --- a/src/ion_data/ion_ord.rs +++ b/src/ion_data/ion_ord.rs @@ -1,6 +1,5 @@ -use crate::element::Annotations; -use crate::IonType; use std::cmp::Ordering; +use std::ops::Deref; /// Trait used for delegating [Eq] and [PartialEq] in [IonData]. /// Implementations of [IonOrd] must be consistent with [IonEq]. @@ -12,37 +11,16 @@ pub(crate) trait IonOrd { fn ion_cmp(&self, other: &Self) -> Ordering; } -impl IonOrd for &T { +impl IonOrd for R +where + ::Target: IonOrd, +{ fn ion_cmp(&self, other: &Self) -> Ordering { - T::ion_cmp(self, other) + ::Target::ion_cmp(self, other) } } -impl IonOrd for IonType { - fn ion_cmp(&self, other: &Self) -> Ordering { - self.partial_cmp(other).unwrap() - } -} - -impl IonOrd for Annotations { - fn ion_cmp(&self, other: &Self) -> Ordering { - self.iter().cmp(other.iter()) - } -} - -impl IonOrd for bool { - fn ion_cmp(&self, other: &Self) -> Ordering { - self.cmp(other) - } -} - -impl IonOrd for f64 { - fn ion_cmp(&self, other: &Self) -> Ordering { - self.total_cmp(other) - } -} - -impl IonOrd for Vec { +impl IonOrd for [T] { fn ion_cmp(&self, other: &Self) -> Ordering { let mut i0 = self.iter(); let mut i1 = other.iter(); diff --git a/src/ion_data/mod.rs b/src/ion_data/mod.rs index f876bf3d..f72b7ccf 100644 --- a/src/ion_data/mod.rs +++ b/src/ion_data/mod.rs @@ -3,14 +3,30 @@ mod ion_ord; use std::cmp::Ordering; use std::fmt::{Display, Formatter}; +use std::ops::Deref; pub(crate) use ion_eq::IonEq; pub(crate) use ion_ord::IonOrd; -/// A wrapper for lifting Ion compatible data into using Ion oriented comparisons (versus the Rust +/// A wrapper for lifting Ion compatible data into using Ion-oriented comparisons (versus the Rust /// value semantics). This enables the default semantics to be what a Rust user expects for native /// values, but allows a user to opt-in to Ion's structural equivalence/order. /// +/// Equivalence with respect to Ion values means that if two Ion values, `X` and `Y`, are equivalent, +/// they represent the same data and can be substituted for the other without loss of information. +/// (See [`IonEq`] for more information.) +/// +/// Some types, such as [`Element`] and [`Value`] cannot be used as the key of a map because they +/// adhere to Rust value semantics—these types cannot implement [`Eq`] because they include `NaN` +/// as a possible value. +/// +/// For use cases that are concerned with preserving the original Ion data, it is necessary to use +/// Ion value equivalence. Many common use cases, such as writing unit tests for code that produces +/// Ion, can be handled with [`IonData::eq()`]. +/// +/// For other use cases, such as using Ion data as the key of a map or passing Ion data to an +/// algorithm that depends on [`Eq`], you can lift values using [`IonData::from()`]. +/// /// Anything that implements [`IonEq`] can be converted into [`IonData`]. [`IonData`] itself does not /// implement [`IonEq`] so that it is impossible to create, for example, `IonData>`. /// @@ -20,13 +36,14 @@ pub(crate) use ion_ord::IonOrd; /// __WARNING__—The Ion specification does _not_ define a total ordering over all Ion values. Do /// not depend on getting any particular result from [Ord]. Use it only as an opaque total ordering /// over all [IonData]. _The algorithm used for [Ord] may change between versions._ +/// #[derive(Debug, Clone)] -pub struct IonData(T); +pub struct IonData(T); impl IonData { /// Checks if two values are equal according to Ion's structural equivalence. - pub fn eq>(a: U, b: U) -> bool { - T::ion_eq(a.as_ref(), b.as_ref()) + pub fn eq>(a: R, b: R) -> bool { + T::ion_eq(a.deref(), b.deref()) } /// Unwraps the value. @@ -55,7 +72,7 @@ impl AsRef for IonData { } } -impl Display for IonData { +impl Display for IonData { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { Display::fmt(&self.0, f) } @@ -72,3 +89,36 @@ impl Ord for IonData { IonOrd::ion_cmp(&self.0, &other.0) } } + +#[cfg(test)] +mod tests { + use crate::ion_data::{IonEq, IonOrd}; + use crate::Element; + use crate::IonData; + use crate::Symbol; + use rstest::*; + use std::boxed::Box; + use std::fmt::Debug; + use std::pin::Pin; + use std::rc::Rc; + use std::sync::Arc; + + /// These tests exist primarily to ensure that we don't break any of trait implementations + /// needed to make this all work. + #[rstest] + #[case::value(|s| Element::read_one(s).unwrap().value().clone().into())] + #[case::symbol(|s| Symbol::from(s).into())] + #[case::element(|s| Element::read_one(s).unwrap().into() )] + #[case::rc_element(|s| Rc::new(Element::read_one(s).unwrap()).into() )] + #[case::vec_element(|s| Element::read_all(s).unwrap().into() )] + #[case::rc_vec_element(|s| Rc::new(Element::read_all(s).unwrap()).into() )] + #[case::box_pin_rc_vec_box_arc_element(|s| Box::new(Pin::new(Rc::new(vec![Box::new(Arc::new(Element::read_one(s).unwrap()))]))).into() )] + fn can_wrap_data( + #[case] the_fn: impl Fn(&'static str) -> IonData, + ) { + let id1: IonData<_> = the_fn("abc"); + let id2: IonData<_> = the_fn("def"); + assert_ne!(id1, id2); + assert!(id1 < id2); + } +} diff --git a/src/types/bool.rs b/src/types/bool.rs new file mode 100644 index 00000000..22393f8f --- /dev/null +++ b/src/types/bool.rs @@ -0,0 +1,45 @@ +use crate::ion_data::{IonEq, IonOrd}; +use std::cmp::Ordering; +use std::fmt::{Display, Formatter}; + +/// Represents an Ion `bool` value, for the purpose of implementing some Ion-related traits. +/// +/// Most of the time, you can use [bool] instead of this type. +#[derive(Debug, Clone, PartialOrd, Ord, PartialEq, Eq)] +pub struct Bool(bool); + +impl Display for Bool { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + ::fmt(&self.0, f) + } +} + +impl From for Bool { + fn from(value: bool) -> Self { + Bool(value) + } +} + +impl From for bool { + fn from(value: Bool) -> Self { + value.0 + } +} + +impl PartialEq for Bool { + fn eq(&self, other: &bool) -> bool { + self.0 == *other + } +} + +impl IonEq for Bool { + fn ion_eq(&self, other: &Self) -> bool { + self.0 == other.0 + } +} + +impl IonOrd for Bool { + fn ion_cmp(&self, other: &Self) -> Ordering { + self.0.cmp(&other.0) + } +} diff --git a/src/types/float.rs b/src/types/float.rs new file mode 100644 index 00000000..cc11c963 --- /dev/null +++ b/src/types/float.rs @@ -0,0 +1,65 @@ +use crate::ion_data::{IonEq, IonOrd}; +use num_traits::Zero; +use std::cmp::Ordering; +use std::fmt::{Display, Formatter}; + +/// Represents an Ion `float` value, for the purpose of implementing some Ion-related traits. +/// +/// Most of the time, you can use [f64] instead of this type. +#[derive(Debug, Clone, PartialOrd, PartialEq)] +pub struct Float(f64); + +impl Float { + /// Implements Ion equivalence for [f64]. + pub(crate) fn ion_eq_f64(this: &f64, that: &f64) -> bool { + if this.is_nan() { + return that.is_nan(); + } + if this.is_zero() { + return that.is_zero() && this.is_sign_negative() == that.is_sign_negative(); + } + // For all other values, fall back to mathematical equivalence + this == that + } + + /// Implements Ion ordering for [f64]. + pub(crate) fn ion_cmp_f64(this: &f64, that: &f64) -> Ordering { + this.total_cmp(that) + } +} + +impl Display for Float { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + ::fmt(&self.0, f) + } +} + +impl From for Float { + fn from(value: f64) -> Self { + Float(value) + } +} + +impl From for f64 { + fn from(value: Float) -> Self { + value.0 + } +} + +impl PartialEq for Float { + fn eq(&self, other: &f64) -> bool { + self.0 == *other + } +} + +impl IonEq for Float { + fn ion_eq(&self, other: &Self) -> bool { + Float::ion_eq_f64(&self.0, &other.0) + } +} + +impl IonOrd for Float { + fn ion_cmp(&self, other: &Self) -> Ordering { + Float::ion_cmp_f64(&self.0, &other.0) + } +} diff --git a/src/types/mod.rs b/src/types/mod.rs index 1aa85f54..db0700ef 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -4,18 +4,22 @@ pub type SymbolId = usize; +pub mod bool; pub mod coefficient; pub mod decimal; +pub mod float; pub mod integer; pub mod string; pub mod timestamp; +use crate::ion_data::IonOrd; +use std::cmp::Ordering; use std::fmt; /// Represents the Ion data type of a given value. To learn more about each data type, /// read [the Ion Data Model](https://amazon-ion.github.io/ion-docs/docs/spec.html#the-ion-data-model) /// section of the spec. -#[derive(Debug, PartialEq, Eq, PartialOrd, Copy, Clone)] +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Copy, Clone)] pub enum IonType { Null, Bool, @@ -63,6 +67,12 @@ impl IonType { } } +impl IonOrd for IonType { + fn ion_cmp(&self, other: &Self) -> Ordering { + self.cmp(other) + } +} + // Represents a level into which the writer has stepped. // A writer that has not yet called step_in() is at the top level. #[derive(Debug, PartialEq, Default)]