Fix enum variant apply

Cargo.toml

@@ -1,7 +1,10 @@
 [package]
 name = "serde-diff"
 version = "0.4.1"
-authors = ["Karl Bergström <[email protected]>", "Philip Degarmo <[email protected]>"]
+authors = [
+    "Karl Bergström <[email protected]>",
+    "Philip Degarmo <[email protected]>",
+]
 readme = "README.md"
 exclude = ["examples/*"]
 license = "Apache-2.0 OR MIT"
@@ -17,10 +20,11 @@ maintenance = { status = "experimental" }
 
 [dependencies]
 serde-diff-derive = { version = "0.4.0", path = "serde-diff-derive" }
-serde =  { version = "1", features = [ "derive" ] }
-serde_derive = { version = "1", features = ["deserialize_in_place"]}
+serde = { version = "1", features = ["derive"] }
+serde_derive = { version = "1", features = ["deserialize_in_place"] }
 
 [dev-dependencies]
 serde_json = "1.0"
 bincode = "1.2"
 rmp-serde = "0.15.0"
+maplit = "1.0.2"

README.md

@@ -52,6 +52,8 @@ serde_json
 - [x] BTreeMap (thanks @milkey-mouse)
 - [x] Fixed-size arrays (thanks @Boscop)
 - [x] Tuples (thanks @Boscop)
+- [x] HashSet (thanks @Diggsey)
+- [x] BTreeSet (thanks @Diggsey)
 
 # Simple example
 

examples/map.rs

@@ -34,11 +34,20 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         .map
         .insert("hi".to_string(), vec!["planet".to_string()]);
 
+    let mut hi_planet_hello_world = TestStruct::default();
+    hi_planet_hello_world
+        .map
+        .insert("hi".to_string(), vec!["planet".to_string()]);
+    hi_planet_hello_world
+        .map
+        .insert("hello".to_string(), vec!["world".to_string()]);
+
     let add_hello = serde_json::to_string(&Diff::serializable(&empty, &hello_world))?;
     let hello_to_hi = serde_json::to_string(&Diff::serializable(&hello_world, &hi_world))?;
     let add_planet = serde_json::to_string(&Diff::serializable(&hi_world, &hi_world_and_planet))?;
     let del_world = serde_json::to_string(&Diff::serializable(&hi_world_and_planet, &hi_planet))?;
     let no_change = serde_json::to_string(&Diff::serializable(&hi_planet, &hi_planet))?;
