Repository: Peternator7/strum
Pull request #270 (2023-05-16 01:14:48)

fix!: use the tuple value for to_string on default
The default attribute on a tuple like variant now causes the to_string
and display format to use the value of the tuple rather than the name
of the variant.

E.g. Color::Green("lime").to_string() will equal "lime" not "Green"

Fixes: how to round trip Display and EnumString with default="true" https://github.com/Peternator7/strum/issues/86

BREAKING CHANGE
This changes how Display and ToString cause the following to renders:
```rust
#[strum(default)]
Green(String)
```
To maintain the previous behavior (use the variant name):
```rust
#[strum(default, to_string("Green"))]
Green(String)
```

joshka: This is the no breaking changes approach. It could reasonably break changes by just making this the default behavior. Happy to see this as a PoC of the functionality, and ask for feedback on how it should be configured.

Some options:
```rust
// the current implementation in this PR
#[strum(default, default_to_string)]

// perhaps this PR's behavior should just be the default
#[strum(default)]

// perhaps we could pass a to_string with no value (or with "")
#[strum(default, to_string)]
```

Perhaps the default to_string for a default variant should automatically include the data item e.g. `Green("lime-green")` or `Green(lime-green)`, with attributes controlling whether to use only the data or to rename the variant (e.g. `SomeOtherVariantName(dataPassedToTheVariant)`)

I don't really have a good enough feel for what the correct approach should be on this, so happy to take any guidance.
joshka: I'm not 100% sure why the 1.32 breakage on this happens :?
joshka: > I was unpleasantly surprised that `default` is not symmetric. This feature is very much needed!

Same - it's not a big deal, but it meant having to work around it for that catchall case. Do you have any thoughts about the ergonomics of the API of this feature? I haven't used strum before and I'm fairly new to rust, so don't know what would is idiomatic here (and it seems like this crate has some pretty high standards for backwards compatible changes, so getting it right is a one time thing).
joshka: Incidentally running `cargo msrv` on a mac suggests that the MSRV right now is 1.54.0, not 1.32.0.
Peternator7: Hey @joshka, I've been thinking about this, and I think you're right. `0.25` is going to have a couple of breaking changes so this seems like a good time to include this one as well. Let's make this the default behavior, and if you can, add an attribute that brings back the old behavior if that's what someone really wants. Seem reasonable?
joshka: Sounds like a good plan. Just to confirm, when there is no to_string, we render the tuple
```rust
#[strum(default]
Custom(String)
// Custom("lime") should render as "lime"
```

while if there is a to_string, we just ignore the tuple:

```rust
#[strum(default, to_string("custom color")]
Custom(String)
// Custom("lime") should render as "custom color"
```

and at least for now there's no way to render something like "custom color: lime"
joshka: Updated PR title and body in case you use github to push rather than doing it locally.

New commit message:

---

fix!: use the tuple value for to_string on default

The default attribute on a tuple like variant now causes the to_string
and display format to use the value of the tuple rather than the name
of the variant.

E.g. Color::Green("lime").to_string() will equal "lime" not "Green"

Fixes: how to round trip Display and EnumString with default="true" https://github.com/Peternator7/strum/issues/86

BREAKING CHANGE
This changes how Display and ToString cause the following to renders:
```rust
#[strum(default)]
Green(String)
```
To maintain the previous behavior (use the variant name):
```rust
#[strum(default, to_string("Green"))]
Green(String)
```

Patch:
diff --git a/strum_macros/Cargo.toml b/strum_macros/Cargo.toml
index bdb1c3c..60e5b57 100644
--- a/strum_macros/Cargo.toml
+++ b/strum_macros/Cargo.toml
@@ -23,7 +23,7 @@ heck = "0.4.1"
 proc-macro2 = "1.0"
 quote = "1.0"
 rustversion = "1.0"
-syn = { version = "1.0", features = ["parsing", "extra-traits"] }
+syn = { version = "2.0", features = ["parsing", "extra-traits"] }
 
 [dev-dependencies]
 strum = "0.24"
