Merge pull request #3452 from epage/lit

refactor(derive): Remove redundant code paths
clap-rs · Feb 11, 2022 · cb06496 · cb06496
2 parents 416ac71 + 85908ce
commit cb06496
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 71 deletions.
diff --git a/clap_derive/src/attrs.rs b/clap_derive/src/attrs.rs
@@ -43,8 +43,6 @@ pub struct Attrs {
     doc_comment: Vec<Method>,
     methods: Vec<Method>,
     parser: Sp<Parser>,
-    author: Option<Method>,
-    version: Option<Method>,
     verbatim_doc_comment: Option<Ident>,
     next_display_order: Option<Method>,
     next_help_heading: Option<Method>,
@@ -378,8 +376,6 @@ impl Attrs {
             doc_comment: vec![],
             methods: vec![],
             parser: Parser::default_spanned(default_span),
-            author: None,
-            version: None,
             verbatim_doc_comment: None,
             next_display_order: None,
             next_help_heading: None,
@@ -393,10 +389,8 @@ impl Attrs {
     fn push_method(&mut self, name: Ident, arg: impl ToTokens) {
         if name == "name" {
             self.name = Name::Assigned(quote!(#arg));
-        } else if name == "version" {
-            self.version = Some(Method::new(name, quote!(#arg)));
         } else {
-            self.methods.push(Method::new(name, quote!(#arg)))
+            self.methods.push(Method::new(name, quote!(#arg)));
         }
     }
 
@@ -547,20 +541,22 @@ impl Attrs {
                     self.next_help_heading = Some(Method::new(ident, quote!(#expr)));
                 }
 
-                About(ident, about) => {
-                    if let Some(method) =
-                        Method::from_lit_or_env(ident, about, "CARGO_PKG_DESCRIPTION")
-                    {
+                About(ident) => {
+                    if let Some(method) = Method::from_env(ident, "CARGO_PKG_DESCRIPTION") {
                         self.methods.push(method);
                     }
                 }
 
-                Author(ident, author) => {
-                    self.author = Method::from_lit_or_env(ident, author, "CARGO_PKG_AUTHORS");
+                Author(ident) => {
+                    if let Some(method) = Method::from_env(ident, "CARGO_PKG_AUTHORS") {
+                        self.methods.push(method);
+                    }
                 }
 
-                Version(ident, version) => {
-                    self.version = Method::from_lit_or_env(ident, version, "CARGO_PKG_VERSION");
+                Version(ident) => {
+                    if let Some(method) = Method::from_env(ident, "CARGO_PKG_VERSION") {
+                        self.methods.push(method);
+                    }
                 }
 
                 NameLitStr(name, lit) => {
@@ -645,12 +641,10 @@ impl Attrs {
     }
 
     pub fn final_top_level_methods(&self) -> TokenStream {
-        let version = &self.version;
-        let author = &self.author;
         let methods = &self.methods;
         let doc_comment = &self.doc_comment;
 
-        quote!( #(#doc_comment)* #author #version #(#methods)*)
+        quote!( #(#doc_comment)* #(#methods)*)
     }
 
     /// generate methods on top of a field
@@ -756,25 +750,21 @@ impl Method {
         Method { name, args }
     }
 
-    fn from_lit_or_env(ident: Ident, lit: Option<LitStr>, env_var: &str) -> Option<Self> {
-        let mut lit = match lit {
-            Some(lit) => lit,
-
-            None => match env::var(env_var) {
-                Ok(val) => {
-                    if val.is_empty() {
-                        return None;
-                    }
-                    LitStr::new(&val, ident.span())
+    fn from_env(ident: Ident, env_var: &str) -> Option<Self> {
+        let mut lit = match env::var(env_var) {
+            Ok(val) => {
+                if val.is_empty() {
+                    return None;
                 }
-                Err(_) => {
-                    abort!(ident,
-                        "cannot derive `{}` from Cargo.toml", ident;
-                        note = "`{}` environment variable is not set", env_var;
-                        help = "use `{} = \"...\"` to set {} manually", ident, ident;
-                    );
-                }
-            },
+                LitStr::new(&val, ident.span())
+            }
+            Err(_) => {
+                abort!(ident,
+                    "cannot derive `{}` from Cargo.toml", ident;
+                    note = "`{}` environment variable is not set", env_var;
+                    help = "use `{} = \"...\"` to set {} manually", ident, ident;
+                );
+            }
         };
 
         if ident == "author" {

diff --git a/clap_derive/src/parse.rs b/clap_derive/src/parse.rs
@@ -33,11 +33,9 @@ pub enum ClapAttr {
     Subcommand(Ident),
     VerbatimDocComment(Ident),
     ExternalSubcommand(Ident),
-
-    // ident [= "string literal"]
-    About(Ident, Option<LitStr>),
-    Author(Ident, Option<LitStr>),
-    Version(Ident, Option<LitStr>),
+    About(Ident),
+    Author(Ident),
+    Version(Ident),
 
     // ident = "string literal"
     RenameAllEnv(Ident, LitStr),
@@ -75,38 +73,11 @@ impl Parse for ClapAttr {
 
             if input.peek(LitStr) {
                 let lit: LitStr = input.parse()?;
-                let lit_str = lit.value();
-
-                let check_empty_lit = |s| {
-                    if lit_str.is_empty() {
-                        abort!(
-                            lit,
-                            "`#[clap({} = \"\")` is deprecated, \
-                             now it's default behavior",
-                            s
-                        );
-                    }
-                };
 
                 match &*name_str {
                     "rename_all" => Ok(RenameAll(name, lit)),
                     "rename_all_env" => Ok(RenameAllEnv(name, lit)),
 
-                    "version" => {
-                        check_empty_lit("version");
-                        Ok(Version(name, Some(lit)))
-                    }
-
-                    "author" => {
-                        check_empty_lit("author");
-                        Ok(Author(name, Some(lit)))
-                    }
-
-                    "about" => {
-                        check_empty_lit("about");
-                        Ok(About(name, Some(lit)))
-                    }
-
                     "skip" => {
                         let expr = ExprLit {
                             attrs: vec![],
@@ -227,9 +198,9 @@ impl Parse for ClapAttr {
                 }
                 "default_value_t" => Ok(DefaultValueT(name, None)),
                 "default_value_os_t" => Ok(DefaultValueOsT(name, None)),
-                "about" => (Ok(About(name, None))),
-                "author" => (Ok(Author(name, None))),
-                "version" => Ok(Version(name, None)),
+                "about" => (Ok(About(name))),
+                "author" => (Ok(Author(name))),
+                "version" => Ok(Version(name)),
 
                 "skip" => Ok(Skip(name, None)),