Merge branch 'enum-improvements'

README.md

@@ -46,10 +46,6 @@ pub struct SetPlayerPosition {
 
 Any user-defined type can have the `Parcel` trait automatically derived.
 
-*CAUTION*: Be careful when using `#[derive(Protocol)]` on an `enum`. These values are transmitted using the 1-based enum variant number as a discriminant. This means that you must always add new variants at the end, otherwise the new variant will be parsed incorrectly as a different variant by an older version of your program. Ideally we would force every enum to have discriminators explicitly specified, but this only works for C-like enums.
-
-It is possible to use the `define_packet_kind!` macro specifically if you'd like to have `protocol::Parcel` to be implemented, while also forcing you to specify IDs on every variant.
-
 ## Example
 
 ```rust
@@ -101,5 +97,51 @@ fn main() {
         }
     }
 }
+```
+
+## Enums
+
+### Discriminators
+
+Enum values can be transmitted either by their 1-based variant index, or by transmitting the string name of each variant.
+
+**NOTE:** The default behaviour is to use *the 1-based variant index* (`integer`).
+
+This behaviour can be changed by the `#[protocol(discriminant = "string")]` attribute.
+
+Supported discriminant types:
+
+* `integer` (default)
+    * This transmits the 1-based variant number as the over-the-wire discriminant
+    * Enum variants cannot be reordered in the source without breaking the protocol
+* `string`
+    * This transmits the enum variant name as the over-the-wire discriminant
+    * This uses more bytes per message, but it very flexible
+
+**N.B.** `string` should become the default discriminant type for `#[derive(Protocol)]`. This would be a breaking change.
+Perhaps a major release?
 
+```rust
+#[derive(Protocol, Clone, Debug, PartialEq)]
+#[protocol(discriminant = "string")]
+pub enum PlayerState {
+  Stationary,
+  Flying { velocity: (f32,f32,f32) },
+  Jumping { height: f32 },
+}
+```
+
+### Misc
+
+You can rename the variant for their serialisation.
+
+```rust
+#[derive(Protocol, Clone, Debug, PartialEq)]
+#[protocol(discriminant = "string")]
+pub enum Foo {
+  Bar,
+  #[protocol(name = "Biz")] // the Bing variant will be send/received as 'Biz'.
+  Bing,
+  Baz,
+}
 ```

protocol-derive-tests/src/enums.rs

@@ -1,48 +1,110 @@
-#[allow(unused_imports)]
-use protocol::Parcel;
-
-#[derive(Protocol, Debug, PartialEq, Eq)]
-pub enum BoatKind {
-    Speedboat { warp_speed_enabled: bool },
-    Dingy(u8, u8),
-    Fart,
-}
+#[cfg(test)]
+mod string_discriminants {
+    #[allow(unused_imports)]
+    use protocol::Parcel;
 
-#[test]
-fn named_fields_are_correctly_written() {
-    assert_eq!(vec![0, 0, 0, 1, 1], BoatKind::Speedboat {
-        warp_speed_enabled: true,
-    }.raw_bytes().unwrap());
-}
+    #[derive(Protocol, Clone, Debug, PartialEq)]
+    #[protocol]
+    pub enum PlayerState {
+      Stationary,
+      Flying { velocity: (f32,f32,f32) },
+      Jumping { height: f32 },
+    }
 
-#[test]
-fn unnamed_fields_are_correctly_written() {
-    assert_eq!(vec![0, 0, 0, 2, // discriminator
-                    0xf1, 0xed], BoatKind::Dingy(0xf1, 0xed).raw_bytes().unwrap());
-}
+    #[derive(Protocol, Debug, PartialEq)]
+    #[protocol(discriminant = "string")]
+    pub enum Axis { X, Y, Z, Other(String), Bimp { val: u64 } }
 
-#[test]
-fn unit_variants_are_correctly_written() {
-    assert_eq!(vec![0, 0, 0, 3], // discriminator
-               BoatKind::Fart.raw_bytes().unwrap());
-}
+    #[derive(Protocol, Debug, PartialEq)]
+    #[protocol(discriminant = "string")]
+    pub enum RenamedVariant {
+        Hello,
+        #[protocol(name = "Universe")]
+        World,
+    }
 
-#[test]
-fn named_fields_are_correctly_read() {
-    assert_eq!(BoatKind::Speedboat {
-        warp_speed_enabled: true,
-    }, BoatKind::from_raw_bytes(&[0, 0, 0, 1, 1]).unwrap());
-}
+    fn verify_read_back<P: Parcel + ::std::fmt::Debug + ::std::cmp::PartialEq>(parcel: P) {
+        let read_back = P::from_raw_bytes(&parcel.raw_bytes().unwrap()[..]).unwrap();
+        assert_eq!(parcel, read_back);
+    }
+
+    #[test]
+    fn variant_names_are_discriminators() {
+        assert_eq!(vec![0, 0, 0, 1, 'X' as _], Axis::X.raw_bytes().unwrap());
+        assert_eq!(vec![0, 0, 0, 5, 'O' as _, 't' as _, 'h' as _, 'e' as _, 'r' as _,
+                        0, 0, 0, 4, 'r' as _, 'o' as _, 'l' as _, 'l' as _],
+                   Axis::Other("roll".to_owned()).raw_bytes().unwrap());
+    }
+
+    #[test]
+    fn can_write_and_read_back() {
+        verify_read_back(Axis::Other("boop".to_owned()));
+        verify_read_back(Axis::X);
+        verify_read_back(Axis::Y);
+        verify_read_back(Axis::Bimp { val: 77 });
+    }
+
+    #[test]
+    fn renamed_variants_are_transmitted() {
+        assert_eq!(vec![0, 0, 0, 5, 'H' as _, 'e' as _, 'l' as _, 'l' as _, 'o' as _], RenamedVariant::Hello.raw_bytes().unwrap());
+        assert_eq!(vec![0, 0, 0, 8, 'U' as _, 'n' as _, 'i' as _, 'v' as _, 'e' as _, 'r' as _, 's' as _, 'e' as _], RenamedVariant::World.raw_bytes().unwrap());
+    }
 
-#[test]
-fn unnamed_fields_are_correctly_read() {
-    assert_eq!(BoatKind::Dingy(99, 78),
-               BoatKind::from_raw_bytes(&[0, 0, 0, 2, 99, 78]).unwrap());
+    #[test]
+    fn renamed_variants_can_be_written_and_read_back() {
+        verify_read_back(RenamedVariant::World);
+    }
 }
 
-#[test]
-fn unit_variants_are_correctly_read() {
-    assert_eq!(BoatKind::Fart,
-               BoatKind::from_raw_bytes(&[0, 0, 0, 3]).unwrap());
+#[cfg(test)]
+mod integer_discriminants {
+    #[allow(unused_imports)]
+    use protocol::Parcel;
+
+    #[derive(Protocol, Debug, PartialEq, Eq)]
+    #[protocol(discriminant = "integer")]
+    pub enum BoatKind {
+        Speedboat { warp_speed_enabled: bool },
+        Dingy(u8, u8),
+        Fart,
+    }
+
+    #[test]
+    fn named_fields_are_correctly_written() {
+        assert_eq!(vec![0, 0, 0, 1, 1], BoatKind::Speedboat {
+            warp_speed_enabled: true,
+        }.raw_bytes().unwrap());
+    }
+
+    #[test]
+    fn unnamed_fields_are_correctly_written() {
+        assert_eq!(vec![0, 0, 0, 2, // discriminator
+                        0xf1, 0xed], BoatKind::Dingy(0xf1, 0xed).raw_bytes().unwrap());
+    }
+
+    #[test]
+    fn unit_variants_are_correctly_written() {
+        assert_eq!(vec![0, 0, 0, 3], // discriminator
+                   BoatKind::Fart.raw_bytes().unwrap());
+    }
+
+    #[test]
+    fn named_fields_are_correctly_read() {
+        assert_eq!(BoatKind::Speedboat {
+            warp_speed_enabled: true,
+        }, BoatKind::from_raw_bytes(&[0, 0, 0, 1, 1]).unwrap());
+    }
+
+    #[test]
+    fn unnamed_fields_are_correctly_read() {
+        assert_eq!(BoatKind::Dingy(99, 78),
+                   BoatKind::from_raw_bytes(&[0, 0, 0, 2, 99, 78]).unwrap());
+    }
+
+    #[test]
+    fn unit_variants_are_correctly_read() {
+        assert_eq!(BoatKind::Fart,
+                   BoatKind::from_raw_bytes(&[0, 0, 0, 3]).unwrap());
+    }
 }
 

protocol-derive/Cargo.toml

@@ -14,6 +14,7 @@ keywords = ["protocol", "tcp", "udp", "connection", "encoding"]
 proc-macro = true
 
 [dependencies]
-syn = "0.12.13"
-quote = "0.4.2"
+syn = "0.14.2"
+quote = "0.6.3"
+proc-macro2 = "0.4"
 

protocol-derive/src/attr.rs

@@ -0,0 +1,66 @@
+use format;
+
+use syn;
+
+/// Gets the discriminant format of an enum.
+pub fn discriminant_format<F: format::Format>(attrs: &[syn::Attribute]) -> Option<F> {
+    helper::protocol_meta_name_value_literal("discriminant", attrs).map(helper::expect_lit_str).map(|format_name| {
+        match F::from_str(&format_name) {
+            Ok(f) => f,
+            Err(..) => panic!("invalid enum discriminant format: '{}'", format_name),
+        }
+    })
+}
+
+/// Gets the name as per the attributes.
+pub fn name(attrs: &[syn::Attribute]) -> Option<String> {
+    helper::protocol_meta_name_value_literal("name", attrs).map(helper::expect_lit_str)
+}
+
+mod helper {
+    use syn;
+    use proc_macro2;
+
+    pub fn protocol_meta_list(attrs: &[syn::Attribute]) -> Option<syn::MetaList> {
+        attrs.iter().filter_map(|attr| match attr.interpret_meta() {
+            Some(syn::Meta::List(meta_list)) => {
+                if meta_list.ident == syn::Ident::new("protocol", proc_macro2::Span::call_site()) {
+                    Some(meta_list)
+                } else {
+                    // Unrelated attribute.
+                    None
+                }
+            },
+            _ => None,
+        }).next()
+    }
+
+    pub fn protocol_meta_nested_name_values(attrs: &[syn::Attribute]) -> Vec<syn::MetaNameValue> {
+        protocol_meta_list(attrs).map(|meta_list| {
+            let name_values: Vec<_> = meta_list.nested.iter().
+                map(|n| match n {
+                    syn::NestedMeta::Meta(syn::Meta::NameValue(nv)) => nv.clone(),
+                    _ => panic!("attribute must look like #[protocol(name = \"value\")]"),
+                }).collect();
+            name_values
+        }).unwrap_or_else(|| Vec::new())
+    }
+
+    pub fn protocol_meta_name_value_literal(meta_name: &str, attrs: &[syn::Attribute]) -> Option<syn::Lit> {
+        protocol_meta_nested_name_values(attrs).iter().filter_map(|name_value| {
+            if name_value.ident == syn::Ident::new(meta_name, proc_macro2::Span::call_site()) {
+                Some(name_value.lit.clone())
+            } else {
+                None // Different meta_name
+            }
+        }).next()
+    }
+
+    pub fn expect_lit_str(lit: syn::Lit) -> String {
+        match lit {
+            syn::Lit::Str(s) => s.value(),
+            _ => panic!("expected a string literal"),
+        }
+    }
+}
+

protocol-derive/src/format.rs

@@ -0,0 +1,85 @@
+//! Different protocol formats.
+
+use attr;
+use syn;
+
+pub type Discriminator = u32;
+
+/// Represents a format.
+pub trait Format : Clone {
+    /// From a string.
+    fn from_str(s: &str) -> Result<Self, ()>;
+}
+
+/// The enum protocol format.
+#[derive(Clone, Debug, PartialEq)]
+pub enum Enum {
+    /// The enum is transmitted by using the 1-based index of the enum variant.
+    IntegerDiscriminator,
+    /// The enum is transmitted by using the name of the variant.
+    StringDiscriminator,
+}
+
+impl Enum {
+    /// Gets the discriminator of the enum.
+    pub fn discriminator(&self, e: &syn::DataEnum,
+                         variant: &syn::Variant) -> ::proc_macro2::TokenStream {
+        match *self {
+            Enum::IntegerDiscriminator => {
+                let variant_index = e.variants.iter().position(|v| v.ident == variant.ident).expect("variant not a part of enum");
+
+                let discriminator = match variant.discriminant {
+                    Some((_, syn::Expr::Lit(syn::ExprLit { lit: syn::Lit::Int(ref n), .. }))) => n.value() as Discriminator,
+                    Some(_) => panic!("unknown discriminator"),
+                    // Reserve discriminator 0.
+                    None => variant_index as Discriminator + 1,
+                };
+
+                quote!(#discriminator)
+            },
+            Enum::StringDiscriminator => {
+                let variant_name = attr::name(&variant.attrs).unwrap_or_else(|| variant.ident.to_string());
+                quote! { String::from(#variant_name) }
+            },
+        }
+    }
+
+    pub fn discriminator_for_pattern_matching(&self) -> ::proc_macro2::TokenStream {
+        match *self {
+            Enum::IntegerDiscriminator => quote!(discriminator),
+            Enum::StringDiscriminator => quote!(&discriminator[..]),
+        }
+    }
+
+    pub fn discriminator_variant_for_pattern_matching(&self, e: &syn::DataEnum,
+                                                       variant: &syn::Variant) -> ::proc_macro2::TokenStream {
+        match *self {
+            Enum::IntegerDiscriminator => self.discriminator(e, variant),
+            Enum::StringDiscriminator => {
+                let variant_name = attr::name(&variant.attrs).unwrap_or_else(|| variant.ident.to_string());
+                quote! { #variant_name }
+            },
+        }
+    }
+
+    pub fn discriminator_type(&self) -> ::proc_macro2::TokenStream {
+        match *self {
+            Enum::IntegerDiscriminator => {
+                let s = syn::Ident::new(&format!("u{}", ::std::mem::size_of::<Discriminator>() * 8), ::proc_macro2::Span::call_site());
+                quote!(#s)
+            },
+            Enum::StringDiscriminator => quote!(String),
+        }
+    }
+}
+
+impl Format for Enum {
+    fn from_str(s: &str) -> Result<Self, ()> {
+        match s {
+            "integer" => Ok(Enum::IntegerDiscriminator),
+            "string" => Ok(Enum::StringDiscriminator),
+            _ => Err(()),
+        }
+    }
+}
+