diff --git a/strum_macros/src/helpers/metadata.rs b/strum_macros/src/helpers/metadata.rs
index 56e4c78..d638ae3 100644
--- a/strum_macros/src/helpers/metadata.rs
+++ b/strum_macros/src/helpers/metadata.rs
@@ -1,12 +1,11 @@
-use proc_macro2::{Span, TokenStream};
+use proc_macro2::TokenStream;
 use syn::{
     parenthesized,
     parse::{Parse, ParseStream},
     parse2, parse_str,
     punctuated::Punctuated,
-    spanned::Spanned,
-    Attribute, DeriveInput, Ident, Lit, LitBool, LitStr, Meta, MetaNameValue, Path, Token, Variant,
-    Visibility,
+    Attribute, DeriveInput, Expr, ExprLit, Ident, Lit, LitBool, LitStr, Meta, MetaNameValue, Path,
+    Token, Variant, Visibility,
 };
 
 use super::case_style::CaseStyle;
@@ -76,17 +75,6 @@ impl Parse for EnumMeta {
     }
 }
 
-impl Spanned for EnumMeta {
-    fn span(&self) -> Span {
-        match self {
-            EnumMeta::SerializeAll { kw, .. } => kw.span(),
-            EnumMeta::AsciiCaseInsensitive(kw) => kw.span(),
-            EnumMeta::Crate { kw, .. } => kw.span(),
-            EnumMeta::UsePhf(use_phf) => use_phf.span(),
-        }
-    }
-}
-
 pub enum EnumDiscriminantsMeta {
     Derive { kw: kw::derive, paths: Vec<Path> },
     Name { kw: kw::name, name: Ident },
@@ -100,7 +88,7 @@ impl Parse for EnumDiscriminantsMeta {
             let kw = input.parse()?;
             let content;
             parenthesized!(content in input);
-            let paths = content.parse_terminated::<_, Token![,]>(Path::parse)?;
+            let paths = content.parse_terminated(Path::parse, Token![,])?;
             Ok(EnumDiscriminantsMeta::Derive {
                 kw,
                 paths: paths.into_iter().collect(),
@@ -127,17 +115,6 @@ impl Parse for EnumDiscriminantsMeta {
     }
 }
 
-impl Spanned for EnumDiscriminantsMeta {
-    fn span(&self) -> Span {
-        match self {
-            EnumDiscriminantsMeta::Derive { kw, .. } => kw.span,
-            EnumDiscriminantsMeta::Name { kw, .. } => kw.span,
-            EnumDiscriminantsMeta::Vis { kw, .. } => kw.span,
-            EnumDiscriminantsMeta::Other { path, .. } => path.span(),
-        }
-    }
-}
-
 pub trait DeriveInputExt {
     /// Get all the strum metadata associated with an enum.
     fn get_metadata(&self) -> syn::Result<Vec<EnumMeta>>;
@@ -228,7 +205,7 @@ impl Parse for VariantMeta {
             let kw = input.parse()?;
             let content;
             parenthesized!(content in input);
-            let props = content.parse_terminated::<_, Token![,]>(Prop::parse)?;
+            let props = content.parse_terminated(Prop::parse, Token![,])?;
             Ok(VariantMeta::Props {
                 kw,
                 props: props
@@ -256,22 +233,6 @@ impl Parse for Prop {
     }
 }
 
-impl Spanned for VariantMeta {
-    fn span(&self) -> Span {
-        match self {
-            VariantMeta::Message { kw, .. } => kw.span,
-            VariantMeta::DetailedMessage { kw, .. } => kw.span,
-            VariantMeta::Documentation { value } => value.span(),
-            VariantMeta::Serialize { kw, .. } => kw.span,
-            VariantMeta::ToString { kw, .. } => kw.span,
-            VariantMeta::Disabled(kw) => kw.span,
-            VariantMeta::Default(kw) => kw.span,
-            VariantMeta::AsciiCaseInsensitive { kw, .. } => kw.span,
-            VariantMeta::Props { kw, .. } => kw.span,
-        }
-    }
-}
-
 pub trait VariantExt {
     /// Get all the metadata associated with an enum variant.
     fn get_metadata(&self) -> syn::Result<Vec<VariantMeta>>;
@@ -282,26 +243,32 @@ impl VariantExt for Variant {
         let result = get_metadata_inner("strum", &self.attrs)?;
         self.attrs
             .iter()
-            .filter(|attr| attr.path.is_ident("doc"))
+            .filter(|attr| attr.path().is_ident("doc"))
             .try_fold(result, |mut vec, attr| {
                 if let Meta::NameValue(MetaNameValue {
-                    lit: Lit::Str(value),
+                    value:
+                        Expr::Lit(ExprLit {
+                            lit: Lit::Str(value),
+                            ..
+                        }),
                     ..
-                }) = attr.parse_meta()?
+                }) = &attr.meta
                 {
-                    vec.push(VariantMeta::Documentation { value })
+                    vec.push(VariantMeta::Documentation {
+                        value: value.clone(),
+                    })
                 }
                 Ok(vec)
             })
     }
 }
 
-fn get_metadata_inner<'a, T: Parse + Spanned>(
+fn get_metadata_inner<'a, T: Parse>(
     ident: &str,
     it: impl IntoIterator<Item = &'a Attribute>,
 ) -> syn::Result<Vec<T>> {
     it.into_iter()
-        .filter(|attr| attr.path.is_ident(ident))
+        .filter(|attr| attr.path().is_ident(ident))
         .try_fold(Vec::new(), |mut vec, attr| {
             vec.extend(attr.parse_args_with(Punctuated::<T, Token![,]>::parse_terminated)?);
             Ok(vec)
diff --git a/strum_macros/src/helpers/variant_props.rs b/strum_macros/src/helpers/variant_props.rs
index a39d3ea..f637253 100644
--- a/strum_macros/src/helpers/variant_props.rs
+++ b/strum_macros/src/helpers/variant_props.rs
@@ -19,7 +19,7 @@ pub struct StrumVariantProperties {
     pub documentation: Vec<LitStr>,
     pub string_props: Vec<(LitStr, LitStr)>,
     serialize: Vec<LitStr>,
-    to_string: Option<LitStr>,
+    pub to_string: Option<LitStr>,
     ident: Option<Ident>,
 }
 
diff --git a/strum_macros/src/macros/enum_count.rs b/strum_macros/src/macros/enum_count.rs
index 44c7f2e..b9e6aaa 100644
--- a/strum_macros/src/macros/enum_count.rs
+++ b/strum_macros/src/macros/enum_count.rs
@@ -2,11 +2,18 @@ use proc_macro2::TokenStream;
 use quote::quote;
 use syn::{Data, DeriveInput};
 
+use crate::helpers::variant_props::HasStrumVariantProperties;
 use crate::helpers::{non_enum_error, HasTypeProperties};
 
 pub(crate) fn enum_count_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
     let n = match &ast.data {
-        Data::Enum(v) => v.variants.len(),
+        Data::Enum(v) => v.variants.iter().try_fold(0usize, |acc, v| {
+            if v.get_variant_properties()?.disabled.is_none() {
+                Ok::<usize, syn::Error>(acc + 1usize)
+            } else {
+                Ok::<usize, syn::Error>(acc)
+            }
+        })?,
         _ => return Err(non_enum_error()),
     };
     let type_properties = ast.get_type_properties()?;
diff --git a/strum_macros/src/macros/enum_discriminants.rs b/strum_macros/src/macros/enum_discriminants.rs
index 66eee46..4e54a30 100644
--- a/strum_macros/src/macros/enum_discriminants.rs
+++ b/strum_macros/src/macros/enum_discriminants.rs
@@ -53,14 +53,16 @@ pub fn enum_discriminants_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
             .filter(|attr| {
                 ATTRIBUTES_TO_COPY
                     .iter()
-                    .any(|attr_whitelisted| attr.path.is_ident(attr_whitelisted))
+                    .any(|attr_whitelisted| attr.path().is_ident(attr_whitelisted))
             })
             .map(|attr| {
-                if attr.path.is_ident("strum_discriminants") {
-                    let passthrough_group = attr
-                        .tokens
-                        .clone()
-                        .into_iter()
+                if attr.path().is_ident("strum_discriminants") {
+                    let mut ts = attr.meta.require_list()?.to_token_stream().into_iter();
+
+                    // Discard strum_discriminants(...)
+                    let _ = ts.next();
+
+                    let passthrough_group = ts
                         .next()
                         .ok_or_else(|| strum_discriminants_passthrough_error(attr))?;
                     let passthrough_attribute = match passthrough_group {
diff --git a/strum_macros/src/macros/from_repr.rs b/strum_macros/src/macros/from_repr.rs
index ec9f557..811467e 100644
--- a/strum_macros/src/macros/from_repr.rs
+++ b/strum_macros/src/macros/from_repr.rs
@@ -1,6 +1,6 @@
 use heck::ToShoutySnakeCase;
 use proc_macro2::{Span, TokenStream};
-use quote::{format_ident, quote};
+use quote::{format_ident, quote, ToTokens};
 use syn::{Data, DeriveInput, Fields, PathArguments, Type, TypeParen};
 
 use crate::helpers::{non_enum_error, HasStrumVariantProperties};
@@ -14,8 +14,13 @@ pub fn from_repr_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
 
     let mut discriminant_type: Type = syn::parse("usize".parse().unwrap()).unwrap();
     for attr in attrs {
-        let path = &attr.path;
-        let tokens = &attr.tokens;
+        let path = attr.path();
+
+        let mut ts = attr.meta.require_list()?.to_token_stream().into_iter();
+        // Discard the path
+        let _ = ts.next();
+        let tokens: TokenStream = ts.collect();
+
         if path.leading_colon.is_some() {
             continue;
         }
diff --git a/strum_macros/src/macros/strings/as_ref_str.rs b/strum_macros/src/macros/strings/as_ref_str.rs
index 4dfdc4b..617b887 100644
--- a/strum_macros/src/macros/strings/as_ref_str.rs
+++ b/strum_macros/src/macros/strings/as_ref_str.rs
@@ -80,7 +80,7 @@ pub fn as_static_str_inner(
     let mut generics = ast.generics.clone();
     generics
         .params
-        .push(syn::GenericParam::Lifetime(syn::LifetimeDef::new(
+        .push(syn::GenericParam::Lifetime(syn::LifetimeParam::new(
             parse_quote!('_derivative_strum),
         )));
     let (impl_generics2, _, _) = generics.split_for_impl();
diff --git a/strum_macros/src/macros/strings/display.rs b/strum_macros/src/macros/strings/display.rs
index 82a34b0..fcc5936 100644
--- a/strum_macros/src/macros/strings/display.rs
+++ b/strum_macros/src/macros/strings/display.rs
@@ -32,7 +32,21 @@ pub fn display_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
             Fields::Named(..) => quote! { {..} },
         };
 
-        arms.push(quote! { #name::#ident #params => f.pad(#output) });
+        if variant_properties.to_string.is_none() && variant_properties.default.is_some() {
+            match &variant.fields {
+                Fields::Unnamed(fields) if fields.unnamed.len() == 1 => {
+                    arms.push(quote! { #name::#ident(ref s) => f.pad(s) });
+                }
+                _ => {
+                    return Err(syn::Error::new_spanned(
+                        variant,
+                        "Default only works on newtype structs with a single String field",
+                    ))
+                }
+            }
+        } else {
+            arms.push(quote! { #name::#ident #params => f.pad(#output) });
+        }
     }
 
     if arms.len() < variants.len() {
diff --git a/strum_macros/src/macros/strings/to_string.rs b/strum_macros/src/macros/strings/to_string.rs
index 7e4c483..9a1e661 100644
--- a/strum_macros/src/macros/strings/to_string.rs
+++ b/strum_macros/src/macros/strings/to_string.rs
@@ -22,6 +22,22 @@ pub fn to_string_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
             continue;
         }
 
+        // display variants like Green("lime") as "lime"
+        if variant_properties.to_string.is_none() && variant_properties.default.is_some() {
+            match &variant.fields {
+                Fields::Unnamed(fields) if fields.unnamed.len() == 1 => {
+                    arms.push(quote! { #name::#ident(ref s) => ::std::string::String::from(s) });
+                    continue;
+                }
+                _ => {
+                    return Err(syn::Error::new_spanned(
+                        variant,
+                        "Default only works on newtype structs with a single String field",
+                    ))
+                }
+            }
+        }
+
         // Look at all the serialize attributes.
         let output = variant_properties.get_preferred_name(type_properties.case_style);
 


Test patch:
diff --git a/strum_tests/tests/display.rs b/strum_tests/tests/display.rs
index 2b20a56..2e1b996 100644
--- a/strum_tests/tests/display.rs
+++ b/strum_tests/tests/display.rs
@@ -44,6 +44,28 @@ fn to_red_string() {
     assert_eq!(String::from("RedRed"), format!("{}", Color::Red));
 }
 
+#[test]
+fn to_green_string() {
+    assert_eq!(
+        String::from("lime"),
+        format!("{}", Color::Green("lime".into()))
+    );
+}
+
+#[derive(Debug, Eq, PartialEq, EnumString, Display)]
+enum ColorWithDefaultAndToString {
+    #[strum(default, to_string = "GreenGreen")]
+    Green(String),
+}
+
+#[test]
+fn to_green_with_default_and_to_string() {
+    assert_eq!(
+        String::from("GreenGreen"),
+        format!("{}", ColorWithDefaultAndToString::Green("lime".into()))
+    );
+}
+
 #[derive(Display, Debug, Eq, PartialEq)]
 #[strum(serialize_all = "snake_case")]
 enum Brightness {
diff --git a/strum_tests/tests/enum_count.rs b/strum_tests/tests/enum_count.rs
index 794e563..db98110 100644
--- a/strum_tests/tests/enum_count.rs
+++ b/strum_tests/tests/enum_count.rs
@@ -11,12 +11,29 @@ enum Week {
     Saturday,
 }
 
+#[allow(dead_code)]
+#[derive(Debug, EnumCount, EnumIter)]
+enum Pets {
+    Dog,
+    Cat,
+    Fish,
+    Bird,
+    #[strum(disabled)]
+    Hamster,
+}
+
 #[test]
 fn simple_test() {
     assert_eq!(7, Week::COUNT);
     assert_eq!(Week::iter().count(), Week::COUNT);
 }
 
+#[test]
+fn disabled_test() {
+    assert_eq!(4, Pets::COUNT);
+    assert_eq!(Pets::iter().count(), Pets::COUNT);
+}
+
 #[test]
 fn crate_module_path_test() {
     pub mod nested {
diff --git a/strum_tests/tests/to_string.rs b/strum_tests/tests/to_string.rs
index 8b59a9e..91538fc 100644
--- a/strum_tests/tests/to_string.rs
+++ b/strum_tests/tests/to_string.rs
@@ -38,6 +38,36 @@ fn to_red_string() {
     );
 }
 
+#[test]
+fn to_green_string_with_default() {
+    assert_eq!(
+        String::from("lime"),
+        (Color::Green("lime".into())).to_string()
+    );
+    assert_eq!(
+        Color::Green("lime".into()),
+        Color::from_str("lime").unwrap()
+    );
+}
+
+#[derive(Debug, Eq, PartialEq, EnumString, ToString)]
+enum ColorWithDefaultAndToString {
+    #[strum(default, to_string = "GreenGreen")]
+    Green(String),
+}
+
+#[test]
+fn to_green_with_default_and_to_string() {
+    assert_eq!(
+        String::from("GreenGreen"),
+        (ColorWithDefaultAndToString::Green("lime".into())).to_string()
+    );
+    assert_eq!(
+        ColorWithDefaultAndToString::Green("lime".into()),
+        ColorWithDefaultAndToString::from_str("lime").unwrap()
+    );
+}
+
 #[derive(Debug, Eq, PartialEq, ToString)]
 #[strum(serialize_all = "snake_case")]
 enum Brightness {