Stable ordering & fix copy partials (#7)

Author: michaelsproul

Cargo.toml

@@ -17,3 +17,7 @@ itertools = "0.10.0"
 proc-macro2 = "1.0.26"
 quote = "1.0.9"
 syn = "1.0.70"
+
+[dev-dependencies]
+serde = { version = "1.0", features = ["derive"] }
+serde_json = "1.0"

src/attributes.rs

@@ -0,0 +1,51 @@
+//! Utilities to help with parsing configuration attributes.
+use darling::{Error, FromMeta};
+use syn::{Ident, NestedMeta};
+
+/// Parse a list of nested meta items.
+///
+/// Useful for passing through attributes intended for other macros.
+#[derive(Debug)]
+pub struct NestedMetaList {
+    pub metas: Vec<NestedMeta>,
+}
+
+impl FromMeta for NestedMetaList {
+    fn from_list(items: &[NestedMeta]) -> Result<Self, Error> {
+        Ok(Self {
+            metas: items.iter().cloned().collect(),
+        })
+    }
+}
+
+/// List of identifiers implementing `FromMeta`.
+///
+/// Useful for imposing ordering, unlike the `HashMap` options provided by `darling`.
+#[derive(Debug)]
+pub struct IdentList {
+    pub idents: Vec<Ident>,
+}
+
+impl FromMeta for IdentList {
+    fn from_list(items: &[NestedMeta]) -> Result<Self, Error> {
+        let idents = items
+            .iter()
+            .map(|nested_meta| {
+                let meta = match nested_meta {
+                    NestedMeta::Meta(m) => m,
+                    NestedMeta::Lit(l) => {
+                        return Err(Error::custom(format!(
+                            "expected ident, got literal: {:?}",
+                            l
+                        )))
+                    }
+                };
+                let path = meta.path();
+                path.get_ident()
+                    .cloned()
+                    .ok_or(Error::custom(format!("can't parse as ident: {:?}", path)))
+            })
+            .collect::<Result<_, _>>()?;
+        Ok(Self { idents })
+    }
+}

src/lib.rs

@@ -1,3 +1,4 @@
+use attributes::{IdentList, NestedMetaList};
 use darling::FromMeta;
 use itertools::Itertools;
 use proc_macro::TokenStream;
@@ -7,14 +8,16 @@ use std::collections::HashMap;
 use std::iter::{self, FromIterator};
 use syn::{
     parse_macro_input, Attribute, AttributeArgs, Expr, Field, GenericParam, Ident, ItemStruct,
-    Lifetime, LifetimeDef, NestedMeta, Type, TypeGenerics,
+    Lifetime, LifetimeDef, Type, TypeGenerics,
 };
 
+mod attributes;
+
 /// Top-level configuration via the `superstruct` attribute.
 #[derive(Debug, FromMeta)]
 struct StructOpts {
     /// List of variant names of the superstruct being derived.
-    variants: HashMap<Ident, ()>,
+    variants: IdentList,
     /// List of attributes to apply to the variant structs.
     #[darling(default)]
     variant_attributes: Option<NestedMetaList>,
@@ -102,19 +105,6 @@ impl FieldData {
     }
 }
 
-#[derive(Debug)]
-struct NestedMetaList {
-    metas: Vec<NestedMeta>,
-}
-
-impl FromMeta for NestedMetaList {
-    fn from_list(items: &[NestedMeta]) -> Result<Self, darling::Error> {
-        Ok(Self {
-            metas: items.iter().cloned().collect(),
-        })
-    }
-}
-
 #[proc_macro_attribute]
 pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream {
     let attr_args = parse_macro_input!(args as AttributeArgs);
@@ -133,7 +123,7 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream {
 
     let mk_struct_name = |variant_name: &Ident| format_ident!("{}{}", type_name, variant_name);
 
-    let variant_names = opts.variants.keys().cloned().collect_vec();
+    let variant_names = &opts.variants.idents;
     let struct_names = variant_names.iter().map(mk_struct_name).collect_vec();
 
     // Vec of field data.
@@ -520,9 +510,12 @@ fn make_self_arg(mutable: bool) -> proc_macro2::TokenStream {
     }
 }
 
-fn make_type_ref(ty: &Type, mutable: bool) -> proc_macro2::TokenStream {
+fn make_type_ref(ty: &Type, mutable: bool, copy: bool) -> proc_macro2::TokenStream {
+    // XXX: bit hacky, ignore `copy` if `mutable` is set
     if mutable {
         quote! { &mut #ty }
+    } else if copy {
+        quote! { #ty }
     } else {
         quote! { &#ty }
     }
@@ -547,10 +540,13 @@ fn make_partial_getter(
     } else {
         renamed_field.clone()
     };
+    let copy = field_data.partial_getter_opts.copy;
     let self_arg = make_self_arg(mutable);
-    let ret_ty = make_type_ref(&field_data.field.ty, mutable);
+    let ret_ty = make_type_ref(&field_data.field.ty, mutable, copy);
     let ret_expr = if mutable {
         quote! { &mut inner.#field_name }
+    } else if copy {
+        quote! { inner.#field_name }
     } else {
         quote! { &inner.#field_name }
     };

tests/basic.rs

@@ -1,45 +1,93 @@
+use serde::Deserialize;
 use superstruct::superstruct;
 
-#[superstruct(
-    variants(Base, Ext),
-    variant_attributes(derive(Debug, PartialEq)),
-    cast_error(ty = "BlockError", expr = "BlockError::WrongVariant")
-)]
-#[derive(Debug, PartialEq)]
-pub struct Block {
-    #[superstruct(getter(copy))]
-    slot: u64,
-    data: Vec<u8>,
-    #[superstruct(only(Ext))]
-    description: &'static str,
-}
-
-pub enum BlockError {
-    WrongVariant,
-}
-
 #[test]
 fn basic() {
+    #[superstruct(
+        variants(Base, Ext),
+        variant_attributes(derive(Debug, PartialEq, Clone)),
+        cast_error(ty = "BlockError", expr = "BlockError::WrongVariant"),
+        partial_getter_error(ty = "BlockError", expr = "BlockError::WrongVariant")
+    )]
+    #[derive(Debug, PartialEq, Clone)]
+    pub struct Block {
+        #[superstruct(getter(copy))]
+        slot: u64,
+        data: Vec<u8>,
+        #[superstruct(only(Ext), partial_getter(copy))]
+        description: &'static str,
+    }
+
+    #[derive(Debug, PartialEq)]
+    pub enum BlockError {
+        WrongVariant,
+    }
+
     let base = BlockBase {
         slot: 10,
         data: vec![],
     };
-    let lmao = BlockExt {
+    let ext = BlockExt {
         slot: 11,
         data: vec![10],
         description: "oooeee look at this",
     };
 
-    let mut block1 = Block::Base(base);
-    let block2 = Block::Ext(lmao);
+    let mut block1 = Block::Base(base.clone());
+    let mut block2 = Block::Ext(ext.clone());
+
+    // Test basic getters.
+    assert_eq!(block1.slot(), 10);
+    assert_eq!(block2.slot(), 11);
 
-    println!("{:?}", block1);
-    println!("{:?}", block2);
-    println!("{}", block1.slot());
+    // Check ref getters.
+    let block1_ref = block1.to_ref();
+    assert_eq!(block1_ref.slot(), 10);
 
-    let block_ref = block1.to_ref();
-    println!("{:?}", block_ref.slot());
+    // Check casting
+    assert_eq!(block1.as_base(), Ok(&base));
+    assert_eq!(block1.as_ext(), Err(BlockError::WrongVariant));
+    assert_eq!(block2.as_ext(), Ok(&ext));
+    assert_eq!(block2.as_base(), Err(BlockError::WrongVariant));
 
+    // Check mutable reference mutators.
     let mut block_mut_ref = block1.to_mut();
-    println!("{:?}", block_mut_ref.slot_mut());
+    *block_mut_ref.slot_mut() = 1000;
+    assert_eq!(block1.slot(), 1000);
+    *block1.slot_mut() = 1001;
+    assert_eq!(block1.slot(), 1001);
+
+    // Check partial getters.
+    assert_eq!(block1.description(), Err(BlockError::WrongVariant));
+    assert_eq!(block2.description().unwrap(), ext.description);
+    *block2.description_mut().unwrap() = "updated";
+    assert_eq!(block2.description().unwrap(), "updated");
+}
+
+// Test that superstruct's enum ordering is based on the ordering in `variants(...)`.
+// This test fails with variant order (A, B) because A is a subset of B and we're not
+// using `serde(deny_unknown_fields)`.
+#[test]
+fn serde_deserialise_order() {
+    #[superstruct(
+        variants(B, A),
+        variant_attributes(derive(Debug, Deserialize, PartialEq))
+    )]
+    #[serde(untagged)]
+    #[derive(Debug, Deserialize, PartialEq)]
+    struct Message {
+        common: String,
+        #[superstruct(only(B))]
+        exclusive: String,
+    }
+
+    let message_str = r#"{"common": "hello", "exclusive": "world"}"#;
+    let message: Message = serde_json::from_str(&message_str).unwrap();
+
+    let expected = Message::B(MessageB {
+        common: "hello".into(),
+        exclusive: "world".into(),
+    });
+
+    assert_eq!(message, expected);
 }