Repository: Peternator7/strum
Pull request #56 (2019-06-01 11:02:53)

Add `EnumVariantNames`
This derive adds a static `variants()` methods yielding the names of
the enum variants. This happens to be exactly what clap [wants][1], and
is the last puzzle piece to use strum with clap/structopt.

[1]: https://docs.rs/clap/2.33.0/clap/macro.arg_enum.html

Dushistov: > This derive adds a static variants() methods yielding the names of
> the enum variants.

Why introduce array while there is `EnumIter` feature?
killercup: This returns a static array of static string slices and is meant as a convenience method.

My main motivation is this: Clap's [`possible_values`](https://docs.rs/clap/2.33.0/clap/struct.Arg.html#method.possible_values) methods wants a `&[&'b str]`. If you only want to use your enum for this purpose, deriving both EnumIter and ToString (and doing something weird to get from the `Iterator<Item=String>` that `&Enum::iter().map(|x| x.to_string())` returns to that type) seems a bit too much boilerplate.
Dushistov: > If  you only want to use your enum for this purpose, deriving both EnumIter  and ToString (and doing > something weird to get from the Iterator<Item=String> that &Enum::iter().map(\|x\| x.to_string()) 
> returns to that type) seems a bit too much boilerplate.


My main concern that such functionality already exists and new functionality duplicate it.
There are EnumCount, EnumIter and IntoStaticStr.

May be it is possible to combine them to achive what you want without
introducing one more derive, something like:

```Rust
#[macro_use]
extern crate strum_macros;
use strum::{EnumCount, IntoEnumIterator};

#[derive(EnumCount, IntoStaticStr, EnumIter)]
enum Boo {
    A,
    B,
    C,
}

