Skip to content

Commit

Permalink
fix(derive): Deprecate using name for Args
Browse files Browse the repository at this point in the history
The builder function was deprecated in v3 and removed in v4 but the
derive masked that, so we're still adapting things but now with a path
towards removal.
  • Loading branch information
epage committed Sep 3, 2022
1 parent f9ad2c5 commit a7ed5d0
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 12 deletions.
47 changes: 41 additions & 6 deletions clap_derive/src/item.rs
Expand Up @@ -328,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))));
Expand Down Expand Up @@ -429,7 +462,7 @@ 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;
}

Expand All @@ -438,6 +471,7 @@ impl Item {
assert_attr_kind(attr, &[AttrKind::Arg]);

self.push_method(
*attr.kind.get(),
attr.name.clone(),
self.name.clone().translate_char(*self.casing),
);
Expand All @@ -446,7 +480,7 @@ impl Item {
Some(MagicAttrName::Long) if attr.value.is_none() => {
assert_attr_kind(attr, &[AttrKind::Arg]);

self.push_method(attr.name.clone(), self.name.clone().translate(*self.casing));
self.push_method(*attr.kind.get(), attr.name.clone(), self.name.clone().translate(*self.casing));
}

#[cfg(not(feature = "unstable-v5"))]
Expand Down Expand Up @@ -479,6 +513,7 @@ impl Item {
assert_attr_kind(attr, &[AttrKind::Arg]);

self.push_method(
*attr.kind.get(),
attr.name.clone(),
self.name.clone().translate(*self.env_casing),
);
Expand Down Expand Up @@ -786,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
Expand Down
2 changes: 1 addition & 1 deletion tests/derive/explicit_name_no_renaming.rs
Expand Up @@ -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,
}

Expand Down
8 changes: 4 additions & 4 deletions tests/derive/naming.rs
Expand Up @@ -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,
}

Expand All @@ -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,
}

Expand Down Expand Up @@ -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,
}

Expand All @@ -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,
}

Expand Down
2 changes: 1 addition & 1 deletion tests/derive/non_literal_attributes.rs
Expand Up @@ -37,7 +37,7 @@ struct Opt {
#[arg(long("values"))]
values: Vec<i32>,

#[arg(name = "FILE", requires_if("FILE", "values"))]
#[arg(id = "FILE", requires_if("FILE", "values"))]
files: Vec<String>,
}

Expand Down

0 comments on commit a7ed5d0

Please sign in to comment.