From 96bd4417a7d2084736a1af9f2526446733f29465 Mon Sep 17 00:00:00 2001 From: OliverNChalk <11343499+OliverNChalk@users.noreply.github.com> Date: Mon, 14 Aug 2023 16:31:51 -0500 Subject: [PATCH] feat: add optional support for hasbrown Map & Set --- Cargo.lock | 23 ++ serde_with/Cargo.toml | 6 + serde_with/src/de/duplicates.rs | 2 + serde_with/src/de/impls.rs | 17 ++ .../duplicate_key_impls/error_on_duplicate.rs | 40 +++ .../duplicate_key_impls/first_value_wins.rs | 28 ++ .../duplicate_key_impls/last_value_wins.rs | 21 ++ serde_with/src/guide/feature_flags.md | 32 ++- serde_with/src/ser/duplicates.rs | 2 + serde_with/src/ser/impls.rs | 14 +- serde_with/tests/hashbrown.rs | 262 ++++++++++++++++++ 11 files changed, 432 insertions(+), 15 deletions(-) create mode 100644 serde_with/tests/hashbrown.rs diff --git a/Cargo.lock b/Cargo.lock index 894d43d9..ce7218c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,17 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "ahash" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c99f64d1e06488f620f932677e24bc6e2897582980441ae90a671415bd7ec2f" +dependencies = [ + "cfg-if", + "once_cell", + "version_check", +] + [[package]] name = "aho-corasick" version = "1.0.2" @@ -11,6 +22,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "allocator-api2" +version = "0.2.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0942ffc6dcaadf03badf6e6a2d0228460359d5e34b57ccdc720b7382dfbd5ec5" + [[package]] name = "android_system_properties" version = "0.1.5" @@ -262,6 +279,11 @@ name = "hashbrown" version = "0.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2c6201b9ff9fd90a5a3bac2e56a830d0caa509576f0e503818ee82c181b3437a" +dependencies = [ + "ahash", + "allocator-api2", + "serde", +] [[package]] name = "hex" @@ -607,6 +629,7 @@ dependencies = [ "expect-test", "fnv", "glob", + "hashbrown 0.14.0", "hex", "indexmap 1.9.2", "indexmap 2.0.0", diff --git a/serde_with/Cargo.toml b/serde_with/Cargo.toml index 989bd21a..803986c3 100644 --- a/serde_with/Cargo.toml +++ b/serde_with/Cargo.toml @@ -66,6 +66,7 @@ time_0_3 = ["dep:time_0_3"] base64 = {version = "0.21.0", optional = true, default-features = false} chrono_0_4 = {package = "chrono", version = "0.4.20", optional = true, default-features = false, features = ["serde"]} doc-comment = {version = "0.3.3", optional = true} +hashbrown = {version = "0.14.0", optional = true, features = ["serde"]} hex = {version = "0.4.3", optional = true, default-features = false} indexmap_1 = {package = "indexmap", version = "1.8", optional = true, default-features = false, features = ["serde-1"]} indexmap_2 = {package = "indexmap", version = "2.0", optional = true, default-features = false, features = ["serde"]} @@ -112,6 +113,11 @@ name = "hex" path = "tests/hex.rs" required-features = ["hex", "macros"] +[[test]] +name = "hashbrown" +path = "tests/hashbrown.rs" +required-features = ["hashbrown", "macros"] + [[test]] name = "indexmap_1" path = "tests/indexmap_1.rs" diff --git a/serde_with/src/de/duplicates.rs b/serde_with/src/de/duplicates.rs index 246b9972..153f966e 100644 --- a/serde_with/src/de/duplicates.rs +++ b/serde_with/src/de/duplicates.rs @@ -7,6 +7,8 @@ use crate::{ prelude::*, MapFirstKeyWins, MapPreventDuplicates, SetLastValueWins, SetPreventDuplicates, }; +#[cfg(feature = "hashbrown")] +use hashbrown::{HashMap as HashbrownMap, HashSet as HashbrownSet}; #[cfg(feature = "indexmap_1")] use indexmap_1::{IndexMap, IndexSet}; #[cfg(feature = "indexmap_2")] diff --git a/serde_with/src/de/impls.rs b/serde_with/src/de/impls.rs index c5f9eb7a..627edde3 100644 --- a/serde_with/src/de/impls.rs +++ b/serde_with/src/de/impls.rs @@ -1,4 +1,6 @@ use crate::{formats::*, prelude::*}; +#[cfg(feature = "hashbrown")] +use hashbrown::{HashMap as HashbrownMap, HashSet as HashbrownSet}; #[cfg(feature = "indexmap_1")] use indexmap_1::{IndexMap, IndexSet}; #[cfg(feature = "indexmap_2")] @@ -16,6 +18,8 @@ macro_rules! foreach_map { $m!(BTreeMap); #[cfg(feature = "std")] $m!(HashMap); + #[cfg(all(feature = "std", feature = "hashbrown"))] + $m!(HashbrownMap); #[cfg(all(feature = "std", feature = "indexmap_1"))] $m!(IndexMap); #[cfg(all(feature = "std", feature = "indexmap_2"))] @@ -33,6 +37,11 @@ macro_rules! foreach_map_create { HashMap, (|size| HashMap::with_capacity_and_hasher(size, Default::default())) ); + #[cfg(feature = "hashbrown")] + $m!( + HashbrownMap, + (|size| HashbrownMap::with_capacity_and_hasher(size, Default::default())) + ); #[cfg(feature = "indexmap_1")] $m!( IndexMap, @@ -53,6 +62,8 @@ macro_rules! foreach_set { $m!(BTreeSet<(K, V): Ord>); #[cfg(feature = "std")] $m!(HashSet<(K, V): Eq + Hash>); + #[cfg(all(feature = "std", feature = "hashbrown"))] + $m!(HashbrownSet<(K, V): Eq + Hash>); #[cfg(all(feature = "std", feature = "indexmap_1"))] $m!(IndexSet<(K, V): Eq + Hash>); #[cfg(all(feature = "std", feature = "indexmap_2"))] @@ -71,6 +82,12 @@ macro_rules! foreach_set_create { (|size| HashSet::with_capacity_and_hasher(size, S::default())), insert ); + #[cfg(feature = "hashbrown")] + $m!( + HashbrownSet, + (|size| HashbrownSet::with_capacity_and_hasher(size, S::default())), + insert + ); #[cfg(feature = "indexmap_1")] $m!( IndexSet, diff --git a/serde_with/src/duplicate_key_impls/error_on_duplicate.rs b/serde_with/src/duplicate_key_impls/error_on_duplicate.rs index b5d1c70d..673e7660 100644 --- a/serde_with/src/duplicate_key_impls/error_on_duplicate.rs +++ b/serde_with/src/duplicate_key_impls/error_on_duplicate.rs @@ -34,6 +34,26 @@ where } } +#[cfg(feature = "hashbrown")] +impl PreventDuplicateInsertsSet for hashbrown::HashSet +where + T: Eq + Hash, + S: BuildHasher + Default, +{ + #[inline] + fn new(size_hint: Option) -> Self { + match size_hint { + Some(size) => Self::with_capacity_and_hasher(size, S::default()), + None => Self::with_hasher(S::default()), + } + } + + #[inline] + fn insert(&mut self, value: T) -> bool { + self.insert(value) + } +} + #[cfg(feature = "indexmap_1")] impl PreventDuplicateInsertsSet for indexmap_1::IndexSet where @@ -109,6 +129,26 @@ where } } +#[cfg(feature = "hashbrown")] +impl PreventDuplicateInsertsMap for hashbrown::HashMap +where + K: Eq + Hash, + S: BuildHasher + Default, +{ + #[inline] + fn new(size_hint: Option) -> Self { + match size_hint { + Some(size) => Self::with_capacity_and_hasher(size, S::default()), + None => Self::with_hasher(S::default()), + } + } + + #[inline] + fn insert(&mut self, key: K, value: V) -> bool { + self.insert(key, value).is_none() + } +} + #[cfg(feature = "indexmap_1")] impl PreventDuplicateInsertsMap for indexmap_1::IndexMap where diff --git a/serde_with/src/duplicate_key_impls/first_value_wins.rs b/serde_with/src/duplicate_key_impls/first_value_wins.rs index 86c4cefc..33035b9a 100644 --- a/serde_with/src/duplicate_key_impls/first_value_wins.rs +++ b/serde_with/src/duplicate_key_impls/first_value_wins.rs @@ -35,6 +35,34 @@ where } } +#[cfg(feature = "hashbrown")] +impl DuplicateInsertsFirstWinsMap for hashbrown::HashMap +where + K: Eq + Hash, + S: BuildHasher + Default, +{ + #[inline] + fn new(size_hint: Option) -> Self { + match size_hint { + Some(size) => Self::with_capacity_and_hasher(size, S::default()), + None => Self::with_hasher(S::default()), + } + } + + #[inline] + fn insert(&mut self, key: K, value: V) { + use hashbrown::hash_map::Entry; + + match self.entry(key) { + // we want to keep the first value, so do nothing + Entry::Occupied(_) => {} + Entry::Vacant(vacant) => { + vacant.insert(value); + } + } + } +} + #[cfg(feature = "indexmap_1")] impl DuplicateInsertsFirstWinsMap for indexmap_1::IndexMap where diff --git a/serde_with/src/duplicate_key_impls/last_value_wins.rs b/serde_with/src/duplicate_key_impls/last_value_wins.rs index 03f6c0f2..e1139545 100644 --- a/serde_with/src/duplicate_key_impls/last_value_wins.rs +++ b/serde_with/src/duplicate_key_impls/last_value_wins.rs @@ -28,6 +28,27 @@ where } } +#[cfg(feature = "hashbrown")] +impl DuplicateInsertsLastWinsSet for hashbrown::HashSet +where + T: Eq + Hash, + S: BuildHasher + Default, +{ + #[inline] + fn new(size_hint: Option) -> Self { + match size_hint { + Some(size) => Self::with_capacity_and_hasher(size, S::default()), + None => Self::with_hasher(S::default()), + } + } + + #[inline] + fn replace(&mut self, value: T) { + // Hashset already fulfils the contract + self.replace(value); + } +} + #[cfg(feature = "indexmap_1")] impl DuplicateInsertsLastWinsSet for indexmap_1::IndexSet where diff --git a/serde_with/src/guide/feature_flags.md b/serde_with/src/guide/feature_flags.md index 54d79c64..5768a76a 100644 --- a/serde_with/src/guide/feature_flags.md +++ b/serde_with/src/guide/feature_flags.md @@ -6,17 +6,19 @@ Each entry will explain the feature in more detail. `serde_with` is fully `no_std` compatible. Using it in a `no_std` environment requires using `default-features = false`, since `std` is enabled by default. -1. [`alloc`](#alloc) -2. [`std` (default)](#std-default) -3. [`base64`](#base64) -4. [`chrono_0_4`](#chrono_0_4) -5. [`guide`](#guide) -6. [`hex`](#hex) -7. [`indexmap_1`](#indexmap_1) -8. [`indexmap_2`](#indexmap_2) -9. [`json`](#json) -10. [`macros` (default)](#macros-default) -11. [`time_0_3`](#time_0_3) +- [Available Feature Flags](#available-feature-flags) + - [`alloc`](#alloc) + - [`std` (default)](#std-default) + - [`base64`](#base64) + - [`chrono_0_4`](#chrono_0_4) + - [`guide`](#guide) + - [`hashbrown`](#hashbrown) + - [`hex`](#hex) + - [`indexmap_1`](#indexmap_1) + - [`indexmap_2`](#indexmap_2) + - [`json`](#json) + - [`macros` (default)](#macros-default) + - [`time_0_3`](#time_0_3) ## `alloc` @@ -49,6 +51,14 @@ This pulls in `chrono` v0.4 as a dependency. The `guide` feature enables inclusion of this user guide. The feature only changes the rustdoc output and enables no other effects. +## `hashbrown` + +The `hashbrown` feature enables `hashbown::{HashMap, HashSet}` as supported containers. + +This pulls in `hashbrown` as a dependency. +It enables the `alloc` feature. +Some functionality is only available when `std` is enabled too. + ## `hex` The `hex` feature enables serializing data in hex format. diff --git a/serde_with/src/ser/duplicates.rs b/serde_with/src/ser/duplicates.rs index 788dec0a..cef3bd9b 100644 --- a/serde_with/src/ser/duplicates.rs +++ b/serde_with/src/ser/duplicates.rs @@ -2,6 +2,8 @@ use super::impls::{foreach_map, foreach_set}; use crate::{ prelude::*, MapFirstKeyWins, MapPreventDuplicates, SetLastValueWins, SetPreventDuplicates, }; +#[cfg(feature = "hashbrown")] +use hashbrown::{HashMap as HashbrownMap, HashSet as HashbrownSet}; #[cfg(feature = "indexmap_1")] use indexmap_1::{IndexMap, IndexSet}; #[cfg(feature = "indexmap_2")] diff --git a/serde_with/src/ser/impls.rs b/serde_with/src/ser/impls.rs index b03b8b41..4f62990e 100644 --- a/serde_with/src/ser/impls.rs +++ b/serde_with/src/ser/impls.rs @@ -1,4 +1,6 @@ use crate::{formats, formats::Strictness, prelude::*}; +#[cfg(feature = "hashbrown")] +use hashbrown::{HashMap as HashbrownMap, HashSet as HashbrownSet}; #[cfg(feature = "indexmap_1")] use indexmap_1::{IndexMap, IndexSet}; #[cfg(feature = "indexmap_2")] @@ -17,9 +19,11 @@ macro_rules! foreach_map { $m!(BTreeMap); #[cfg(feature = "std")] $m!(HashMap); - #[cfg(all(feature = "indexmap_1"))] + #[cfg(feature = "hashbrown")] + $m!(HashbrownMap); + #[cfg(feature = "indexmap_1")] $m!(IndexMap); - #[cfg(all(feature = "indexmap_2"))] + #[cfg(feature = "indexmap_2")] $m!(IndexMap2); }; } @@ -31,9 +35,11 @@ macro_rules! foreach_set { $m!(BTreeSet<$T>); #[cfg(feature = "std")] $m!(HashSet<$T, H: Sized>); - #[cfg(all(feature = "indexmap_1"))] + #[cfg(feature = "hashbrown")] + $m!(HashbrownSet<$T, H: Sized>); + #[cfg(feature = "indexmap_1")] $m!(IndexSet<$T, H: Sized>); - #[cfg(all(feature = "indexmap_2"))] + #[cfg(feature = "indexmap_2")] $m!(IndexSet2<$T, H: Sized>); }; ($m:ident) => { diff --git a/serde_with/tests/hashbrown.rs b/serde_with/tests/hashbrown.rs new file mode 100644 index 00000000..00f39adb --- /dev/null +++ b/serde_with/tests/hashbrown.rs @@ -0,0 +1,262 @@ +#![allow( + // clippy is broken and shows wrong warnings + // clippy on stable does not know yet about the lint name + unknown_lints, + // https://github.com/rust-lang/rust-clippy/issues/8867 + clippy::derive_partial_eq_without_eq, +)] + +mod utils; + +use crate::utils::{check_deserialization, check_error_deserialization, is_equal}; +use core::iter::FromIterator; +use expect_test::expect; +use hashbrown::{HashMap, HashSet}; +use serde::{Deserialize, Serialize}; +use serde_with::{serde_as, DisplayFromStr, Same}; +use std::net::IpAddr; + +#[test] +fn test_hashmap() { + #[serde_as] + #[derive(Debug, Serialize, Deserialize, PartialEq)] + struct S(#[serde_as(as = "HashMap")] HashMap); + + // Normal + is_equal( + S([(1, 1), (3, 3), (111, 111)].iter().cloned().collect()), + expect![[r#" + { + "1": "1", + "111": "111", + "3": "3" + }"#]], + ); + is_equal(S(HashMap::default()), expect![[r#"{}"#]]); +} + +#[test] +fn test_hashset() { + #[serde_as] + #[derive(Debug, Serialize, Deserialize, PartialEq)] + struct S(#[serde_as(as = "HashSet")] HashSet); + + // Normal + is_equal( + S([1, 2, 3, 4, 5].iter().cloned().collect()), + expect![[r#" + [ + "1", + "5", + "4", + "3", + "2" + ]"#]], + ); + is_equal(S(HashSet::default()), expect![[r#"[]"#]]); +} + +#[test] +fn test_map_as_tuple_list() { + let ip = "1.2.3.4".parse().unwrap(); + let ip2 = "255.255.255.255".parse().unwrap(); + + #[serde_as] + #[derive(Debug, Serialize, Deserialize, PartialEq)] + struct SI(#[serde_as(as = "Vec<(DisplayFromStr, DisplayFromStr)>")] HashMap); + + let map: HashMap<_, _> = vec![(1, ip), (10, ip), (200, ip2)].into_iter().collect(); + is_equal( + SI(map.clone()), + expect![[r#" + [ + [ + "1", + "1.2.3.4" + ], + [ + "200", + "255.255.255.255" + ], + [ + "10", + "1.2.3.4" + ] + ]"#]], + ); + + #[serde_as] + #[derive(Debug, Serialize, Deserialize, PartialEq)] + struct SI2(#[serde_as(as = "Vec<(Same, DisplayFromStr)>")] HashMap); + + is_equal( + SI2(map), + expect![[r#" + [ + [ + 1, + "1.2.3.4" + ], + [ + 200, + "255.255.255.255" + ], + [ + 10, + "1.2.3.4" + ] + ]"#]], + ); +} + +#[test] +fn duplicate_key_first_wins_hashmap() { + #[derive(Debug, PartialEq, Deserialize, Serialize)] + struct S(#[serde(with = "::serde_with::rust::maps_first_key_wins")] HashMap); + + // Different value and key always works + is_equal( + S(HashMap::from_iter(vec![(1, 1), (2, 2), (3, 3)])), + expect![[r#" + { + "1": 1, + "2": 2, + "3": 3 + }"#]], + ); + + // Same value for different keys is ok + is_equal( + S(HashMap::from_iter(vec![(1, 1), (2, 1), (3, 1)])), + expect![[r#" + { + "1": 1, + "2": 1, + "3": 1 + }"#]], + ); + + // Duplicate keys, the first one is used + check_deserialization( + S(HashMap::from_iter(vec![(1, 1), (2, 2)])), + r#"{"1": 1, "2": 2, "1": 3}"#, + ); +} + +#[test] +fn prohibit_duplicate_key_hashmap() { + #[derive(Debug, Eq, PartialEq, Deserialize, Serialize)] + struct S( + #[serde(with = "::serde_with::rust::maps_duplicate_key_is_error")] HashMap, + ); + + // Different value and key always works + is_equal( + S(HashMap::from_iter(vec![(1, 1), (2, 2), (3, 3)])), + expect![[r#" + { + "1": 1, + "2": 2, + "3": 3 + }"#]], + ); + + // Same value for different keys is ok + is_equal( + S(HashMap::from_iter(vec![(1, 1), (2, 1), (3, 1)])), + expect![[r#" + { + "1": 1, + "2": 1, + "3": 1 + }"#]], + ); + + // Duplicate keys are an error + check_error_deserialization::( + r#"{"1": 1, "2": 2, "1": 3}"#, + expect![[r#"invalid entry: found duplicate key at line 1 column 24"#]], + ); +} + +#[test] +fn duplicate_value_last_wins_hashset() { + #[derive(Debug, PartialEq, Deserialize, Serialize)] + struct S(#[serde(with = "::serde_with::rust::sets_last_value_wins")] HashSet); + + #[derive(Debug, Eq, Deserialize, Serialize)] + struct W(i32, bool); + impl PartialEq for W { + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 + } + } + impl std::hash::Hash for W { + fn hash(&self, state: &mut H) + where + H: std::hash::Hasher, + { + self.0.hash(state) + } + } + + // Different values always work + is_equal( + S(HashSet::from_iter(vec![ + W(1, true), + W(2, false), + W(3, true), + ])), + expect![[r#" + [ + [ + 1, + true + ], + [ + 2, + false + ], + [ + 3, + true + ] + ]"#]], + ); + + let value: S = serde_json::from_str( + r#"[ + [1, false], + [1, true], + [2, true], + [2, false] + ]"#, + ) + .unwrap(); + let entries: Vec<_> = value.0.into_iter().collect(); + assert_eq!(1, entries[0].0); + assert!(entries[0].1); + assert_eq!(2, entries[1].0); + assert!(!entries[1].1); +} + +#[test] +fn prohibit_duplicate_value_hashset() { + #[derive(Debug, PartialEq, Deserialize, Serialize)] + struct S(#[serde(with = "::serde_with::rust::sets_duplicate_value_is_error")] HashSet); + + is_equal( + S(HashSet::from_iter(vec![1, 2, 3, 4])), + expect![[r#" + [ + 1, + 4, + 3, + 2 + ]"#]], + ); + check_error_deserialization::( + r#"[1, 2, 3, 4, 1]"#, + expect![[r#"invalid entry: found duplicate value at line 1 column 15"#]], + ); +}