+    let add_world = serde_json::to_string(&Diff::serializable(&hi_planet, &hi_planet_hello_world))?;
 
     let mut built = TestStruct::default();
     for (diff, after) in &[
@@ -47,6 +56,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         (add_planet, hi_world_and_planet),
         (del_world, hi_planet.clone()),
         (no_change, hi_planet),
+        (add_world, hi_planet_hello_world),
     ] {
         println!("{}", diff);
 

examples/set.rs

@@ -0,0 +1,51 @@
+use serde::{Deserialize, Serialize};
+use serde_diff::{Apply, Diff, SerdeDiff};
+use std::collections::HashSet;
+
+#[derive(SerdeDiff, Serialize, Deserialize, Debug, Default, PartialEq, Clone)]
+struct TestStruct {
+    test: bool,
+    //#[serde_diff(opaque)]
+    set: HashSet<String>,
+}
+
+fn main() -> Result<(), Box<dyn std::error::Error>> {
+    let mut empty = TestStruct::default();
+    empty.test = true;
+
+    let mut a = TestStruct::default();
+    a.set.insert("a".to_string());
+
+    let mut ab = TestStruct::default();
+    ab.set.insert("a".to_string());
+    ab.set.insert("b".to_string());
+
+    let mut b = TestStruct::default();
+    b.set.insert("b".to_string());
+
+    let mut c = TestStruct::default();
+    c.set.insert("c".to_string());
+
+    let add_a = serde_json::to_string(&Diff::serializable(&empty, &a))?;
+    let add_b = serde_json::to_string(&Diff::serializable(&a, &ab))?;
+    let del_a = serde_json::to_string(&Diff::serializable(&ab, &b))?;
+    let rep_b_c = serde_json::to_string(&Diff::serializable(&b, &c))?;
+    let no_change = serde_json::to_string(&Diff::serializable(&c, &c))?;
+
+    let mut built = TestStruct::default();
+    for (diff, after) in &[
+        (add_a, a),
+        (add_b, ab),
+        (del_a, b),
+        (rep_b_c, c.clone()),
+        (no_change, c),
+    ] {
+        println!("{}", diff);
+
+        let mut deserializer = serde_json::Deserializer::from_str(&diff);
+        Apply::apply(&mut deserializer, &mut built)?;
+
+        assert_eq!(after, &built);
+    }
+    Ok(())
+}

examples/vec.rs

@@ -0,0 +1,65 @@
+use serde::{Deserialize, Serialize};
+use serde_diff::{Apply, Diff, SerdeDiff};
+
+#[derive(SerdeDiff, Serialize, Deserialize, Debug, PartialEq, Clone)]
+enum Value {
+    Str(String),
+    Int(i32),
+}
+
+impl From<&str> for Value {
+    fn from(other: &str) -> Self {
+        Self::Str(other.into())
+    }
+}
+
+#[derive(SerdeDiff, Serialize, Deserialize, Debug, Default, PartialEq, Clone)]
+struct TestStruct {
+    test: bool,
+    //#[serde_diff(opaque)]
+    list: Vec<Value>,
+}
+
+fn main() -> Result<(), Box<dyn std::error::Error>> {
+    let mut empty = TestStruct::default();
+    empty.test = true;
+
+    let mut a = TestStruct::default();
+    a.list.push("a".into());
+
+    let mut abc = TestStruct::default();
+    abc.list.push("a".into());
+    abc.list.push("b".into());
+    abc.list.push("c".into());
+
+    let mut cba = TestStruct::default();
+    cba.list.push("c".into());
+    cba.list.push("b".into());
+    cba.list.push("a".into());
+
+    let mut c = TestStruct::default();
+    c.list.push("c".into());
+
+    let add_a = serde_json::to_string(&Diff::serializable(&empty, &a))?;
+    let add_b = serde_json::to_string(&Diff::serializable(&a, &abc))?;
+    let del_a = serde_json::to_string(&Diff::serializable(&abc, &cba))?;
+    let rep_b_c = serde_json::to_string(&Diff::serializable(&cba, &c))?;
+    let no_change = serde_json::to_string(&Diff::serializable(&c, &c))?;
+
+    let mut built = TestStruct::default();
+    for (diff, after) in &[
+        (add_a, a),
+        (add_b, abc),
+        (del_a, cba),
+        (rep_b_c, c.clone()),
+        (no_change, c),
+    ] {
+        println!("{}", diff);
+
+        let mut deserializer = serde_json::Deserializer::from_str(&diff);
+        Apply::apply(&mut deserializer, &mut built)?;
+
+        assert_eq!(after, &built);
+    }
+    Ok(())
+}

serde-diff-derive/src/serde_diff/mod.rs

@@ -77,7 +77,7 @@ fn generate_fields_diff(
         let right = format_ident!("r{}", field_idx);
 
         let push = if let Some(_) = ident {
-            quote!{ctx.push_field(#ident_as_str);}
+            quote!{ctx.push_field(#field_idx, #ident_as_str);}
         } else {
             quote!{ctx.push_field_index(#field_idx);}
         };
@@ -170,7 +170,7 @@ fn ok_fields(fields : &syn::Fields) -> Result<Vec<ParsedField>, proc_macro::Toke
     }
 }
 
-fn generate_arms(name: &syn::Ident, variant: Option<&syn::Ident>, fields: &syn::Fields, matching: bool)
+fn generate_arms(name: &syn::Ident, variant: Option<(u16, &syn::Ident)>, fields: &syn::Fields, matching: bool)
                  -> Result<(Vec<proc_macro2::TokenStream>, Vec<proc_macro2::TokenStream>),
                            proc_macro2::TokenStream>
 {
@@ -182,14 +182,15 @@ fn generate_arms(name: &syn::Ident, variant: Option<&syn::Ident>, fields: &syn::
         matching,
     );                    
     let (left, right) =  enum_fields(&fields, false);
-    let variant_specifier = if let Some(id) = variant {
+    let variant_specifier = if let Some((_, id)) = variant {
         quote!{ :: #id}
     } else {
         quote!{}
     };
 
-    let variant_as_str = variant.map(|i| i.to_string());
-    let push_variant = variant.map(|_| quote!{ctx.push_variant(#variant_as_str);});
+    let variant_idx = variant.map(|(i, _)| i);
+    let variant_as_str = variant.map(|(_, i)| i.to_string());
+    let push_variant = variant.map(|_| quote!{ctx.push_variant(#variant_idx, #variant_as_str);});
     let pop_variant = variant.map(|_| quote!{ctx.pop_path_element()?;});
 
     let left = if matching {
@@ -259,6 +260,16 @@ fn generate_arms(name: &syn::Ident, variant: Option<&syn::Ident>, fields: &syn::
         }
 
         if let Some(_) = variant {
+            apply_match_arms.push(quote!{
+                ( &mut #name #variant_specifier #left, Some(serde_diff::DiffPathElementValue::EnumVariantIndex(#variant_idx))) => {
+                    while let Some(element) = ctx.next_path_element(seq)? {
+                        match element {
+                            #(#apply_fn_field_handlers)* 
+                            _ =>  ctx.skip_value(seq)?
+                        }
+                    }
+                }
+            });
             apply_match_arms.push(quote!{
                 ( &mut #name #variant_specifier #left, Some(serde_diff::DiffPathElementValue::EnumVariant(variant))) if variant == #variant_as_str => {
                     while let Some(element) = ctx.next_path_element(seq)? {
@@ -299,8 +310,8 @@ fn generate(
     let has_variants = match &input.data {
         Data::Enum(e) => {
             for matching in &[true, false] {
-                for v in &e.variants {
-                    let (diff, apply) = generate_arms(&struct_args.ident, Some(&v.ident), &v.fields, *matching)?;
+                for (i, v) in e.variants.iter().enumerate() {
+                    let (diff, apply) = generate_arms(&struct_args.ident, Some((i as u16, &v.ident)), &v.fields, *matching)?;
                     diff_match_arms.extend(diff);
                     apply_match_arms.extend(apply);
                 }
@@ -375,6 +386,8 @@ fn generate(
                         #(#apply_match_arms)*
                         _ => ctx.skip_value(seq)?,
                     }
+                    // Consume the extra Exit command generated at the end of enums.
+                    ctx.next_path_element(seq)?;
                     Ok(__changed__)
                 }
             }

src/difference.rs

@@ -27,7 +27,7 @@ pub struct DiffContext<'a, S: SerializeSeq> {
     /// Mode for serializing field paths
     field_path_mode: FieldPathMode,
     /// Set to true if any change is detected
-    has_changes: bool,
+    has_changes: &'a mut bool,
 }
 
 impl<'a, S: SerializeSeq> Drop for DiffContext<'a, S> {
@@ -52,27 +52,44 @@ impl<'a, S: SerializeSeq> DiffContext<'a, S> {
 
     /// True if a change operation has been written
     pub fn has_changes(&self) -> bool {
-        self.has_changes
+        *self.has_changes
     }
 
     /// Called when we visit a field. If the structure is recursive (i.e. struct within struct,
     /// elements within an array) this may be called more than once before a corresponding pop_path_element
     /// is called. See `pop_path_element`
-    pub fn push_field(&mut self, field_name: &'static str) {
-        self.element_stack
-            .as_mut()
-            .unwrap()
-            .push(ElementStackEntry::PathElement(DiffPathElementValue::Field(
-                Cow::Borrowed(field_name),
-            )));
+    pub fn push_field(&mut self, field_idx: u16, field_name: &'static str) {
+        if matches!(self.field_path_mode, FieldPathMode::Index) {
+            self.push_field_index(field_idx);
+        } else {
+            self.element_stack
+                .as_mut()
+                .unwrap()
+                .push(ElementStackEntry::PathElement(DiffPathElementValue::Field(
+                    Cow::Borrowed(field_name),
+                )));
+        }
+    }
+
+    pub fn push_variant(&mut self, variant_idx: u16, variant_name: &'static str) {
+        if matches!(self.field_path_mode, FieldPathMode::Index) {
+            self.push_variant_index(variant_idx);
+        } else {
+            self.element_stack
+                .as_mut()
+                .unwrap()
+                .push(ElementStackEntry::PathElement(
+                    DiffPathElementValue::EnumVariant(Cow::Borrowed(variant_name)),
+                ));
+        }
     }
 
-    pub fn push_variant(&mut self, variant_name: &'static str) {
+    pub fn push_variant_index(&mut self, variant_idx: u16) {
         self.element_stack
             .as_mut()
             .unwrap()
             .push(ElementStackEntry::PathElement(
-                DiffPathElementValue::EnumVariant(Cow::Borrowed(variant_name)),
+                DiffPathElementValue::EnumVariantIndex(variant_idx),
             ));
     }
 
@@ -169,7 +186,7 @@ impl<'a, S: SerializeSeq> DiffContext<'a, S> {
             }
             self.element_stack_start = 0;
         }
-        self.has_changes |= is_change;
+        *self.has_changes |= is_change;
         self.implicit_exit_written = implicit_exit;
         self.serializer.serialize_element(value)
     }
@@ -208,7 +225,7 @@ impl<'a, S: SerializeSeq> DiffContext<'a, S> {
             serializer: &mut *self.serializer,
             implicit_exit_written: self.implicit_exit_written,
             field_path_mode: self.field_path_mode,
-            has_changes: false,
+            has_changes: self.has_changes,
         }
     }
 }
@@ -277,7 +294,7 @@ impl<'a, 'b, T: SerdeDiff> Serialize for Diff<'a, 'b, T> {
                     implicit_exit_written: false,
                     parent_element_stack: None,
                     field_path_mode: self.field_path_mode,
-                    has_changes: false,
+                    has_changes: &mut false,
                 };
                 self.old.diff(&mut ctx, &self.new).unwrap();
             }
@@ -297,13 +314,13 @@ impl<'a, 'b, T: SerdeDiff> Serialize for Diff<'a, 'b, T> {
                 implicit_exit_written: false,
                 parent_element_stack: None,
                 field_path_mode: self.field_path_mode,
-                has_changes: false,
+                has_changes: &mut false,
             };
 
             // Do the actual comparison, writing diff commands (see DiffCommandRef, DiffCommandValue)
             // into the sequence
             self.old.diff(&mut ctx, &self.new)?;
-            self.has_changes.set(ctx.has_changes);
+            self.has_changes.set(*ctx.has_changes);
         }
 
         // End the sequence on the serializer
@@ -601,6 +618,7 @@ pub enum DiffPathElementValue<'a> {
     Field(Cow<'a, str>),
     FieldIndex(u16),
     EnumVariant(Cow<'a, str>),
+    EnumVariantIndex(u16),
     FullEnumVariant,
     CollectionIndex(usize),
     AddToCollection,