Use arg_name for redacted positionals

This changes redacted positional arguments from using the field name to using the arg name. This allows the field name to be changed without impacting the redaction. In addition, this fixes all the outstanding warnings.
google · Dec 8, 2021 · 3a363a6 · 3a363a6
1 parent 4f7f891
commit 3a363a6
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 51 deletions.
diff --git a/argh/tests/lib.rs b/argh/tests/lib.rs
@@ -176,7 +176,7 @@ fn missing_option_value() {
     struct Cmd {
         #[argh(option)]
         /// fooey
-        msg: String,
+        _msg: String,
     }
 
     let e = Cmd::from_args(&["cmdname"], &["--msg"])
@@ -539,10 +539,10 @@ mod fuchsia_commandline_tools_rubric {
     struct TwoSwitches {
         #[argh(switch, short = 'a')]
         /// a
-        a: bool,
+        _a: bool,
         #[argh(switch, short = 'b')]
         /// b
-        b: bool,
+        _b: bool,
     }
 
     /// Running switches together is not allowed
@@ -558,7 +558,7 @@ mod fuchsia_commandline_tools_rubric {
     struct OneOption {
         #[argh(option)]
         /// some description
-        foo: String,
+        _foo: String,
     }
 
     /// Do not use an equals punctuation or similar to separate the key and value.
@@ -666,15 +666,15 @@ mod fuchsia_commandline_tools_rubric {
     /// A type for testing `--help`/`help`
     struct HelpTopLevel {
         #[argh(subcommand)]
-        sub: HelpFirstSub,
+        _sub: HelpFirstSub,
     }
 
     #[derive(FromArgs, Debug)]
     #[argh(subcommand, name = "first")]
     /// First subcommmand for testing `help`.
     struct HelpFirstSub {
         #[argh(subcommand)]
-        sub: HelpSecondSub,
+        _sub: HelpSecondSub,
     }
 
     #[derive(FromArgs, Debug)]
@@ -912,7 +912,7 @@ fn redact_arg_values_no_args() {
     struct Cmd {
         #[argh(option)]
         /// a msg param
-        msg: Option<String>,
+        _msg: Option<String>,
     }
 
     let actual = Cmd::redact_arg_values(&["program-name"], &[]).unwrap();
@@ -926,25 +926,53 @@ fn redact_arg_values_optional_arg() {
     struct Cmd {
         #[argh(option)]
         /// a msg param
-        msg: Option<String>,
+        _msg: Option<String>,
     }
 
     let actual = Cmd::redact_arg_values(&["program-name"], &["--msg", "hello"]).unwrap();
     assert_eq!(actual, &["program-name", "--msg"]);
 }
 
+#[test]
+fn redact_arg_values_optional_arg_short() {
+    #[derive(FromArgs, Debug)]
+    /// Short description
+    struct Cmd {
+        #[argh(option, short = 'm')]
+        /// a msg param
+        _msg: Option<String>,
+    }
+
+    let actual = Cmd::redact_arg_values(&["program-name"], &["-m", "hello"]).unwrap();
+    assert_eq!(actual, &["program-name", "-m"]);
+}
+
+#[test]
+fn redact_arg_values_optional_arg_long() {
+    #[derive(FromArgs, Debug)]
+    /// Short description
+    struct Cmd {
+        #[argh(option, long = "my-msg")]
+        /// a msg param
+        _msg: Option<String>,
+    }
+
+    let actual = Cmd::redact_arg_values(&["program-name"], &["--my-msg", "hello"]).unwrap();
+    assert_eq!(actual, &["program-name", "--my-msg"]);
+}
+
 #[test]
 fn redact_arg_values_two_option_args() {
     #[derive(FromArgs, Debug)]
     /// Short description
     struct Cmd {
         #[argh(option)]
         /// a msg param
-        msg: String,
+        _msg: String,
 
         #[argh(option)]
         /// a delivery param
-        delivery: String,
+        _delivery: String,
     }
 
     let actual =
@@ -960,11 +988,11 @@ fn redact_arg_values_option_one_optional_args() {
     struct Cmd {
         #[argh(option)]
         /// a msg param
-        msg: String,
+        _msg: String,
 
         #[argh(option)]
         /// a delivery param
-        delivery: Option<String>,
+        _delivery: Option<String>,
     }
 
     let actual =
@@ -983,7 +1011,7 @@ fn redact_arg_values_switch() {
     struct Cmd {
         #[argh(switch, short = 'f')]
         /// speed of cmd
-        faster: bool,
+        _faster: bool,
     }
 
     let actual = Cmd::redact_arg_values(&["program-name"], &["--faster"]).unwrap();
@@ -998,6 +1026,7 @@ fn redact_arg_values_positional() {
     #[derive(FromArgs, Debug)]
     /// Short description
     struct Cmd {
+        #[allow(dead_code)]
         #[argh(positional)]
         /// speed of cmd
         speed: u8,
@@ -1007,14 +1036,28 @@ fn redact_arg_values_positional() {
     assert_eq!(actual, &["program-name", "speed"]);
 }
 
+#[test]
+fn redact_arg_values_positional_arg_name() {
+    #[derive(FromArgs, Debug)]
+    /// Short description
+    struct Cmd {
+        #[argh(positional, arg_name = "speed")]
+        /// speed of cmd
+        _speed: u8,
+    }
+
+    let actual = Cmd::redact_arg_values(&["program-name"], &["5"]).unwrap();
+    assert_eq!(actual, &["program-name", "speed"]);
+}
+
 #[test]
 fn redact_arg_values_positional_repeating() {
     #[derive(FromArgs, Debug)]
     /// Short description
     struct Cmd {
-        #[argh(positional)]
+        #[argh(positional, arg_name = "speed")]
         /// speed of cmd
-        speed: Vec<u8>,
+        _speed: Vec<u8>,
     }
 
     let actual = Cmd::redact_arg_values(&["program-name"], &["5", "6"]).unwrap();
@@ -1026,9 +1069,9 @@ fn redact_arg_values_positional_err() {
     #[derive(FromArgs, Debug)]
     /// Short description
     struct Cmd {
-        #[argh(positional)]
+        #[argh(positional, arg_name = "speed")]
         /// speed of cmd
-        speed: u8,
+        _speed: u8,
     }
 
     let actual = Cmd::redact_arg_values(&["program-name"], &[]).unwrap_err();
@@ -1046,13 +1089,13 @@ fn redact_arg_values_two_positional() {
     #[derive(FromArgs, Debug)]
     /// Short description
     struct Cmd {
-        #[argh(positional)]
+        #[argh(positional, arg_name = "speed")]
         /// speed of cmd
-        speed: u8,
+        _speed: u8,
 
-        #[argh(positional)]
+        #[argh(positional, arg_name = "direction")]
         /// direction
-        direction: String,
+        _direction: String,
     }
 
     let actual = Cmd::redact_arg_values(&["program-name"], &["5", "north"]).unwrap();
@@ -1064,13 +1107,13 @@ fn redact_arg_values_positional_option() {
     #[derive(FromArgs, Debug)]
     /// Short description
     struct Cmd {
-        #[argh(positional)]
+        #[argh(positional, arg_name = "speed")]
         /// speed of cmd
-        speed: u8,
+        _speed: u8,
 
         #[argh(option)]
         /// direction
-        direction: String,
+        _direction: String,
     }
 
     let actual = Cmd::redact_arg_values(&["program-name"], &["5", "--direction", "north"]).unwrap();
@@ -1082,13 +1125,13 @@ fn redact_arg_values_positional_optional_option() {
     #[derive(FromArgs, Debug)]
     /// Short description
     struct Cmd {
-        #[argh(positional)]
+        #[argh(positional, arg_name = "speed")]
         /// speed of cmd
-        speed: u8,
+        _speed: u8,
 
         #[argh(option)]
         /// direction
-        direction: Option<String>,
+        _direction: Option<String>,
     }
 
     let actual = Cmd::redact_arg_values(&["program-name"], &["5"]).unwrap();
@@ -1100,13 +1143,13 @@ fn redact_arg_values_subcommand() {
     #[derive(FromArgs, Debug)]
     /// Short description
     struct Cmd {
-        #[argh(positional)]
+        #[argh(positional, arg_name = "speed")]
         /// speed of cmd
-        speed: u8,
+        _speed: u8,
 
         #[argh(subcommand)]
         /// means of transportation
-        means: MeansSubcommand,
+        _means: MeansSubcommand,
     }
 
     #[derive(FromArgs, Debug)]
@@ -1124,7 +1167,7 @@ fn redact_arg_values_subcommand() {
     struct WalkingSubcommand {
         #[argh(option)]
         /// a song to listen to
-        music: String,
+        _music: String,
     }
 
     #[derive(FromArgs, Debug)]
@@ -1146,13 +1189,13 @@ fn redact_arg_values_subcommand_with_space_in_name() {
     #[derive(FromArgs, Debug)]
     /// Short description
     struct Cmd {
-        #[argh(positional)]
+        #[argh(positional, arg_name = "speed")]
         /// speed of cmd
-        speed: u8,
+        _speed: u8,
 
         #[argh(subcommand)]
         /// means of transportation
-        means: MeansSubcommand,
+        _means: MeansSubcommand,
     }
 
     #[derive(FromArgs, Debug)]
@@ -1169,7 +1212,7 @@ fn redact_arg_values_subcommand_with_space_in_name() {
     struct WalkingSubcommand {
         #[argh(option)]
         /// a song to listen to
-        music: String,
+        _music: String,
     }
 
     #[derive(FromArgs, Debug)]

diff --git a/argh_derive/src/help.rs b/argh_derive/src/help.rs
@@ -11,7 +11,6 @@ use {
     argh_shared::INDENT,
     proc_macro2::{Span, TokenStream},
     quote::quote,
-    syn::LitStr,
 };
 
 const SECTION_SEPARATOR: &str = "\n\n";
@@ -131,22 +130,13 @@ fn lits_section(out: &mut String, heading: &str, lits: &[syn::LitStr]) {
     }
 }
 
-fn get_positional_name(field: &StructField<'_>) -> String {
-    return field
-        .attrs
-        .arg_name
-        .as_ref()
-        .map(LitStr::value)
-        .unwrap_or_else(|| field.name.to_string());
-}
-
 /// Add positional arguments like `[<foo>...]` to a help format string.
 fn positional_usage(out: &mut String, field: &StructField<'_>) {
     if !field.optionality.is_required() {
         out.push('[');
     }
     out.push('<');
-    let name = get_positional_name(field);
+    let name = field.arg_name();
     out.push_str(&name);
     if field.optionality == Optionality::Repeating {
         out.push_str("...");
@@ -219,7 +209,7 @@ Add a doc comment or an `#[argh(description = \"...\")]` attribute.",
 /// Describes a positional argument like this:
 ///  hello       positional argument description
 fn positional_description(out: &mut String, field: &StructField<'_>) {
-    let field_name = get_positional_name(field);
+    let field_name = field.arg_name();
 
     let mut description = String::from("");
     if let Some(desc) = &field.attrs.description {

diff --git a/argh_derive/src/lib.rs b/argh_derive/src/lib.rs
@@ -16,7 +16,7 @@ use {
     proc_macro2::{Span, TokenStream},
     quote::{quote, quote_spanned, ToTokens},
     std::str::FromStr,
-    syn::spanned::Spanned,
+    syn::{spanned::Spanned, LitStr},
 };
 
 mod errors;
@@ -203,6 +203,10 @@ impl<'a> StructField<'a> {
 
         Some(StructField { field, attrs, kind, optionality, ty_without_wrapper, name, long_name })
     }
+
+    pub(crate) fn arg_name(&self) -> String {
+        self.attrs.arg_name.as_ref().map(LitStr::value).unwrap_or_else(|| self.name.to_string())
+    }
 }
 
 /// Implements `FromArgs` and `TopLevelCommand` or `SubCommand` for a `#[derive(FromArgs)]` struct.
@@ -640,12 +644,12 @@ fn declare_local_storage_for_redacted_fields<'a>(
                     }
                 };
 
-                let long_name = field.name.to_string();
+                let arg_name = field.arg_name();
                 quote! {
                     let mut #field_name: argh::ParseValueSlotTy::<#field_slot_type, String> =
                         argh::ParseValueSlotTy {
                         slot: std::default::Default::default(),
-                        parse_func: |_, _| { Ok(#long_name.to_string()) },
+                        parse_func: |_, _| { Ok(#arg_name.to_string()) },
                     };
                 }
             }
@@ -718,7 +722,7 @@ fn append_missing_requirements<'a>(
         match field.kind {
             FieldKind::Switch => unreachable!("switches are always optional"),
             FieldKind::Positional => {
-                let name = field.name.to_string();
+                let name = field.arg_name();
                 quote! {
                     if #field_name.slot.is_none() {
                         #mri.missing_positional_arg(#name)