fn get_me_static_array<E, I>() -> [&'static str; 3]
where
    E: IntoEnumIterator<Iterator = I> + EnumCount + Into<&'static str>,
    I: Iterator<Item = E>,
{
    let mut arr: [&'static str; 3] = [""; 3/*something like <E as EnumCount>::COUNT*/];
    for (idx, it) in E::iter().enumerate() {
        arr[idx] = it.into();
    }
    arr
}

fn main() {
    println!("{:?}", get_me_static_array::<Boo, _>());
}
```

This requires only modification of `EnumCount` trait to introduce associated constant.
killercup: Thanks for your comments!

It seems I have misjudged the philosophy of this library, so I'm a bit surprised. You are suggesting to use three of the existing derives (and making one more complex in the process) to not add a new one that is 36 lines of code. I assumed that adding more derives to allow more conveniences was totally in scope. But I guess it is you who will have to maintain this, so it's not my call.

Assuming I agree and we go ahead and introduce `get_me_static_array`. Where would you put the `get_me_static_array` function? In a generic trait in `strum`? How'd you teach people that they need to derive `EnumCount`, `IntoStaticStr`, and `EnumIter` to be able to use that function?

My goal is unchanged; I want to use this with clap/structopt without writing any boilerplate code.
Dushistov: > It seems I have misjudged the philosophy of this library

Take into consideration that I am not maintainer of this crate, I just user and contributor.

And this is point of view of strum user.
I used/use strum in several crates and I always need different things,
like `&[(&'staic str, usize(index))]`,  `[&'static str]` or `[(&'static str, Enum(C like enum))]`,
So I think it is impractical to add 1 of 10 possible use cases into `strum`.
But again this is not maintainer point of view.

> How'd you teach people that they need to derive EnumCount, IntoStaticStr, and EnumIter to be able to > use that function?

I suppose it can be added as generic "free" function,
and because of traits there is no need to teach, there is list of trait in `where bound`.
But again this is not maintainer point of view.
Peternator7: I appreciate the discussion and the PR!

I agree it's a little redundant, but my inclination is to **merge it** anyway. Here's my thinking:

1. This project by it's very nature is about removing boilerplate code even if it is pretty simple to write manually. I know it's impractical to cover all combinations of things that people might want, but supporting a scenario that allows better integration with other popular packages makes sense to me.
2. I've wished `strum` and `clap`/`structopt` had better integration for a while now, but never had time to look into it. This seems like a natural first step towards tighter integration.
3. Philosophically, I try to include as many PR's as reasonably possible. As long as the docs are good, maintenance cost is low, and it makes it easier for people to find them if they're all in 1 package together.

@killercup I'll give the code a review in the next day or 2 :) 

killercup: Ping on review? Not urgent, just one of the dependencies I use a local fork for right now :)
Peternator7: Whoops I forgot to submit the review. thanks for the ping
killercup: Thanks! That's super good feedback! I've pushed some updates that address all your comments and add tests asserting that this actually works with clap and structopt.

Patch:
diff --git a/README.md b/README.md
index 3b696a3..031c2c1 100644
--- a/README.md
+++ b/README.md
@@ -61,6 +61,7 @@ Strum has implemented the following macros:
 | [Display](#Display) | Converts enum variants to strings |
 | [AsRefStr](#AsRefStr) | Converts enum variants to `&'static str` |
 | [IntoStaticStr](#IntoStaticStr) | Implements `From<MyEnum> for &'static str` on an enum |
+| [EnumVariantNames](#EnumVariantNames) | Adds a `variants` method returning an array of discriminant names |
 | [EnumIter](#EnumIter) | Creates a new type that iterates of the variants of an enum. |
 | [EnumProperty](#EnumProperty) | Add custom properties to enum variants. |
 | [EnumMessage](#EnumMessage) | Add a verbose message to an enum variant. |
@@ -191,6 +192,55 @@ fn main() {
 }
 ```
 
+## EnumVariantNames
+
+Adds an `impl` block for the `enum` that adds a static `variants()` method that returns an array of `&'static str` that are the discriminant names.
+This will respect the `serialize_all` attribute on the `enum` (like `#[strum(serialize_all = "snake_case")]`, see **Additional Attributes** below).
+
+**Note:** This is compatible with the format [clap](https://docs.rs/clap/2) expects for `enums`, meaning this works:
+
+```rust
+use strum::{EnumString, EnumVariantNames};
+
+#[derive(EnumString, EnumVariantNames)]
+#[strum(serialize_all = "kebab_case")]
+enum Color {
+    Red,
+    Blue,
+    Yellow,
+    RebeccaPurple,
+}
+
+fn main() {
+    // This is what you get:
+    assert_eq!(
+        &Color::variants(),
+        &["red", "blue", "yellow", "rebecca-purple"]
+    );
+    
+    // Use it with clap like this:
+    let args = clap::App::new("app")
+        .arg(Arg::with_name("color")
+            .long("color")
+            .possible_values(&Color::variants())
+            .case_insensitive(true))
+        .get_matches();
+
+    // ...
+}
+```
+
+This also works with [structopt](https://docs.rs/structopt/0.2) (assuming the same definition of `Color` as above):
+
+```rust
+#[derive(Debug, StructOpt)]
+struct Cli {
+    /// The main color
+    #[structopt(long = "color", default_value = "Color::Blue", raw(possible_values = "&Color::variants()"))]
+    color: Color,
+}
+```
+
 ## EnumIter
 
 Iterate over the variants of an Enum. Any additional data on your variants will be set to `Default::default()`.
diff --git a/strum_macros/Cargo.toml b/strum_macros/Cargo.toml
index af3d40d..65c03dd 100644
--- a/strum_macros/Cargo.toml
+++ b/strum_macros/Cargo.toml
@@ -25,6 +25,7 @@ syn = { version = "0.15", features = ["parsing", "extra-traits"] }
 [features]
 verbose-enumstring-name = []
 verbose-asrefstr-name = []
+verbose-variant-names = []
 verbose-asstaticstr-name = []
 verbose-intostaticstr-name = []
 verbose-tostring-name = []
diff --git a/strum_macros/src/enum_variant_names.rs b/strum_macros/src/enum_variant_names.rs
new file mode 100644
index 0000000..d3b3645
--- /dev/null
+++ b/strum_macros/src/enum_variant_names.rs
@@ -0,0 +1,36 @@
+use proc_macro2::TokenStream;
+use syn;
+
+use case_style::CaseStyle;
+use helpers::{convert_case, extract_meta, unique_attr};
+
+pub fn enum_variant_names_inner(ast: &syn::DeriveInput) -> TokenStream {
+    let name = &ast.ident;
+
+    let variants = match ast.data {
+        syn::Data::Enum(ref v) => &v.variants,
+        _ => panic!("EnumVariantNames only works on Enums"),
+    };
+
+    // Derives for the generated enum
+    let type_meta = extract_meta(&ast.attrs);
+    let case_style = unique_attr(&type_meta, "strum", "serialize_all")
+        .map(|style| CaseStyle::from(style.as_ref()));
+
+    let names = variants
+        .iter()
+        .map(|v| convert_case(&v.ident, case_style))
+        .collect::<Vec<_>>();
+
+    quote! {
+        impl #name {
+            /// Return a slice containing the names of the variants of this enum
+            #[allow(dead_code)]
+            pub fn variants() -> &'static [&'static str] {
+                &[
+                    #(#names),*
+                ]
+            }
+        }
+    }
+}
diff --git a/strum_macros/src/lib.rs b/strum_macros/src/lib.rs
index a9925ef..9abd193 100644
--- a/strum_macros/src/lib.rs
+++ b/strum_macros/src/lib.rs
@@ -22,6 +22,7 @@ mod case_style;
 mod display;
 mod enum_count;
 mod enum_discriminants;
+mod enum_variant_names;
 mod enum_iter;
 mod enum_messages;
 mod enum_properties;
@@ -65,6 +66,16 @@ pub fn as_ref_str(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
     toks.into()
 }
 
+#[cfg_attr(not(feature = "verbose-variant-names"), proc_macro_derive(EnumVariantNames, attributes(strum)))]
+#[cfg_attr(feature = "verbose-variant-names", proc_macro_derive(StrumEnumVariantNames, attributes(strum)))]
+pub fn variant_names(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
+    let ast = syn::parse(input).unwrap();
+
+    let toks = enum_variant_names::enum_variant_names_inner(&ast);
+    debug_print_generated(&ast, &toks);
+    toks.into()
+}
+
 #[cfg_attr(feature = "verbose-asstaticstr-name", proc_macro_derive(StrumAsStaticStr, attributes(strum)))]
 #[cfg_attr(not(feature = "verbose-asstaticstr-name"), proc_macro_derive(AsStaticStr, attributes(strum)))]
 pub fn as_static_str(input: proc_macro::TokenStream) -> proc_macro::TokenStream {


Test patch:
diff --git a/strum_tests/Cargo.toml b/strum_tests/Cargo.toml
index 9458b9e..105ca71 100644
--- a/strum_tests/Cargo.toml
+++ b/strum_tests/Cargo.toml
@@ -5,4 +5,6 @@ authors = ["Peter Glotfelty <peglotfe@microsoft.com>"]
 
 [dependencies]
 strum = { path = "../strum" }
-strum_macros = { path = "../strum_macros", features = [] }
\ No newline at end of file
+strum_macros = { path = "../strum_macros", features = [] }
+clap = "2.33.0"
+structopt = "0.2.18"
diff --git a/strum_tests/tests/enum_variant_names.rs b/strum_tests/tests/enum_variant_names.rs
new file mode 100644
index 0000000..957c14b
--- /dev/null
+++ b/strum_tests/tests/enum_variant_names.rs
@@ -0,0 +1,90 @@
+#[macro_use]
+extern crate strum_macros;
+#[macro_use]
+extern crate structopt;
+extern crate strum;
+
+#[test]
+fn simple() {
+    #[allow(dead_code)]
+    #[derive(EnumVariantNames)]
+    enum Color {
+        Red,
+        Blue,
+        Yellow,
+    }
+
+    assert_eq!(&Color::variants(), &["Red", "Blue", "Yellow"]);
+}
+
+#[test]
+fn plain_kebab() {
+    #[allow(dead_code)]
+    #[derive(EnumVariantNames)]
+    #[strum(serialize_all = "kebab_case")]
+    enum Color {
+        Red,
+        Blue,
+        Yellow,
+        RebeccaPurple,
+    }
+
+    assert_eq!(
+        &Color::variants(),
+        &["red", "blue", "yellow", "rebecca-purple"]
+    );
+}
+
+#[test]
+fn non_plain_camel() {
+    #[allow(dead_code)]
+    #[derive(EnumVariantNames)]
+    #[strum(serialize_all = "kebab_case")]
+    enum Color {
+        DeepPink,
+        GreenYellow,
+        CornflowerBlue,
+        Other { r: u8, g: u8, b: u8 },
+    }
+
+    assert_eq!(
+        &Color::variants(),
+        &["deep-pink", "green-yellow", "cornflower-blue", "other"]
+    );
+}
+
+#[test]
+fn clap_and_structopt() {
+    #[derive(Debug, EnumString, EnumVariantNames)]
+    #[strum(serialize_all = "kebab_case")]
+    enum Color {
+        Red,
+        Blue,
+        Yellow,
+        RebeccaPurple,
+    }
+
+    assert_eq!(
+        &Color::variants(),
+        &["red", "blue", "yellow", "rebecca-purple"]
+    );
+
+    let _clap_example = clap::App::new("app").arg(
+        clap::Arg::with_name("color")
+            .long("color")
+            .possible_values(Color::variants())
+            .case_insensitive(true),
+    );
+
+    #[derive(Debug, StructOpt)]
+    #[allow(unused)]
+    struct StructOptExample {
+        /// The main color
+        #[structopt(
+            long = "color",
+            default_value = "Color::Blue",
+            raw(possible_values = "Color::variants()")
+        )]
+        color: Color,
+    }
+}