Add default value for properties (takes a function instead of an ex…

…pression) (yewstack#881) * Add `default` value for properties * Default values specified by paths instead of exprs * Fix misleading error message * Remove confusing Option
llebout · Jan 20, 2020 · af74630 · af74630
1 parent 2f44745
commit af74630
Show file tree

Hide file tree

Showing 4 changed files with 294 additions and 66 deletions.
diff --git a/crates/macro/src/derive_props/field.rs b/crates/macro/src/derive_props/field.rs
@@ -1,23 +1,35 @@
 use super::generics::GenericArguments;
 use proc_macro2::{Ident, Span};
-use quote::quote;
+use quote::{quote, quote_spanned};
 use std::cmp::{Ord, Ordering, PartialEq, PartialOrd};
 use std::convert::TryFrom;
 use syn::parse::Result;
 use syn::spanned::Spanned;
-use syn::{Error, Field, Meta, MetaList, NestedMeta, Type, Visibility};
+use syn::{
+    Error, ExprPath, Field, Lit, Meta, MetaList, MetaNameValue, NestedMeta, Type, Visibility,
+};
+
+#[derive(PartialEq, Eq)]
+enum PropAttr {
+    Required { wrapped_name: Ident },
+    Default { default: ExprPath },
+    None,
+}
 
 #[derive(Eq)]
 pub struct PropField {
     ty: Type,
     name: Ident,
-    wrapped_name: Option<Ident>,
+    attr: PropAttr,
 }
 
 impl PropField {
     /// All required property fields are wrapped in an `Option`
     pub fn is_required(&self) -> bool {
-        self.wrapped_name.is_some()
+        match self.attr {
+            PropAttr::Required { .. } => true,
+            _ => false,
+        }
     }
 
     /// This step name is descriptive to help a developer realize they missed a required prop
@@ -31,42 +43,69 @@ impl PropField {
     /// Used to transform the `PropWrapper` struct into `Properties`
     pub fn to_field_setter(&self) -> proc_macro2::TokenStream {
         let name = &self.name;
-        if let Some(wrapped_name) = &self.wrapped_name {
-            quote! {
-                #name: self.wrapped.#wrapped_name.unwrap(),
+        match &self.attr {
+            PropAttr::Required { wrapped_name } => {
+                quote! {
+                    #name: self.wrapped.#wrapped_name.unwrap(),
+                }
             }
-        } else {
-            quote! {
-                #name: self.wrapped.#name,
+            _ => {
+                quote! {
+                    #name: self.wrapped.#name,
+                }
             }
         }
     }
 
     /// Wrap all required props in `Option`
     pub fn to_field_def(&self) -> proc_macro2::TokenStream {
         let ty = &self.ty;
-        if let Some(wrapped_name) = &self.wrapped_name {
-            quote! {
-                #wrapped_name: ::std::option::Option<#ty>,
+        match &self.attr {
+            PropAttr::Required { wrapped_name } => {
+                quote! {
+                    #wrapped_name: ::std::option::Option<#ty>,
+                }
             }
-        } else {
-            let name = &self.name;
-            quote! {
-                #name: #ty,
+            _ => {
+                let name = &self.name;
+                quote! {
+                    #name: #ty,
+                }
             }
         }
     }
 
     /// All optional props must implement the `Default` trait
     pub fn to_default_setter(&self) -> proc_macro2::TokenStream {
-        if let Some(wrapped_name) = &self.wrapped_name {
-            quote! {
-                #wrapped_name: ::std::default::Default::default(),
+        match &self.attr {
+            PropAttr::Required { wrapped_name } => {
+                quote! {
+                    #wrapped_name: ::std::option::Option::None,
+                }
             }
-        } else {
-            let name = &self.name;
-            quote! {
-                #name: ::std::default::Default::default(),
+            PropAttr::Default { default } => {
+                let name = &self.name;
+                let ty = &self.ty;
+                let span = default.span();
+                // Hacks to avoid misleading error message.
+                quote_spanned! {span=>
+                    #name: {
+                        match true {
+                            #[allow(unreachable_code)]
+                            false => {
+                                let __unreachable: #ty = ::std::unreachable!();
+                                __unreachable
+                            },
+                            true => #default()
+                        }
+                    },
+                }
+            }
+            PropAttr::None => {
+                let name = &self.name;
+                quote! {
+                    #name: ::std::default::Default::default(),
+                }
             }
         }
     }
@@ -78,64 +117,77 @@ impl PropField {
         generic_arguments: &GenericArguments,
         vis: &Visibility,
     ) -> proc_macro2::TokenStream {
-        let Self {
-            name,
-            ty,
-            wrapped_name,
-        } = self;
-        if let Some(wrapped_name) = wrapped_name {
-            quote! {
-                #[doc(hidden)]
-                #vis fn #name(mut self, #name: #ty) -> #builder_name<#generic_arguments> {
-                    self.wrapped.#wrapped_name = ::std::option::Option::Some(#name);
-                    #builder_name {
-                        wrapped: self.wrapped,
-                        _marker: ::std::marker::PhantomData,
+        let Self { name, ty, attr } = self;
+        match attr {
+            PropAttr::Required { wrapped_name } => {
+                quote! {
+                    #[doc(hidden)]
+                    #vis fn #name(mut self, #name: #ty) -> #builder_name<#generic_arguments> {
+                        self.wrapped.#wrapped_name = ::std::option::Option::Some(#name);
+                        #builder_name {
+                            wrapped: self.wrapped,
+                            _marker: ::std::marker::PhantomData,
+                        }
                     }
                 }
             }
-        } else {
-            quote! {
-                #[doc(hidden)]
-                #vis fn #name(mut self, #name: #ty) -> #builder_name<#generic_arguments> {
-                    self.wrapped.#name = #name;
-                    self
+            _ => {
+                quote! {
+                    #[doc(hidden)]
+                    #vis fn #name(mut self, #name: #ty) -> #builder_name<#generic_arguments> {
+                        self.wrapped.#name = #name;
+                        self
+                    }
                 }
             }
         }
     }
 
-    // Detect the `#[props(required)]` attribute which denotes required fields
-    fn required_wrapper(named_field: &syn::Field) -> Result<Option<Ident>> {
+    // Detect `#[props(required)]` or `#[props(default="...")]` attribute
+    fn attribute(named_field: &syn::Field) -> Result<PropAttr> {
         let meta_list = if let Some(meta_list) = Self::find_props_meta_list(named_field) {
             meta_list
         } else {
-            return Ok(None);
+            return Ok(PropAttr::None);
         };
 
-        let expected_required = syn::Error::new(meta_list.span(), "expected `props(required)`");
+        let expected_attr = syn::Error::new(
+            meta_list.span(),
+            "expected `props(required)` or `#[props(default=\"...\")]`",
+        );
         let first_nested = if let Some(first_nested) = meta_list.nested.first() {
             first_nested
         } else {
-            return Err(expected_required);
+            return Err(expected_attr);
         };
+        match first_nested {
+            NestedMeta::Meta(Meta::Path(word_path)) => {
+                if !word_path.is_ident("required") {
+                    return Err(expected_attr);
+                }
 
-        let word_path = match first_nested {
-            NestedMeta::Meta(Meta::Path(path)) => path,
-            _ => return Err(expected_required),
-        };
+                if let Some(ident) = &named_field.ident {
+                    let wrapped_name = Ident::new(&format!("{}_wrapper", ident), Span::call_site());
+                    Ok(PropAttr::Required { wrapped_name })
+                } else {
+                    unreachable!()
+                }
+            }
+            NestedMeta::Meta(Meta::NameValue(name_value)) => {
+                let MetaNameValue { path, lit, .. } = name_value;
 
-        if !word_path.is_ident("required") {
-            return Err(expected_required);
-        }
+                if !path.is_ident("default") {
+                    return Err(expected_attr);
+                }
 
-        if let Some(ident) = &named_field.ident {
-            Ok(Some(Ident::new(
-                &format!("{}_wrapper", ident),
-                Span::call_site(),
-            )))
-        } else {
-            unreachable!()
+                if let Lit::Str(lit_str) = lit {
+                    let default = lit_str.parse()?;
+                    Ok(PropAttr::Default { default })
+                } else {
+                    Err(expected_attr)
+                }
+            }
+            _ => Err(expected_attr),
         }
     }
 
@@ -161,7 +213,7 @@ impl TryFrom<Field> for PropField {
 
     fn try_from(field: Field) -> Result<Self> {
         Ok(PropField {
-            wrapped_name: Self::required_wrapper(&field)?,
+            attr: Self::attribute(&field)?,
             ty: field.ty,
             name: field.ident.unwrap(),
         })

diff --git a/tests/derive_props/fail.rs b/tests/derive_props/fail.rs
@@ -50,4 +50,72 @@ mod t4 {
     }
 }
 
+mod t5 {
+    use super::*;
+    #[derive(Clone, Properties)]
+    pub struct Props {
+        // ERROR: default must be given a value
+        #[props(default)]
+        value: String,
+    }
+}
+
+mod t6 {
+    use super::*;
+    #[derive(Clone, Properties)]
+    pub struct Props {
+        // ERROR: 123 is not a path or an identifier
+        #[props(default = 123)]
+        value: i32,
+    }
+}
+
+mod t7 {
+    use super::*;
+    #[derive(Clone, Properties)]
+    pub struct Props {
+        // ERROR: the value must be parsed into a path to a function
+        #[props(default = "123")]
+        value: String,
+    }
+}
+
+mod t8 {
+    use super::*;
+    #[derive(Clone, Properties)]
+    pub struct Props {
+        // ERROR: cannot find function foo in this scope
+        #[props(default = "foo")]
+        value: String,
+    }
+}
+
+mod t9 {
+    use super::*;
+    #[derive(Clone, Properties)]
+    pub struct Props {
+        // ERROR: the function must take no arguments
+        #[props(default = "foo")]
+        value: String,
+    }
+
+    fn foo(bar: i32) -> String {
+        unimplemented!()
+    }
+}
+
+mod t10 {
+    use super::*;
+    #[derive(Clone, Properties)]
+    pub struct Props {
+        // ERROR: the function returns incompatible types
+        #[props(default = "foo")]
+        value: String,
+    }
+
+    fn foo() -> i32 {
+        unimplemented!()
+    }
+}
+
 fn main() {}
diff --git a/tests/derive_props/fail.stderr b/tests/derive_props/fail.stderr
@@ -1,9 +1,40 @@
-error: expected `props(required)`
+error: expected `props(required)` or `#[props(default="...")]`
   --> $DIR/fail.rs:21:11
    |
 21 |         #[props(optional)]
    |           ^^^^^
 
+error: expected `props(required)` or `#[props(default="...")]`
+  --> $DIR/fail.rs:58:11
+   |
+58 |         #[props(default)]
+   |           ^^^^^
+
+error: expected `props(required)` or `#[props(default="...")]`
+  --> $DIR/fail.rs:68:11
+   |
+68 |         #[props(default = 123)]
+   |           ^^^^^
+
+error: expected identifier
+  --> $DIR/fail.rs:78:27
+   |
+78 |         #[props(default = "123")]
+   |                           ^^^^^
+
+error[E0425]: cannot find function `foo` in this scope
+  --> $DIR/fail.rs:88:27
+   |
+88 |         #[props(default = "foo")]
+   |                           ^^^^^ not found in this scope
+   |
+help: possible candidates are found in other modules, you can import them into scope
+   |
+84 |     use crate::t10::foo;
+   |
+84 |     use crate::t9::foo;
+   |
+
 error[E0277]: the trait bound `t1::Value: std::default::Default` is not satisfied
  --> $DIR/fail.rs:9:21
   |
@@ -29,3 +60,25 @@ error[E0599]: no method named `b` found for type `t4::PropsBuilder<t4::PropsBuil
 ...
 49 |         Props::builder().b(1).a(2).build();
    |                          ^ help: there is a method with a similar name: `a`
+
+error[E0061]: this function takes 1 parameter but 0 parameters were supplied
+   --> $DIR/fail.rs:98:27
+    |
+98  |         #[props(default = "foo")]
+    |                           ^^^^^ expected 1 parameter
+...
+102 |     fn foo(bar: i32) -> String {
+    |     -------------------------- defined here
+
+error[E0308]: match arms have incompatible types
+   --> $DIR/fail.rs:112:27
+    |
+112 |         #[props(default = "foo")]
+    |                           ^^^^^
+    |                           |
+    |                           expected struct `std::string::String`, found i32
+    |                           `match` arms have incompatible types
+    |                           this is found to be of type `std::string::String`
+    |
+    = note: expected type `std::string::String`
+               found type `i32`