From 9a15e47af0796f40c79723aa869242977fb4a37f Mon Sep 17 00:00:00 2001 From: CreepySkeleton Date: Thu, 13 Feb 2020 21:06:11 +0300 Subject: [PATCH 1/2] Fix dummy implementations --- clap_derive/Cargo.toml | 2 +- clap_derive/src/derives/clap.rs | 12 +++-- clap_derive/src/derives/dummies.rs | 54 ++++++++++++++++++++++ clap_derive/src/derives/from_argmatches.rs | 11 +++-- clap_derive/src/derives/into_app.rs | 4 +- clap_derive/src/derives/mod.rs | 1 + clap_derive/src/derives/subcommand.rs | 19 ++------ clap_derive/src/lib.rs | 27 +---------- clap_derive/tests/ui/tuple_struct.stderr | 13 ++++++ 9 files changed, 92 insertions(+), 51 deletions(-) create mode 100644 clap_derive/src/derives/dummies.rs diff --git a/clap_derive/Cargo.toml b/clap_derive/Cargo.toml index 54d28e64..ec85d13a 100644 --- a/clap_derive/Cargo.toml +++ b/clap_derive/Cargo.toml @@ -39,7 +39,7 @@ syn = { version = "1", features = ["full"] } quote = "1" proc-macro2 = "1" heck = "0.3.0" -proc-macro-error = "0.4.3" +proc-macro-error = "0.4.9" [dev-dependencies] clap = { path = "../", version = "3.0.0-beta.1" } diff --git a/clap_derive/src/derives/clap.rs b/clap_derive/src/derives/clap.rs index fc81e7f9..08fb5144 100644 --- a/clap_derive/src/derives/clap.rs +++ b/clap_derive/src/derives/clap.rs @@ -12,7 +12,7 @@ // commit#ea76fa1b1b273e65e3b0b1046643715b49bec51f which is licensed under the // MIT/Apache 2.0 license. -use super::{from_argmatches, into_app, subcommand}; +use super::{dummies, from_argmatches, into_app, subcommand}; use proc_macro2::Ident; use proc_macro_error::abort_call_site; use quote::quote; @@ -27,8 +27,14 @@ pub fn derive_clap(input: &syn::DeriveInput) -> proc_macro2::TokenStream { Struct(syn::DataStruct { fields: syn::Fields::Named(ref fields), .. - }) => gen_for_struct(ident, &fields.named, &input.attrs), - Enum(ref e) => gen_for_enum(ident, &input.attrs, e), + }) => { + dummies::clap_struct(ident); + gen_for_struct(ident, &fields.named, &input.attrs) + } + Enum(ref e) => { + dummies::clap_enum(ident); + gen_for_enum(ident, &input.attrs, e) + } _ => abort_call_site!("`#[derive(Clap)]` only supports non-tuple structs and enums"), } } diff --git a/clap_derive/src/derives/dummies.rs b/clap_derive/src/derives/dummies.rs new file mode 100644 index 00000000..582a7c0f --- /dev/null +++ b/clap_derive/src/derives/dummies.rs @@ -0,0 +1,54 @@ +//! Dummy implementations that we emit along with an error. + +use proc_macro2::Ident; +use proc_macro_error::append_dummy; +use quote::quote; + +pub fn clap_struct(name: &Ident) { + into_app(name); + from_arg_matches(name); + append_dummy(quote!( impl ::clap::Clap for #name {} )); +} + +pub fn clap_enum(name: &Ident) { + into_app(name); + from_arg_matches(name); + subcommand(name); + append_dummy(quote!( impl ::clap::Clap for #name {} )); +} + +pub fn into_app(name: &Ident) { + append_dummy(quote! { + impl ::clap::IntoApp for #name { + fn into_app<'b>() -> ::clap::App<'b> { + unimplemented!() + } + fn augment_clap<'b>(_app: ::clap::App<'b>) -> ::clap::App<'b> { + unimplemented!() + } + } + }); +} + +pub fn from_arg_matches(name: &Ident) { + append_dummy(quote! { + impl ::clap::FromArgMatches for #name { + fn from_arg_matches(_m: &::clap::ArgMatches) -> Self { + unimplemented!() + } + } + }); +} + +pub fn subcommand(name: &Ident) { + append_dummy(quote! { + impl ::clap::Subcommand for #name { + fn from_subcommand(_name: &str, _matches: Option<&::clap::ArgMatches>) -> Option { + unimplemented!() + } + fn augment_subcommands(_app: ::clap::App<'_>) -> ::clap::App<'_> { + unimplemented!() + } + } + }); +} diff --git a/clap_derive/src/derives/from_argmatches.rs b/clap_derive/src/derives/from_argmatches.rs index 1052cebf..b654e8c9 100644 --- a/clap_derive/src/derives/from_argmatches.rs +++ b/clap_derive/src/derives/from_argmatches.rs @@ -18,13 +18,16 @@ use quote::{quote, quote_spanned}; use syn::{punctuated::Punctuated, spanned::Spanned, Token}; use super::{ - spanned::Sp, sub_type, Attrs, Kind, Name, ParserKind, Ty, DEFAULT_CASING, DEFAULT_ENV_CASING, + dummies, spanned::Sp, sub_type, Attrs, Kind, Name, ParserKind, Ty, DEFAULT_CASING, + DEFAULT_ENV_CASING, }; pub fn derive_from_argmatches(input: &syn::DeriveInput) -> proc_macro2::TokenStream { use syn::Data::*; - let struct_name = &input.ident; + let ident = &input.ident; + dummies::from_arg_matches(ident); + match input.data { Struct(syn::DataStruct { fields: syn::Fields::Named(ref fields), @@ -40,9 +43,9 @@ pub fn derive_from_argmatches(input: &syn::DeriveInput) -> proc_macro2::TokenStr Sp::call_site(DEFAULT_ENV_CASING), ); - gen_for_struct(struct_name, &fields.named, &attrs) + gen_for_struct(ident, &fields.named, &attrs) } - Enum(_) => gen_for_enum(struct_name), + Enum(_) => gen_for_enum(ident), _ => { abort_call_site!("#[derive(FromArgMatches)] only supports non-tuple structs and enums") } diff --git a/clap_derive/src/derives/into_app.rs b/clap_derive/src/derives/into_app.rs index 5f44c4d0..aaef906a 100644 --- a/clap_derive/src/derives/into_app.rs +++ b/clap_derive/src/derives/into_app.rs @@ -20,7 +20,7 @@ use quote::{quote, quote_spanned}; use syn::{punctuated::Punctuated, spanned::Spanned, Token}; use super::{ - spanned::Sp, ty::Ty, Attrs, GenOutput, Kind, Name, ParserKind, DEFAULT_CASING, + dummies, spanned::Sp, ty::Ty, Attrs, GenOutput, Kind, Name, ParserKind, DEFAULT_CASING, DEFAULT_ENV_CASING, }; use crate::derives::ty::sub_type; @@ -28,6 +28,8 @@ use crate::derives::ty::sub_type; pub fn derive_into_app(input: &syn::DeriveInput) -> proc_macro2::TokenStream { use syn::Data::*; + dummies::into_app(&input.ident); + match input.data { Struct(syn::DataStruct { fields: syn::Fields::Named(ref fields), diff --git a/clap_derive/src/derives/mod.rs b/clap_derive/src/derives/mod.rs index ce4ceb3b..c14387b2 100644 --- a/clap_derive/src/derives/mod.rs +++ b/clap_derive/src/derives/mod.rs @@ -15,6 +15,7 @@ pub mod arg_enum; pub mod attrs; mod clap; mod doc_comments; +mod dummies; mod from_argmatches; mod into_app; pub mod parse; diff --git a/clap_derive/src/derives/subcommand.rs b/clap_derive/src/derives/subcommand.rs index 79d6dcc8..ecb4c601 100644 --- a/clap_derive/src/derives/subcommand.rs +++ b/clap_derive/src/derives/subcommand.rs @@ -1,8 +1,9 @@ +use super::dummies; use crate::derives::attrs::{Attrs, Kind, Name, DEFAULT_CASING, DEFAULT_ENV_CASING}; use crate::derives::{from_argmatches, into_app, spanned::Sp}; use proc_macro2::{Ident, Span, TokenStream}; -use proc_macro_error::{abort, abort_call_site, set_dummy}; +use proc_macro_error::{abort, abort_call_site}; use quote::{quote, quote_spanned}; use syn::{ punctuated::Punctuated, spanned::Spanned, Attribute, Data, DataEnum, FieldsUnnamed, Token, @@ -11,21 +12,7 @@ use syn::{ pub fn derive_subcommand(input: &syn::DeriveInput) -> TokenStream { let name = &input.ident; - - set_dummy(quote! { - impl ::clap::Subcommand for #name { - fn from_subcommand<'b>( - name: &str, - matches: ::std::option::Option<&::clap::ArgMatches> - ) -> Option { - unimplemented!() - } - - fn augment_subcommands<'b>(app: ::clap::App<'b>) -> ::clap::App<'b> { - unimplemented!() - } - } - }); + dummies::subcommand(name); match input.data { Data::Enum(ref e) => gen_for_enum(name, &input.attrs, e), diff --git a/clap_derive/src/lib.rs b/clap_derive/src/lib.rs index a906bba3..fffabcaa 100644 --- a/clap_derive/src/lib.rs +++ b/clap_derive/src/lib.rs @@ -18,8 +18,7 @@ extern crate proc_macro; -use proc_macro_error::{proc_macro_error, set_dummy}; -use quote::quote; +use proc_macro_error::proc_macro_error; mod derives; @@ -35,7 +34,6 @@ mod derives; #[proc_macro_error] pub fn clap(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input: syn::DeriveInput = syn::parse_macro_input!(input); - set_dummy_clap_impl(&input.ident); derives::derive_clap(&input).into() } @@ -44,7 +42,6 @@ pub fn clap(input: proc_macro::TokenStream) -> proc_macro::TokenStream { #[proc_macro_error] pub fn into_app(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input: syn::DeriveInput = syn::parse_macro_input!(input); - set_dummy_clap_impl(&input.ident); derives::derive_into_app(&input).into() } @@ -53,7 +50,6 @@ pub fn into_app(input: proc_macro::TokenStream) -> proc_macro::TokenStream { #[proc_macro_error] pub fn from_arg_matches(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input: syn::DeriveInput = syn::parse_macro_input!(input); - set_dummy_clap_impl(&input.ident); derives::derive_from_argmatches(&input).into() } @@ -64,24 +60,3 @@ pub fn subcommand(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input: syn::DeriveInput = syn::parse_macro_input!(input); derives::derive_subcommand(&input).into() } - -fn set_dummy_clap_impl(struct_name: &syn::Ident) { - set_dummy(quote! { - impl ::clap::Clap for #struct_name {} - - impl ::clap::IntoApp for #struct_name { - fn into_app<'b>() -> ::clap::App<'b> { - unimplemented!() - } - fn augment_clap<'b>(app: ::clap::App<'b>) -> ::clap::App<'b> { - unimplemented!() - } - } - - impl ::clap::FromArgMatches for #struct_name { - fn from_arg_matches(m: &::clap::ArgMatches) -> Self { - unimplemented!() - } - } - }); -} diff --git a/clap_derive/tests/ui/tuple_struct.stderr b/clap_derive/tests/ui/tuple_struct.stderr index 29ee8c89..3303352e 100644 --- a/clap_derive/tests/ui/tuple_struct.stderr +++ b/clap_derive/tests/ui/tuple_struct.stderr @@ -3,3 +3,16 @@ error: `#[derive(Clap)]` only supports non-tuple structs and enums | 11 | #[derive(Clap, Debug)] | ^^^^ + +error[E0599]: no function or associated item named `parse` found for type `Opt` in the current scope + --> $DIR/tuple_struct.rs:16:20 + | +13 | struct Opt(u32); + | ---------------- function or associated item `parse` not found for this +... +16 | let opt = Opt::parse(); + | ^^^^^ function or associated item not found in `Opt` + | + = help: items from traits can only be used if the trait is implemented and in scope + = note: the following trait defines an item `parse`, perhaps you need to implement it: + candidate #1: `clap::derive::Clap` From 3f314ce39e1eefa98f9dfd2f4a8c6e404a1cf4f1 Mon Sep 17 00:00:00 2001 From: CreepySkeleton Date: Thu, 13 Feb 2020 21:44:43 +0300 Subject: [PATCH 2/2] Use full span information whenever possible --- clap_derive/src/derives/attrs.rs | 26 +++++++++---------- clap_derive/src/derives/parse.rs | 11 ++++---- clap_derive/src/derives/subcommand.rs | 11 ++++---- clap_derive/tests/ui/enum_flatten.stderr | 5 ++-- .../tests/ui/opt_opt_nonpositional.stderr | 2 +- .../tests/ui/opt_vec_nonpositional.stderr | 2 +- .../tests/ui/subcommand_opt_opt.stderr | 2 +- .../tests/ui/subcommand_opt_vec.stderr | 2 +- 8 files changed, 30 insertions(+), 31 deletions(-) diff --git a/clap_derive/src/derives/attrs.rs b/clap_derive/src/derives/attrs.rs index 42a60070..0489946d 100644 --- a/clap_derive/src/derives/attrs.rs +++ b/clap_derive/src/derives/attrs.rs @@ -118,7 +118,7 @@ impl Method { None => match env::var(env_var) { Ok(val) => syn::LitStr::new(&val, ident.span()), Err(_) => { - abort!(ident.span(), + abort!(ident, "cannot derive `{}` from Cargo.toml", ident; note = "`{}` environment variable is not set", env_var; help = "use `{} = \"...\"` to set {} manually", ident, ident; @@ -186,7 +186,7 @@ impl Parser { Some(func) => match func { syn::Expr::Path(_) => quote!(#func), - _ => abort!(func.span(), "`parse` argument must be a function path"), + _ => abort!(func, "`parse` argument must be a function path"), }, }; @@ -210,7 +210,7 @@ impl CasingStyle { "screamingsnake" | "screamingsnakecase" => cs(ScreamingSnake), "snake" | "snakecase" => cs(Snake), "verbatim" | "verbatimcase" => cs(Verbatim), - s => abort!(name.span(), "unsupported casing: `{}`", s), + s => abort!(name, "unsupported casing: `{}`", s), } } } @@ -312,7 +312,7 @@ impl Attrs { ty } else { abort!( - ident.span(), + ident, "#[clap(default_value)] (without an argument) can be used \ only on field level"; @@ -468,13 +468,13 @@ impl Attrs { match *ty { Ty::OptionOption => { abort!( - ty.span(), + field.ty, "Option> type is not allowed for subcommand" ); } Ty::OptionVec => { abort!( - ty.span(), + field.ty, "Option> type is not allowed for subcommand" ); } @@ -503,7 +503,7 @@ impl Attrs { match *ty { Ty::Bool => { if res.is_positional() && !res.has_custom_parser { - abort!(ty.span(), + abort!(field.ty, "`bool` cannot be used as positional parameter with default parser"; help = "if you want to create a flag add `long` or `short`"; help = "If you really want a boolean parameter \ @@ -512,24 +512,24 @@ impl Attrs { ) } if let Some(m) = res.find_method("default_value") { - abort!(m.name.span(), "default_value is meaningless for bool") + abort!(m.name, "default_value is meaningless for bool") } if let Some(m) = res.find_method("required") { - abort!(m.name.span(), "required is meaningless for bool") + abort!(m.name, "required is meaningless for bool") } } Ty::Option => { if let Some(m) = res.find_method("default_value") { - abort!(m.name.span(), "default_value is meaningless for Option") + abort!(m.name, "default_value is meaningless for Option") } if let Some(m) = res.find_method("required") { - abort!(m.name.span(), "required is meaningless for Option") + abort!(m.name, "required is meaningless for Option") } } Ty::OptionOption => { if res.is_positional() { abort!( - ty.span(), + field.ty, "Option> type is meaningless for positional argument" ) } @@ -537,7 +537,7 @@ impl Attrs { Ty::OptionVec => { if res.is_positional() { abort!( - ty.span(), + field.ty, "Option> type is meaningless for positional argument" ) } diff --git a/clap_derive/src/derives/parse.rs b/clap_derive/src/derives/parse.rs index aab83f34..e70512af 100644 --- a/clap_derive/src/derives/parse.rs +++ b/clap_derive/src/derives/parse.rs @@ -6,7 +6,6 @@ use syn::{ self, parenthesized, parse::{Parse, ParseBuffer, ParseStream}, punctuated::Punctuated, - spanned::Spanned, Attribute, Expr, ExprLit, Ident, Lit, LitBool, LitStr, Token, }; @@ -62,7 +61,7 @@ impl Parse for ClapAttr { let check_empty_lit = |s| { if lit_str.is_empty() { abort!( - lit.span(), + lit, "`#[clap({} = \"\")` is deprecated, \ now it's default behavior", s @@ -112,7 +111,7 @@ impl Parse for ClapAttr { } Err(_) => abort! { - assign_token.span(), + assign_token, "expected `string literal` or `expression` after `=`" }, } @@ -130,7 +129,7 @@ impl Parse for ClapAttr { if parser_specs.len() == 1 { Ok(Parse(name, parser_specs[0].clone())) } else { - abort!(name.span(), "parse must have exactly one argument") + abort!(name, "parse must have exactly one argument") } } @@ -145,7 +144,7 @@ impl Parse for ClapAttr { } Err(_) => { - abort!(name.span(), + abort!(name, "`#[clap(raw(...))` attributes are removed, \ they are replaced with raw methods"; help = "if you meant to call `clap::Arg::raw()` method \ @@ -179,7 +178,7 @@ impl Parse for ClapAttr { "skip" => Ok(Skip(name, None)), - _ => abort!(name.span(), "unexpected attribute: {}", name_str), + _ => abort!(name, "unexpected attribute: {}", name_str), } } } diff --git a/clap_derive/src/derives/subcommand.rs b/clap_derive/src/derives/subcommand.rs index ecb4c601..7c1973d2 100644 --- a/clap_derive/src/derives/subcommand.rs +++ b/clap_derive/src/derives/subcommand.rs @@ -67,7 +67,7 @@ fn gen_augment_subcommands( } } _ => abort!( - variant.span(), + variant, "`flatten` is usable only with single-typed tuple variants" ), }, @@ -87,10 +87,9 @@ fn gen_augment_subcommands( } } } - Unnamed(..) => abort!( - variant.span(), - "non single-typed tuple enums are not supported" - ), + Unnamed(..) => { + abort!(variant, "non single-typed tuple enums are not supported") + } }; let name = attrs.cased_name(); @@ -175,7 +174,7 @@ fn gen_from_subcommand( } } _ => abort!( - variant.span(), + variant, "`flatten` is usable only with single-typed tuple variants" ), } diff --git a/clap_derive/tests/ui/enum_flatten.stderr b/clap_derive/tests/ui/enum_flatten.stderr index 30acd20a..d7aeb294 100644 --- a/clap_derive/tests/ui/enum_flatten.stderr +++ b/clap_derive/tests/ui/enum_flatten.stderr @@ -1,5 +1,6 @@ error: `flatten` is usable only with single-typed tuple variants --> $DIR/enum_flatten.rs:14:5 | -14 | #[clap(flatten)] - | ^ +14 | / #[clap(flatten)] +15 | | Variant1, + | |____________^ diff --git a/clap_derive/tests/ui/opt_opt_nonpositional.stderr b/clap_derive/tests/ui/opt_opt_nonpositional.stderr index 586bf7a5..cb9f1728 100644 --- a/clap_derive/tests/ui/opt_opt_nonpositional.stderr +++ b/clap_derive/tests/ui/opt_opt_nonpositional.stderr @@ -2,4 +2,4 @@ error: Option> type is meaningless for positional argument --> $DIR/opt_opt_nonpositional.rs:14:8 | 14 | n: Option>, - | ^^^^^^ + | ^^^^^^^^^^^^^^^^^^^ diff --git a/clap_derive/tests/ui/opt_vec_nonpositional.stderr b/clap_derive/tests/ui/opt_vec_nonpositional.stderr index 6f01de56..c6b343f9 100644 --- a/clap_derive/tests/ui/opt_vec_nonpositional.stderr +++ b/clap_derive/tests/ui/opt_vec_nonpositional.stderr @@ -2,4 +2,4 @@ error: Option> type is meaningless for positional argument --> $DIR/opt_vec_nonpositional.rs:14:8 | 14 | n: Option>, - | ^^^^^^ + | ^^^^^^^^^^^^^^^^ diff --git a/clap_derive/tests/ui/subcommand_opt_opt.stderr b/clap_derive/tests/ui/subcommand_opt_opt.stderr index feb33a4e..1b4eaf8e 100644 --- a/clap_derive/tests/ui/subcommand_opt_opt.stderr +++ b/clap_derive/tests/ui/subcommand_opt_opt.stderr @@ -2,4 +2,4 @@ error: Option> type is not allowed for subcommand --> $DIR/subcommand_opt_opt.rs:17:10 | 17 | cmd: Option>, - | ^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clap_derive/tests/ui/subcommand_opt_vec.stderr b/clap_derive/tests/ui/subcommand_opt_vec.stderr index 9cb4af9e..6cd19157 100644 --- a/clap_derive/tests/ui/subcommand_opt_vec.stderr +++ b/clap_derive/tests/ui/subcommand_opt_vec.stderr @@ -2,4 +2,4 @@ error: Option> type is not allowed for subcommand --> $DIR/subcommand_opt_vec.rs:17:10 | 17 | cmd: Option>, - | ^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^