Repository: serde-rs/json (language: rust)

Issue:
Implement Eq for Value, Number and Map
All these types currently implement `PartialEq`. However, they also fulfil the requirements for `Eq`, so we may implement that trait as well.

The `Map` type currently has a [custom implementation for `PartialEq`](https://github.com/serde-rs/json/blob/7fc289015003c39044ffeef42819b5ddf5188a5e/src/map.rs#L229-L243), which looks identical to what we would get by simply deriving `PartialEq`. If the `preserve_order` feature is enabled, the code is [identical to the `PartialEq` implementation for `IndexMap`](https://docs.rs/indexmap/1.3.2/src/indexmap/map.rs.html#2204-2219). For the case that `preserve_order` is disabled, the code simply defers to the implementation for `BTreeMap`. Both `IndexMap` and `BTreeMap` implement `Eq`.

The `Number` type has three variants, wrapping `u64`, `i64` and `f64` respectively. The former two are `Eq`, while the latter is only `PartialEq`. However, since floating-point numbers in JSON are always finite, we can assume them to be `Eq` as well, so we can implement `Eq` for `Number`.

Once `Map` and `Number` implement `Eq`, it can be derived for `Value`.


Discussion / hints:
Could you share a bit about your use case that requires these impls?
The specific use case where this came up was JSON schema validation. The `uniqueItems` key can [mark the items of an array as unique](https://json-schema.org/understanding-json-schema/reference/array.html#uniqueness), and there are no restrictions on the nature of the items, so they may be arbitrary JSON values. One way to check for duplicates is [given in this answer on StackOverflow](https://stackoverflow.com/a/60884343). If `Value` would implement `Eq`, I could simply derive it for the newtype wrapper in this code. This doesn't make much of a difference for the code in that answer, but it would save the tedious work of figuring out whether `Value` is actually `Eq`.

I personally see this more as a matter of consistency rather than specific to a use case, though. If a `PartialEq` implementation defines an equivalence relation, the type should generally also implement `Eq` to mark it as an equivalence relation. The only reason not to is when it is conceivable that a future version of the `PartialEq` implementation might not be `Eq` anymore, but I don't think this is the case here – the JSON specification intentionally omitted NaN, and this isn't going to change anytime soon.
Thanks, makes sense. I would accept a PR for Eq impls.

Fix patch:
diff --git a/src/map.rs b/src/map.rs
index 98f13b2ba..2b49ce438 100644
--- a/src/map.rs
+++ b/src/map.rs
@@ -233,6 +233,8 @@ impl PartialEq for Map<String, Value> {
     }
 }
 
+impl Eq for Map<String, Value> {}
+
 /// Access an element of this map. Panics if the given key is not present in the
 /// map.
 ///
diff --git a/src/number.rs b/src/number.rs
index cedd2bb1e..fd1fac9cc 100644
--- a/src/number.rs
+++ b/src/number.rs
@@ -16,7 +16,7 @@ use serde::de::{IntoDeserializer, MapAccess};
 pub(crate) const TOKEN: &str = "$serde_json::private::Number";
 
 /// Represents a JSON number, whether integer or floating point.
-#[derive(Clone, PartialEq)]
+#[derive(Clone, Eq, PartialEq)]
 pub struct Number {
     n: N,
 }
@@ -31,6 +31,10 @@ enum N {
     Float(f64),
 }
 
+// Implementing Eq is fine since any float values are always finite.
+#[cfg(not(feature = "arbitrary_precision"))]
+impl Eq for N {}
+
 #[cfg(feature = "arbitrary_precision")]
 type N = String;
 
diff --git a/src/value/mod.rs b/src/value/mod.rs
index 3ddabe2b3..d806ee971 100644
--- a/src/value/mod.rs
+++ b/src/value/mod.rs
@@ -107,7 +107,7 @@ pub use crate::raw::RawValue;
 /// Represents any valid JSON value.
 ///
 /// See the `serde_json::value` module documentation for usage examples.
-#[derive(Clone, PartialEq)]
+#[derive(Clone, Eq, PartialEq)]
 pub enum Value {
     /// Represents a JSON null value.
     ///


Test patch: