Fix handling of enum unit variants

An error in the annotate proc-macro produced invalid code for enums with
unit variants.

1. Fix the repetition expansions in the proc macro.
2. Emit comments with unit variants.  Place the comment after the
   variant name.
3. Add a special case for node-then-comment in the emitter to emit the
   comment on the same line following the node.

Signed-off-by: Chris Frantz <cfrantz@google.com>

Author: cfrantz

annotate_derive/src/expand.rs

@@ -89,7 +89,7 @@ fn impl_variants(variants: &[Variant]) -> (Vec<TokenStream>, Vec<TokenStream>) {
             quote! {
                 #variant => match field {
                     MemberId::Variant => #vformat,
-                    #(#formats),*,
+                    #(#formats,)*
                     _ => None,
                 }
             }
@@ -104,7 +104,7 @@ fn impl_variants(variants: &[Variant]) -> (Vec<TokenStream>, Vec<TokenStream>) {
             quote! {
                 #variant => match field {
                     MemberId::Variant => #vcomment,
-                    #(#comments),*,
+                    #(#comments,)*
                     _ => None,
                 }
             }
@@ -126,13 +126,13 @@ fn impl_struct(input: Struct) -> TokenStream {
             impl Annotate for #name {
                 fn format(&self, _variant: Option<&str>, field: &MemberId) -> Option<Format> {
                     match field {
-                        #(#formats),*,
+                        #(#formats,)*
                         _ => None,
                     }
                 }
                 fn comment(&self, _variant: Option<&str>, field: &MemberId) -> Option<String> {
                     match field {
-                        #(#comments),*,
+                        #(#comments,)*
                         _ => None,
                     }
                 }
@@ -156,14 +156,14 @@ fn impl_enum(input: Enum) -> TokenStream {
                 fn format(&self, variant: Option<&str>, field: &MemberId) -> Option<Format> {
                     let variant = variant?;
                     match variant {
-                        #(#formats),*,
+                        #(#formats,)*
                         _ => None,
                     }
                 }
                 fn comment(&self, variant: Option<&str>, field: &MemberId) -> Option<String> {
                     let variant = variant?;
                     match variant {
-                        #(#comments),*,
+                        #(#comments,)*
                         _ => None,
                     }
                 }

src/json.rs

@@ -210,13 +210,29 @@ impl JsonEmitter {
             Document::Null => self.emit_null(w),
             Document::Compact(d) => self.emit_compact(w, d),
             Document::Fragment(ds) => {
-                for d in ds {
-                    self.emit_node(w, d)?;
-                    if d.has_value() {
-                        self.writeln(w, "")?;
-                        self.emit_indent(w)?;
+                match &ds[..] {
+                    // Currently, an enum unit-variant is the only place in the serializer where a
+                    // Fragment is constructed placing the comment after the node.  For this case,
+                    // we want to emit the variant name followed by the comment on the same line.
+                    [n, Document::Comment(c, f)] => {
+                        self.emit_node(w, n)?;
+                        if !self.compact && !self.comment.is_empty() {
+                            write!(w, " ")?;
+                            self.emit_comment(w, c, f)?;
+                        }
                     }
-                }
+                    _ => {
+                        let mut prior_val = false;
+                        for d in ds {
+                            if prior_val {
+                                self.writeln(w, "")?;
+                                self.emit_indent(w)?;
+                            }
+                            self.emit_node(w, d)?;
+                            prior_val = d.has_value();
+                        }
+                    }
+                };
                 Ok(())
             }
         }

src/ser.rs

@@ -203,7 +203,12 @@ impl<'s, 'a> ser::Serializer for &'s mut AnnotatedSerializer<'a> {
         _variant_index: u32,
         variant: &'static str,
     ) -> Result<Self::Ok, Self::Error> {
-        self.serialize_str(variant)
+        let node = self.serialize_str(variant)?;
+        if let Some(c) = self.comment(Some(variant), &MemberId::Variant) {
+            Ok(Document::Fragment(vec![node, c]))
+        } else {
+            Ok(node)
+        }
     }
 
     fn serialize_newtype_struct<T>(

src/yaml.rs

@@ -97,13 +97,26 @@ impl YamlEmitter {
             Document::Null => self.emit_null(w),
             Document::Compact(d) => self.emit_compact(w, d),
             Document::Fragment(ds) => {
-                for d in ds {
-                    self.emit_node(w, d)?;
-                    if d.has_value() {
-                        self.writeln(w, "")?;
-                        self.emit_indent(w)?;
+                match &ds[..] {
+                    [n, Document::Comment(c, f)] => {
+                        self.emit_node(w, n)?;
+                        if !self.compact {
+                            write!(w, " ")?;
+                            self.emit_comment(w, c, f)?;
+                        }
                     }
-                }
+                    _ => {
+                        let mut prior_val = false;
+                        for d in ds {
+                            if prior_val {
+                                self.writeln(w, "")?;
+                                self.emit_indent(w)?;
+                            }
+                            self.emit_node(w, d)?;
+                            prior_val = d.has_value();
+                        }
+                    }
+                };
                 Ok(())
             }
         }

tests/test_format.rs

@@ -263,6 +263,8 @@ enum NesAddress {
     Prg(#[annotate(format=hex)] u8, #[annotate(format=hex)] u16),
     #[annotate(format=compact, comment="NES CHR bank:address")]
     Chr(#[annotate(format=hex)] u8, #[annotate(format=hex)] u16),
+    #[annotate(comment = "Bad Address")]
+    Invalid,
 }
 
 #[derive(Serialize, Deserialize, Annotate, Debug, PartialEq)]
@@ -274,6 +276,7 @@ struct Addresses {
     b: NesAddress,
     c: Option<NesAddress>,
     d: CpuAddress,
+    e: NesAddress,
 }
 
 #[test]
@@ -283,6 +286,7 @@ fn test_nes_addresses() -> Result<()> {
         b: NesAddress::Prg(1, 0x8000),
         c: Some(NesAddress::Chr(2, 0x400)),
         d: CpuAddress(0xFFFA),
+        e: NesAddress::Invalid,
     };
 
     tester!(
@@ -300,7 +304,8 @@ fn test_nes_addresses() -> Result<()> {
           "c": {
             "Chr": [2, 1024]
           },
-          "d": 65530
+          "d": 65530,
+          "e": "Invalid"
         }"#
     );
 
@@ -322,7 +327,8 @@ fn test_nes_addresses() -> Result<()> {
             // NES CHR bank:address
             Chr: [0x2, 0x400]
           },
-          d: 0xFFFA
+          d: 0xFFFA,
+          e: "Invalid" // Bad Address
         }"#
     );
 
@@ -341,7 +347,8 @@ fn test_nes_addresses() -> Result<()> {
           c:
             # NES CHR bank:address
             Chr: [0x2, 0x400]
-          d: 0xFFFA"#
+          d: 0xFFFA
+          e: Invalid # Bad Address"#
     );
 
     Ok(())