diff --git a/clap_derive/src/item.rs b/clap_derive/src/item.rs index 4dba666ddd0..21e02412236 100644 --- a/clap_derive/src/item.rs +++ b/clap_derive/src/item.rs @@ -91,19 +91,6 @@ impl Item { res.push_attrs(attrs); res.push_doc_comment(attrs, "about"); - if let Some(value_parser) = res.value_parser.as_ref() { - abort!( - value_parser.span(), - "`value_parser` attribute is only allowed on fields" - ); - } - if let Some(action) = res.action.as_ref() { - abort!( - action.span(), - "`action` attribute is only allowed on fields" - ); - } - res } @@ -121,18 +108,6 @@ impl Item { match &*res.kind { Kind::Flatten => { - if let Some(value_parser) = res.value_parser.as_ref() { - abort!( - value_parser.span(), - "`value_parser` attribute is not allowed for flattened entry" - ); - } - if let Some(action) = res.action.as_ref() { - abort!( - action.span(), - "`action` attribute is not allowed for flattened entry" - ); - } if res.has_explicit_methods() { abort!( res.kind.span(), @@ -145,19 +120,6 @@ impl Item { } Kind::Subcommand(_) => { - if let Some(value_parser) = res.value_parser.as_ref() { - abort!( - value_parser.span(), - "`value_parser` attribute is not allowed for subcommand" - ); - } - if let Some(action) = res.action.as_ref() { - abort!( - action.span(), - "`action` attribute is not allowed for subcommand" - ); - } - use syn::Fields::*; use syn::FieldsUnnamed; let field_ty = match variant.fields { @@ -223,19 +185,6 @@ impl Item { res.push_attrs(&variant.attrs); res.push_doc_comment(&variant.attrs, "help"); - if let Some(value_parser) = res.value_parser.as_ref() { - abort!( - value_parser.span(), - "`value_parser` attribute is only allowed on fields" - ); - } - if let Some(action) = res.action.as_ref() { - abort!( - action.span(), - "`action` attribute is only allowed on fields" - ); - } - res } @@ -259,18 +208,6 @@ impl Item { match &*res.kind { Kind::Flatten => { - if let Some(value_parser) = res.value_parser.as_ref() { - abort!( - value_parser.span(), - "`value_parser` attribute is not allowed for flattened entry" - ); - } - if let Some(action) = res.action.as_ref() { - abort!( - action.span(), - "`action` attribute is not allowed for flattened entry" - ); - } if res.has_explicit_methods() { abort!( res.kind.span(), @@ -283,18 +220,6 @@ impl Item { } Kind::Subcommand(_) => { - if let Some(value_parser) = res.value_parser.as_ref() { - abort!( - value_parser.span(), - "`value_parser` attribute is not allowed for subcommand" - ); - } - if let Some(action) = res.action.as_ref() { - abort!( - action.span(), - "`action` attribute is not allowed for subcommand" - ); - } if res.has_explicit_methods() { abort!( res.kind.span(), @@ -403,8 +328,41 @@ impl Item { } } - fn push_method(&mut self, name: Ident, arg: impl ToTokens) { - if name == "name" || name == "id" { + fn push_method(&mut self, kind: AttrKind, name: Ident, arg: impl ToTokens) { + if name == "id" { + match kind { + AttrKind::Command | AttrKind::Value => { + self.deprecations.push(Deprecation { + span: name.span(), + id: "id_is_only_for_arg", + version: "4.0.0", + description: format!( + "`#[{}(id)] was allowed by mistake, instead use `#[{}(name)]`", + kind.as_str(), + kind.as_str() + ), + }); + } + AttrKind::Arg | AttrKind::Clap | AttrKind::StructOpt => {} + } + self.name = Name::Assigned(quote!(#arg)); + } else if name == "name" { + match kind { + AttrKind::Arg => { + self.deprecations.push(Deprecation { + span: name.span(), + id: "id_is_only_for_arg", + version: "4.0.0", + description: format!( + "`#[{}(name)] was allowed by mistake, instead use `#[{}(id)]` or `#[{}(value_name)]`", + kind.as_str(), + kind.as_str(), + kind.as_str() + ), + }); + } + AttrKind::Command | AttrKind::Value | AttrKind::Clap | AttrKind::StructOpt => {} + } self.name = Name::Assigned(quote!(#arg)); } else if name == "value_parser" { self.value_parser = Some(ValueParser::Explicit(Method::new(name, quote!(#arg)))); @@ -504,24 +462,31 @@ impl Item { if let Some(AttrValue::Call(tokens)) = &attr.value { // Force raw mode with method call syntax - self.push_method(attr.name.clone(), quote!(#(#tokens),*)); + self.push_method(*attr.kind.get(), attr.name.clone(), quote!(#(#tokens),*)); continue; } match &attr.magic { Some(MagicAttrName::Short) if attr.value.is_none() => { + assert_attr_kind(attr, &[AttrKind::Arg]); + self.push_method( + *attr.kind.get(), attr.name.clone(), self.name.clone().translate_char(*self.casing), ); } Some(MagicAttrName::Long) if attr.value.is_none() => { - self.push_method(attr.name.clone(), self.name.clone().translate(*self.casing)); + assert_attr_kind(attr, &[AttrKind::Arg]); + + self.push_method(*attr.kind.get(), attr.name.clone(), self.name.clone().translate(*self.casing)); } #[cfg(not(feature = "unstable-v5"))] Some(MagicAttrName::ValueParser) if attr.value.is_none() => { + assert_attr_kind(attr, &[AttrKind::Arg]); + self.deprecations.push(Deprecation { span: attr.name.span(), id: "bare_value_parser", @@ -533,6 +498,8 @@ impl Item { #[cfg(not(feature = "unstable-v5"))] Some(MagicAttrName::Action) if attr.value.is_none() => { + assert_attr_kind(attr, &[AttrKind::Arg]); + self.deprecations.push(Deprecation { span: attr.name.span(), id: "bare_action", @@ -543,13 +510,18 @@ impl Item { } Some(MagicAttrName::Env) if attr.value.is_none() => { + assert_attr_kind(attr, &[AttrKind::Arg]); + self.push_method( + *attr.kind.get(), attr.name.clone(), self.name.clone().translate(*self.env_casing), ); } Some(MagicAttrName::ValueEnum) if attr.value.is_none() => { + assert_attr_kind(attr, &[AttrKind::Arg]); + self.is_enum = true } @@ -558,6 +530,8 @@ impl Item { } Some(MagicAttrName::About) if attr.value.is_none() => { + assert_attr_kind(attr, &[AttrKind::Command]); + if let Some(method) = Method::from_env(attr.name.clone(), "CARGO_PKG_DESCRIPTION") { @@ -566,18 +540,24 @@ impl Item { } Some(MagicAttrName::Author) if attr.value.is_none() => { + assert_attr_kind(attr, &[AttrKind::Command]); + if let Some(method) = Method::from_env(attr.name.clone(), "CARGO_PKG_AUTHORS") { self.methods.push(method); } } Some(MagicAttrName::Version) if attr.value.is_none() => { + assert_attr_kind(attr, &[AttrKind::Command]); + if let Some(method) = Method::from_env(attr.name.clone(), "CARGO_PKG_VERSION") { self.methods.push(method); } } Some(MagicAttrName::DefaultValueT) => { + assert_attr_kind(attr, &[AttrKind::Arg]); + let ty = if let Some(ty) = self.ty.as_ref() { ty } else { @@ -620,6 +600,8 @@ impl Item { } Some(MagicAttrName::DefaultValuesT) => { + assert_attr_kind(attr, &[AttrKind::Arg]); + let ty = if let Some(ty) = self.ty.as_ref() { ty } else { @@ -689,6 +671,8 @@ impl Item { } Some(MagicAttrName::DefaultValueOsT) => { + assert_attr_kind(attr, &[AttrKind::Arg]); + let ty = if let Some(ty) = self.ty.as_ref() { ty } else { @@ -731,6 +715,8 @@ impl Item { } Some(MagicAttrName::DefaultValuesOsT) => { + assert_attr_kind(attr, &[AttrKind::Arg]); + let ty = if let Some(ty) = self.ty.as_ref() { ty } else { @@ -800,11 +786,15 @@ impl Item { } Some(MagicAttrName::NextDisplayOrder) => { + assert_attr_kind(attr, &[AttrKind::Command]); + let expr = attr.value_or_abort(); self.next_display_order = Some(Method::new(attr.name.clone(), quote!(#expr))); } Some(MagicAttrName::NextHelpHeading) => { + assert_attr_kind(attr, &[AttrKind::Command]); + let expr = attr.value_or_abort(); self.next_help_heading = Some(Method::new(attr.name.clone(), quote!(#expr))); } @@ -815,6 +805,8 @@ impl Item { } Some(MagicAttrName::RenameAllEnv) => { + assert_attr_kind(attr, &[AttrKind::Command, AttrKind::Arg]); + let lit = attr.lit_str_or_abort(); self.env_casing = CasingStyle::from_lit(lit); } @@ -829,14 +821,14 @@ impl Item { | Some(MagicAttrName::Version) => { let expr = attr.value_or_abort(); - self.push_method(attr.name.clone(), expr); + self.push_method(*attr.kind.get(), attr.name.clone(), expr); } // Magic only for the default, otherwise just forward to the builder #[cfg(not(feature = "unstable-v5"))] Some(MagicAttrName::ValueParser) | Some(MagicAttrName::Action) => { let expr = attr.value_or_abort(); - self.push_method(attr.name.clone(), expr); + self.push_method(*attr.kind.get(), attr.name.clone(), expr); } // Directives that never receive a value @@ -1263,6 +1255,22 @@ impl ToTokens for Deprecation { } } +fn assert_attr_kind(attr: &ClapAttr, possible_kind: &[AttrKind]) { + if !possible_kind.contains(attr.kind.get()) { + let options = possible_kind + .iter() + .map(|k| format!("`#[{}({})]", k.as_str(), attr.name)) + .collect::>(); + abort!( + attr.name, + "Unknown `#[{}({})]` attribute ({} exists)", + attr.kind.as_str(), + attr.name, + options.join(", ") + ); + } +} + /// replace all `:` with `, ` when not inside the `<>` /// /// `"author1:author2:author3" => "author1, author2, author3"` diff --git a/tests/derive/explicit_name_no_renaming.rs b/tests/derive/explicit_name_no_renaming.rs index 7a31b8f73fb..6e0f79c2241 100644 --- a/tests/derive/explicit_name_no_renaming.rs +++ b/tests/derive/explicit_name_no_renaming.rs @@ -27,7 +27,7 @@ fn explicit_short_long_no_rename() { fn explicit_name_no_rename() { #[derive(Parser, PartialEq, Debug)] struct Opt { - #[arg(name = ".options")] + #[arg(id = ".options")] foo: String, } diff --git a/tests/derive/naming.rs b/tests/derive/naming.rs index e8363556656..693856ff133 100644 --- a/tests/derive/naming.rs +++ b/tests/derive/naming.rs @@ -33,7 +33,7 @@ fn test_custom_long_overwrites_default_name() { fn test_standalone_long_uses_previous_defined_custom_name() { #[derive(Parser, Debug, PartialEq)] struct Opt { - #[arg(name = "foo", long)] + #[arg(id = "foo", long)] foo_option: bool, } @@ -47,7 +47,7 @@ fn test_standalone_long_uses_previous_defined_custom_name() { fn test_standalone_long_ignores_afterwards_defined_custom_name() { #[derive(Parser, Debug, PartialEq)] struct Opt { - #[arg(long, name = "foo")] + #[arg(long, id = "foo")] foo_option: bool, } @@ -118,7 +118,7 @@ fn test_custom_short_overwrites_default_name() { fn test_standalone_short_uses_previous_defined_custom_name() { #[derive(Parser, Debug, PartialEq)] struct Opt { - #[arg(name = "option", short)] + #[arg(id = "option", short)] foo_option: bool, } @@ -132,7 +132,7 @@ fn test_standalone_short_uses_previous_defined_custom_name() { fn test_standalone_short_ignores_afterwards_defined_custom_name() { #[derive(Parser, Debug, PartialEq)] struct Opt { - #[arg(short, name = "option")] + #[arg(short, id = "option")] foo_option: bool, } diff --git a/tests/derive/non_literal_attributes.rs b/tests/derive/non_literal_attributes.rs index 5547d86fd7a..69441de2401 100644 --- a/tests/derive/non_literal_attributes.rs +++ b/tests/derive/non_literal_attributes.rs @@ -37,7 +37,7 @@ struct Opt { #[arg(long("values"))] values: Vec, - #[arg(name = "FILE", requires_if("FILE", "values"))] + #[arg(id = "FILE", requires_if("FILE", "values"))] files: Vec, } diff --git a/tests/derive_ui/flatten_and_methods.rs b/tests/derive_ui/flatten_and_methods.rs index e07a2dafc35..137fa5ebe08 100644 --- a/tests/derive_ui/flatten_and_methods.rs +++ b/tests/derive_ui/flatten_and_methods.rs @@ -19,7 +19,7 @@ struct DaemonOpts { #[derive(Parser, Debug)] #[command(name = "basic")] struct Opt { - #[command(short, flatten)] + #[command(flatten, version = "foo")] opts: DaemonOpts, } diff --git a/tests/derive_ui/flatten_and_methods.stderr b/tests/derive_ui/flatten_and_methods.stderr index 48f273f205f..18976de0770 100644 --- a/tests/derive_ui/flatten_and_methods.stderr +++ b/tests/derive_ui/flatten_and_methods.stderr @@ -1,5 +1,5 @@ error: methods are not allowed for flattened entry - --> tests/derive_ui/flatten_and_methods.rs:22:22 + --> tests/derive_ui/flatten_and_methods.rs:22:15 | -22 | #[command(short, flatten)] - | ^^^^^^^ +22 | #[command(flatten, version = "foo")] + | ^^^^^^^ diff --git a/tests/derive_ui/subcommand_and_methods.rs b/tests/derive_ui/subcommand_and_methods.rs index 0b0b4d05591..b88422f671e 100644 --- a/tests/derive_ui/subcommand_and_methods.rs +++ b/tests/derive_ui/subcommand_and_methods.rs @@ -13,7 +13,7 @@ struct MakeCookie { #[arg(short)] s: String, - #[command(subcommand, long)] + #[command(subcommand, version = "foo")] cmd: Command, } diff --git a/tests/derive_ui/subcommand_and_methods.stderr b/tests/derive_ui/subcommand_and_methods.stderr index a3f714c3f5a..24183696c0d 100644 --- a/tests/derive_ui/subcommand_and_methods.stderr +++ b/tests/derive_ui/subcommand_and_methods.stderr @@ -1,5 +1,5 @@ error: methods in attributes are not allowed for subcommand --> tests/derive_ui/subcommand_and_methods.rs:16:15 | -16 | #[command(subcommand, long)] +16 | #[command(subcommand, version = "foo")] | ^^^^^^^^^^