Fix #269 and #265 (#278)

CHANGELOG.md

@@ -1,3 +1,10 @@
+# v0.3.4
+
+* `rename_all` does not apply to fields that were annotated with explicit
+  `short/long/name = "..."` anymore ([#265](https://github.com/TeXitoi/structopt/issues/265))
+* Now raw idents are handled correctly ([#269](https://github.com/TeXitoi/structopt/issues/269))
+* Some documentation improvements and clarification.
+
 # v0.3.3 (2019-10-10)
 
 * Add `from_flag` custom parser to create flags from non-bool types.

structopt-derive/src/attrs.rs

@@ -15,8 +15,9 @@ use proc_macro2::{Span, TokenStream};
 use proc_macro_error::span_error;
 use quote::{quote, quote_spanned, ToTokens};
 use syn::{
-    self, spanned::Spanned, AngleBracketedGenericArguments, Attribute, Expr, GenericArgument,
-    Ident, LitStr, MetaNameValue, PathArguments, PathSegment, Type::Path, TypePath,
+    self, ext::IdentExt, spanned::Spanned, AngleBracketedGenericArguments, Attribute, Expr,
+    GenericArgument, Ident, LitStr, MetaNameValue, PathArguments, PathSegment, Type::Path,
+    TypePath,
 };
 
 #[derive(Clone)]
@@ -76,10 +77,15 @@ pub enum CasingStyle {
     Verbatim,
 }
 
+#[derive(Clone)]
+pub enum Name {
+    Derived(Ident),
+    Assigned(LitStr),
+}
+
 #[derive(Clone)]
 pub struct Attrs {
-    name: Sp<String>,
-    cased_name: String,
+    name: Name,
     casing: Sp<CasingStyle>,
     methods: Vec<Method>,
     parser: Sp<Parser>,
@@ -148,7 +154,9 @@ impl Parser {
 
         let func = match spec.parse_func {
             None => match kind {
-                FromStr | FromOsStr => quote_spanned!(spec.kind.span()=> ::std::convert::From::from),
+                FromStr | FromOsStr => {
+                    quote_spanned!(spec.kind.span()=> ::std::convert::From::from)
+                }
                 TryFromStr => quote_spanned!(spec.kind.span()=> ::std::str::FromStr::from_str),
                 TryFromOsStr => span_error!(
                     spec.kind.span(),
@@ -171,19 +179,6 @@ impl Parser {
 }
 
 impl CasingStyle {
-    fn translate(&self, input: &str) -> String {
-        use CasingStyle::*;
-
-        match self {
-            Pascal => input.to_camel_case(),
-            Kebab => input.to_kebab_case(),
-            Camel => input.to_mixed_case(),
-            ScreamingSnake => input.to_shouty_snake_case(),
-            Snake => input.to_snake_case(),
-            Verbatim => String::from(input),
-        }
-    }
-
     fn from_lit(name: LitStr) -> Sp<Self> {
         use CasingStyle::*;
 
@@ -202,13 +197,32 @@ impl CasingStyle {
     }
 }
 
-impl Attrs {
-    fn new(default_span: Span, name: Sp<String>, casing: Sp<CasingStyle>) -> Self {
-        let cased_name = casing.translate(&name);
+impl Name {
+    pub fn translate(self, style: &CasingStyle) -> LitStr {
+        use CasingStyle::*;
 
+        match self {
+            Name::Assigned(lit) => lit,
+            Name::Derived(ident) => {
+                let s = ident.unraw().to_string();
+                let s = match style {
+                    Pascal => s.to_camel_case(),
+                    Kebab => s.to_kebab_case(),
+                    Camel => s.to_mixed_case(),
+                    ScreamingSnake => s.to_shouty_snake_case(),
+                    Snake => s.to_snake_case(),
+                    Verbatim => s,
+                };
+                LitStr::new(&s, ident.span())
+            }
+        }
+    }
+}
+
+impl Attrs {
+    fn new(default_span: Span, name: Name, casing: Sp<CasingStyle>) -> Self {
         Self {
             name,
-            cased_name,
             casing,
             methods: vec![],
             parser: Parser::default_spanned(default_span.clone()),
@@ -226,8 +240,7 @@ impl Attrs {
     fn push_str_method(&mut self, name: Sp<String>, arg: Sp<String>) {
         match (&**name, &**arg) {
             ("name", _) => {
-                self.cased_name = self.casing.translate(&arg);
-                self.name = arg;
+                self.name = Name::Assigned(arg.as_lit());
             }
             _ => self
                 .methods
@@ -240,14 +253,11 @@ impl Attrs {
 
         for attr in parse_structopt_attributes(attrs) {
             match attr {
-                Short(ident) => {
-                    let cased_name = Sp::call_site(self.cased_name.clone());
-                    self.push_str_method(ident.into(), cased_name);
-                }
-
-                Long(ident) => {
-                    let cased_name = Sp::call_site(self.cased_name.clone());
-                    self.push_str_method(ident.into(), cased_name);
+                Short(ident) | Long(ident) => {
+                    self.push_str_method(
+                        ident.into(),
+                        self.name.clone().translate(&self.casing).into(),
+                    );
                 }
 
                 Subcommand(ident) => {
@@ -292,7 +302,6 @@ impl Attrs {
 
                 RenameAll(_, casing_lit) => {
                     self.casing = CasingStyle::from_lit(casing_lit);
-                    self.cased_name = self.casing.translate(&self.name);
                 }
 
                 Parse(ident, spec) => {
@@ -390,7 +399,7 @@ impl Attrs {
     pub fn from_struct(
         span: Span,
         attrs: &[Attribute],
-        name: Sp<String>,
+        name: Name,
         argument_casing: Sp<CasingStyle>,
     ) -> Self {
         let mut res = Self::new(span, name, argument_casing);
@@ -443,7 +452,7 @@ impl Attrs {
 
     pub fn from_field(field: &syn::Field, struct_casing: Sp<CasingStyle>) -> Self {
         let name = field.ident.clone().unwrap();
-        let mut res = Self::new(field.span(), name.into(), struct_casing);
+        let mut res = Self::new(field.span(), Name::Derived(name.clone()), struct_casing);
         res.push_doc_comment(&field.attrs, "help");
         res.push_attrs(&field.attrs);
 
@@ -455,7 +464,7 @@ impl Attrs {
                         "parse attribute is not allowed for flattened entry"
                     );
                 }
-                if !res.methods.is_empty() {
+                if res.has_explicit_methods() || res.has_doc_methods() {
                     span_error!(
                         res.kind.span(),
                         "methods and doc comments are not allowed for flattened entry"
@@ -469,9 +478,9 @@ impl Attrs {
                         "parse attribute is not allowed for subcommand"
                     );
                 }
-                if let Some(m) = res.methods.iter().find(|m| m.name != "help") {
+                if res.has_explicit_methods() {
                     span_error!(
-                        m.name.span(),
+                        res.kind.span(),
                         "methods in attributes are not allowed for subcommand"
                     );
                 }
@@ -496,12 +505,11 @@ impl Attrs {
                 res.kind = Sp::new(Kind::Subcommand(ty), res.kind.span());
             }
             Kind::Skip(_) => {
-                if let Some(m) = res
-                    .methods
-                    .iter()
-                    .find(|m| m.name != "help" && m.name != "long_help")
-                {
-                    span_error!(m.name.span(), "methods are not allowed for skipped fields");
+                if res.has_explicit_methods() {
+                    span_error!(
+                        res.kind.span(),
+                        "methods are not allowed for skipped fields"
+                    );
                 }
             }
             Kind::Arg(orig_ty) => {
@@ -531,17 +539,15 @@ impl Attrs {
                         }
                     }
                     Ty::OptionOption => {
-                        // If it's a positional argument.
-                        if !(res.has_method("long") || res.has_method("short")) {
+                        if res.is_positional() {
                             span_error!(
                                 ty.span(),
                                 "Option<Option<T>> type is meaningless for positional argument"
                             )
                         }
                     }
                     Ty::OptionVec => {
-                        // If it's a positional argument.
-                        if !(res.has_method("long") || res.has_method("short")) {
+                        if res.is_positional() {
                             span_error!(
                                 ty.span(),
                                 "Option<Vec<T>> type is meaningless for positional argument"
@@ -604,12 +610,11 @@ impl Attrs {
     /// generate methods on top of a field
     pub fn field_methods(&self) -> TokenStream {
         let methods = &self.methods;
-
         quote!( #(#methods)* )
     }
 
-    pub fn cased_name(&self) -> String {
-        self.cased_name.to_string()
+    pub fn cased_name(&self) -> LitStr {
+        self.name.clone().translate(&self.casing)
     }
 
     pub fn parser(&self) -> &Sp<Parser> {
@@ -623,6 +628,24 @@ impl Attrs {
     pub fn casing(&self) -> Sp<CasingStyle> {
         self.casing.clone()
     }
+
+    pub fn is_positional(&self) -> bool {
+        self.methods
+            .iter()
+            .all(|m| m.name != "long" && m.name != "short")
+    }
+
+    pub fn has_explicit_methods(&self) -> bool {
+        self.methods
+            .iter()
+            .any(|m| m.name != "help" && m.name != "long_help")
+    }
+
+    pub fn has_doc_methods(&self) -> bool {
+        self.methods
+            .iter()
+            .any(|m| m.name == "help" || m.name == "long_help")
+    }
 }
 
 pub fn sub_type(t: &syn::Type) -> Option<&syn::Type> {

structopt-derive/src/lib.rs

@@ -17,7 +17,7 @@ mod parse;
 mod spanned;
 
 use crate::{
-    attrs::{sub_type, Attrs, CasingStyle, Kind, ParserKind, Ty},
+    attrs::{sub_type, Attrs, CasingStyle, Kind, Name, ParserKind, Ty},
     spanned::Sp,
 };
 
@@ -354,7 +354,7 @@ fn gen_clap(attrs: &[Attribute]) -> GenOutput {
     let attrs = Attrs::from_struct(
         Span::call_site(),
         attrs,
-        Sp::call_site(name),
+        Name::Assigned(LitStr::new(&name, Span::call_site())),
         Sp::call_site(DEFAULT_CASING),
     );
     let tokens = {
@@ -424,7 +424,7 @@ fn gen_augment_clap_enum(
         let attrs = Attrs::from_struct(
             variant.span(),
             &variant.attrs,
-            variant.ident.clone().into(),
+            Name::Derived(variant.ident.clone()),
             parent_attribute.casing(),
         );
         let app_var = Ident::new("subcommand", Span::call_site());
@@ -490,7 +490,7 @@ fn gen_from_subcommand(
         let attrs = Attrs::from_struct(
             variant.span(),
             &variant.attrs,
-            variant.ident.clone().into(),
+            Name::Derived(variant.ident.clone()),
             parent_attribute.casing(),
         );
         let sub_name = attrs.cased_name();

structopt-derive/src/spanned.rs

@@ -1,5 +1,5 @@
 use proc_macro2::{Ident, Span, TokenStream};
-use quote::{quote_spanned, ToTokens};
+use quote::ToTokens;
 use std::ops::{Deref, DerefMut};
 use syn::LitStr;
 
@@ -31,6 +31,10 @@ impl<T: ToString> Sp<T> {
     pub fn as_ident(&self) -> Ident {
         Ident::new(&self.to_string(), self.span.clone())
     }
+
+    pub fn as_lit(&self) -> LitStr {
+        LitStr::new(&self.to_string(), self.span.clone())
+    }
 }
 
 impl<T> Deref for Sp<T> {
@@ -85,8 +89,13 @@ impl<T: AsRef<str>> AsRef<str> for Sp<T> {
 
 impl<T: ToTokens> ToTokens for Sp<T> {
     fn to_tokens(&self, stream: &mut TokenStream) {
-        let val = &self.val;
-        let quoted = quote_spanned!(self.span=> #val);
-        stream.extend(quoted);
+        // this is the simplest way out of correct ones to change span on
+        // arbitrary token tree I can come up with
+        let tt = self.val.to_token_stream().into_iter().map(|mut tt| {
+            tt.set_span(self.span.clone());
+            tt
+        });
+
+        stream.extend(tt);
     }
 }

tests/explicit_name_no_renaming.rs

@@ -0,0 +1,32 @@
+use structopt::StructOpt;
+
+#[test]
+fn explicit_short_long_no_rename() {
+    #[derive(StructOpt, PartialEq, Debug)]
+    struct Opt {
+        #[structopt(short = ".", long = ".foo")]
+        foo: Vec<String>,
+    }
+
+    assert_eq!(
+        Opt {
+            foo: vec!["short".into(), "long".into()]
+        },
+        Opt::from_iter(&["test", "-.", "short", "--.foo", "long"])
+    );
+}
+
+#[test]
+fn explicit_name_no_rename() {
+    #[derive(StructOpt, PartialEq, Debug)]
+    struct Opt {
+        #[structopt(name = ".options")]
+        foo: Vec<String>,
+    }
+
+    let mut output = Vec::new();
+    Opt::clap().write_long_help(&mut output).unwrap();
+    let help = String::from_utf8(output).unwrap();
+
+    assert!(help.contains("[.options]..."))
+}

tests/macro-errors.rs

@@ -5,7 +5,7 @@
 // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
 // option. This file may not be copied, modified, or distributed
 
-#[rustversion::attr(not(stable), ignore)]
+#[rustversion::attr(any(not(stable), before(1.39)), ignore)]
 #[test]
 fn ui() {
     let t = trybuild::TestCases::new();

tests/raw_idents.rs

@@ -0,0 +1,17 @@
+use structopt::StructOpt;
+
+#[test]
+fn raw_idents() {
+    #[derive(StructOpt, Debug, PartialEq)]
+    struct Opt {
+        #[structopt(short, long)]
+        r#type: Vec<String>,
+    }
+
+    assert_eq!(
+        Opt {
+            r#type: vec!["long".into(), "short".into()]
+        },
+        Opt::from_iter(&["test", "--type", "long", "-t", "short"])
+    );
+}