Change author/about/version behavior

CHANGELOG.md

@@ -3,17 +3,28 @@
 ## Breaking changes
 
 * Bump minimum rust version to 1.34 by [@TeXitoi](https://github.com/TeXitoi)
+
 * Support optional vectors of arguments for distinguishing between `-o 1 2`, `-o` and no option provided at all
  by [@sphynx](https://github.com/sphynx)
  ([#180](https://github.com/TeXitoi/structopt/issues/188)). This is a breaking change
  as `Option<Vec<T>>` is handled differently. If you need to have a `Option<Vec<T>>`
  handled the old was, you can `type Something = Vec<T>;` and then use `Option<Something>`
  as your structopt field.
+
 * Change default case from 'Verbatim' into 'Kebab' by [@0ndorio](https://github.com/0ndorio)
  ([#202](https://github.com/TeXitoi/structopt/issues/202)). This is a breaking change.
  If you rely on the old behavior you need to add `#[structopt(rename_all = "verbatim")]`
  as an attribute to each data structure deriving the `StructOpt` trait.
 
+* Change `version`, `author` and `about` attributes behavior. `version/author/about = ""` is no longer a
+ valid syntax. `author` and `about` are no longer derived from `Cargo.toml` by default, i.e when no
+ attributes mentioned. `version` is still to be derived from `Cargo.toml` by default.
+ Introduced explicit `author` and `about` attributes (with no arguments, i.e `#[structopt(author)]`)
+ for explicit requests to deduce author/about fields from `Cargo.toml`.
+ `version/author/about = "version/author/about"` is still valid syntax.
+ Introduced `no_version` attribute (no args) which prevents version auto deducing by default.
+ Proposed by [@TeXitoi](https://github.com/TeXitoi) [(#217)](https://github.com/TeXitoi/structopt/issues/217), implemented by [@CreepySkeleton](https://github.com/CreepySkeleton) [(#229)](https://github.com/TeXitoi/structopt/pull/229).
+
 ## improvements
 
 * Support skipping struct fields by [@sphynx](https://github.com/sphynx)
@@ -22,6 +33,9 @@
 * Add optional feature to support `paw` by [@gameldar](https://github.com/gameldar)
  ([#187](https://github.com/TeXitoi/structopt/issues/187))
 
+* Significantly improve error reporting by [@CreepySkeleton](https://github.com/CreepySkeleton)
+ ([#225](https://github.com/TeXitoi/structopt/pull/225/))
+
 # v0.2.16 (2019-05-29)
 
 * Support optional options with optional argument, allowing `cmd [--opt[=value]]`

examples/no_version.rs

@@ -4,9 +4,7 @@ use structopt::StructOpt;
 #[derive(StructOpt, Debug)]
 #[structopt(
  name = "no_version",
- about = "",
- version = "",
- author = "",
+ no_version,
  global_settings = &[AppSettings::DisableVersion]
 )]
 struct Opt {}

structopt-derive/src/attrs.rs

@@ -5,18 +5,18 @@
 // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
+
 use crate::spanned::Sp;
+
+use std::env;
+
 use heck::{CamelCase, KebabCase, MixedCase, ShoutySnakeCase, SnakeCase};
 use proc_macro2::{Span, TokenStream};
-use proc_macro_error::span_error;
+use proc_macro_error::{call_site_error, span_error};
 use quote::quote;
-use std::{env, mem};
-use syn::spanned::Spanned as _;
-
-use syn::Type::Path;
 use syn::{
- self, AngleBracketedGenericArguments, Attribute, GenericArgument, Ident, LitStr, MetaNameValue,
- PathArguments, PathSegment, TypePath,
+ self, spanned::Spanned, AngleBracketedGenericArguments, Attribute, GenericArgument, Ident,
+ LitStr, MetaNameValue, PathArguments, PathSegment, Type::Path, TypePath,
 };
 
 use crate::parse::*;
@@ -39,12 +39,13 @@ pub enum Ty {
  Other,
 }
 
+#[derive(Clone)]
 pub struct Method {
  name: Ident,
  args: TokenStream,
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone)]
 pub enum Parser {
  FromStr,
  TryFromStr,
@@ -70,12 +71,17 @@ pub enum CasingStyle {
  Verbatim,
 }
 
+#[derive(Clone)]
 pub struct Attrs {
  name: Sp<String>,
  cased_name: String,
  casing: Sp<CasingStyle>,
  methods: Vec<Method>,
  parser: Sp<(Sp<Parser>, TokenStream)>,
+ author: Option<(Ident, LitStr)>,
+ about: Option<(Ident, LitStr)>,
+ version: Option<(Ident, LitStr)>,
+ no_version: Option<Ident>,
  has_custom_parser: bool,
  kind: Sp<Kind>,
 }
@@ -141,17 +147,19 @@ impl Attrs {
  Sp::call_site(Parser::TryFromStr),
  quote!(::std::str::FromStr::from_str),
  )),
+ about: None,
+ author: None,
+ version: None,
+ no_version: None,
+
  has_custom_parser: false,
  kind: Sp::call_site(Kind::Arg(Sp::call_site(Ty::Other))),
  }
  }
 
+ /// push `.method("str literal")`
  fn push_str_method(&mut self, name: Sp<String>, arg: Sp<String>) {
  match (&**name, &**arg) {
- ("about", "") | ("version", "") | ("author", "") => {
- let methods = mem::replace(&mut self.methods, vec![]);
- self.methods = methods.into_iter().filter(|m| m.name != name).collect();
- }
  ("name", _) => {
  self.cased_name = self.casing.translate(&arg);
  self.name = arg;
@@ -166,6 +174,21 @@ impl Attrs {
  fn push_attrs(&mut self, attrs: &[Attribute]) {
  use crate::parse::StructOptAttr::*;
 
+ fn from_lit_or_env(
+ ident: Ident,
+ lit: Option<LitStr>,
+ env_var: &str,
+ ) -> Option<(Ident, LitStr)> {
+ let lit = lit.unwrap_or_else(|| {
+ let gen = env::var(env_var)
+ .unwrap_or_else(|_|
+ span_error!(ident.span(), "`{}` environment variable is not defined, use `{} = \"{}\"` to set it manually", env_var, env_var, env_var));
+ LitStr::new(&gen, Span::call_site())
+ });
+
+ Some((ident, lit))
+ }
+
  for attr in parse_structopt_attributes(attrs) {
  match attr {
  Short(ident) => {
@@ -194,6 +217,22 @@ impl Attrs {
  self.set_kind(kind);
  }
 
+ NoVersion(ident) => self.no_version = Some(ident),
+
+ About(ident, about) => {
+ self.about = from_lit_or_env(ident, about, "CARGO_PKG_DESCRIPTION")
+ }
+
+ Author(ident, author) => {
+ self.author =
+ from_lit_or_env(ident, author, "CARGO_PKG_AUTHORS").map(|(ident, lit)| {
+ let value = lit.value().replace(":", ", ");
+ (ident.clone(), LitStr::new(&value, ident.span()))
+ })
+ }
+
+ Version(ident, version) => self.version = Some((ident, version)),
+
  NameLitStr(name, lit) => {
  self.push_str_method(name.into(), lit.into());
  }
@@ -205,7 +244,7 @@ impl Attrs {
 
  MethodCall(name, args) => self.methods.push(Method {
  name: name.into(),
- args: quote!(#args),
+ args: quote!(#(#args),*),
  }),
 
  RenameAll(_, casing_lit) => {
@@ -272,6 +311,7 @@ impl Attrs {
  return None;
  }
  let value = s.value();
+
  let text = value
  .trim_start_matches("//!")
  .trim_start_matches("///")
@@ -335,33 +375,16 @@ impl Attrs {
  });
  }
  }
+
  pub fn from_struct(
  attrs: &[Attribute],
  name: Sp<String>,
  argument_casing: Sp<CasingStyle>,
  ) -> Self {
  let mut res = Self::new(name, argument_casing);
- let attrs_with_env = [
- ("version", "CARGO_PKG_VERSION"),
- ("about", "CARGO_PKG_DESCRIPTION"),
- ("author", "CARGO_PKG_AUTHORS"),
- ];
- attrs_with_env
- .iter()
- .filter_map(|&(m, v)| env::var(v).ok().and_then(|arg| Some((m, arg))))
- .filter(|&(_, ref arg)| !arg.is_empty())
- .for_each(|(name, arg)| {
- let new_arg = if name == "author" {
- arg.replace(":", ", ")
- } else {
- arg
- };
- let name = Sp::call_site(name.to_string());
- let new_arg = Sp::call_site(new_arg.to_string());
- res.push_str_method(name, new_arg);
- });
- res.push_doc_comment(attrs, "about");
  res.push_attrs(attrs);
+ res.push_doc_comment(attrs, "about");
+
  if res.has_custom_parser {
  span_error!(
  res.parser.span(),
@@ -379,6 +402,7 @@ impl Attrs {
  Kind::Arg(_) => res,
  }
  }
+
  fn ty_from_field(ty: &syn::Type) -> Sp<Ty> {
  let t = |kind| Sp::new(kind, ty.span());
  if let Path(TypePath {
@@ -404,6 +428,7 @@ impl Attrs {
  t(Ty::Other)
  }
  }
+
  pub fn from_field(field: &syn::Field, struct_casing: Sp<CasingStyle>) -> Self {
  let name = field.ident.clone().unwrap();
  let mut res = Self::new(name.into(), struct_casing);
@@ -463,7 +488,7 @@ impl Attrs {
  span_error!(m.name.span(), "methods are not allowed for skipped fields");
  }
  }
- Kind::Arg(_) => {
+ Kind::Arg(orig_ty) => {
  let mut ty = Self::ty_from_field(&field.ty);
  if res.has_custom_parser {
  match *ty {
@@ -510,7 +535,7 @@ impl Attrs {
 
  _ => (),
  }
- res.kind = Sp::call_site(Kind::Arg(ty));
+ res.kind = Sp::new(Kind::Arg(ty), orig_ty.span());
  }
  }
 
@@ -536,16 +561,59 @@ impl Attrs {
  self.methods.iter().find(|m| m.name == name)
  }
 
- pub fn methods(&self) -> TokenStream {
+ /// generate methods from attributes on top of struct or enum
+ pub fn top_level_methods(&self) -> TokenStream {
+ let version = match (&self.no_version, &self.version) {
+ (Some(no_version), Some(_)) => span_error!(
+ no_version.span(),
+ "`no_version` and `version = \"version\"` can't be used together"
+ ),
+
+ (None, Some((_, version))) => quote!(.version(#version)),
+
+ (None, None) => {
+ let version = env::var("CARGO_PKG_VERSION").unwrap_or_else(|_|{
+ call_site_error!("`CARGO_PKG_VERSION` environment variable is not defined, use `version = \"version\" to set it manually or `no_version` to not set it at all")
+ });
+ quote!(.version(#version))
+ }
+
+ (Some(_), None) => TokenStream::new(),
+ };
+
+ let version = Some(version);
+ let author = self
+ .author
+ .as_ref()
+ .map(|(_, version)| quote!(.author(#version)));
+ let about = self
+ .about
+ .as_ref()
+ .map(|(_, version)| quote!(.about(#version)));
+
+ let methods = self
+ .methods
+ .iter()
+ .map(|&Method { ref name, ref args }| quote!( .#name(#args) ))
+ .chain(version)
+ .chain(author)
+ .chain(about);
+
+ quote!( #(#methods)* )
+ }
+
+ /// generate methods on top of a field
+ pub fn field_methods(&self) -> TokenStream {
  let methods = self
  .methods
  .iter()
  .map(|&Method { ref name, ref args }| quote!( .#name(#args) ));
+
  quote!( #(#methods)* )
  }
 
- pub fn cased_name(&self) -> &str {
- &self.cased_name
+ pub fn cased_name(&self) -> String {
+ self.cased_name.to_string()
  }
 
  pub fn parser(&self) -> &(Sp<Parser>, TokenStream) {

structopt-derive/src/lib.rs

@@ -16,15 +16,15 @@ mod attrs;
 mod parse;
 mod spanned;
 
-use crate::attrs::{sub_type, Attrs, CasingStyle, Kind, Parser, Ty};
-use crate::spanned::Sp;
+use crate::{
+ attrs::{sub_type, Attrs, CasingStyle, Kind, Parser, Ty},
+ spanned::Sp,
+};
+
 use proc_macro2::{Span, TokenStream};
 use proc_macro_error::{call_site_error, filter_macro_errors, span_error};
 use quote::{quote, quote_spanned};
-use syn::punctuated::Punctuated;
-use syn::spanned::Spanned;
-use syn::token::Comma;
-use syn::*;
+use syn::{punctuated::Punctuated, spanned::Spanned, token::Comma, *};
 
 /// Default casing style for generated arguments.
 const DEFAULT_CASING: CasingStyle = CasingStyle::Kebab;
@@ -154,8 +154,9 @@ fn gen_augmentation(
  quote!( .takes_value(true).multiple(false).required(#required) #validator )
  }
  };
- let methods = attrs.methods();
+
  let name = attrs.cased_name();
+ let methods = attrs.field_methods();
 
  Some(quote! {
  let #app_var = #app_var.arg(
@@ -282,7 +283,7 @@ fn gen_clap(attrs: &[Attribute]) -> GenOutput {
  let attrs = Attrs::from_struct(attrs, Sp::call_site(name), Sp::call_site(DEFAULT_CASING));
  let tokens = {
  let name = attrs.cased_name();
- let methods = attrs.methods();
+ let methods = attrs.top_level_methods();
 
  quote!(::structopt::clap::App::new(#name)#methods)
  };
@@ -372,7 +373,8 @@ fn gen_augment_clap_enum(
  };
 
  let name = attrs.cased_name();
- let from_attrs = attrs.methods();
+ let from_attrs = attrs.top_level_methods();
+
  quote! {
  .subcommand({
  let #app_var = ::structopt::clap::SubCommand::with_name(#name);