Merge pull request #4292 from epage/func

refactor: Use getters internally

src/builder/arg.rs

@@ -50,7 +50,6 @@ use crate::INTERNAL_ERROR_MSG;
 /// // Using a usage string (setting a similar argument to the one above)
 /// let input = arg!(-i --input <FILE> "Provides an input file to the program");
 /// ```
-#[allow(missing_debug_implementations)]
 #[derive(Default, Clone)]
 pub struct Arg {
     pub(crate) id: Id,
@@ -3904,7 +3903,7 @@ impl Arg {
     /// assert_eq!(arg.is_positional(), false);
     /// ```
     pub fn is_positional(&self) -> bool {
-        self.long.is_none() && self.short.is_none()
+        self.get_long().is_none() && self.get_short().is_none()
     }
 
     /// Reports whether [`Arg::required`] is set
@@ -4236,7 +4235,7 @@ impl PartialOrd for Arg {
 
 impl Ord for Arg {
     fn cmp(&self, other: &Arg) -> Ordering {
-        self.id.cmp(&other.id)
+        self.get_id().cmp(other.get_id())
     }
 }
 

src/builder/command.rs

@@ -3502,7 +3502,7 @@ impl Command {
                             .iter()
                             .flat_map(|x| x.args.args()),
                     )
-                    .find(|arg| arg.id == *id)
+                    .find(|arg| arg.get_id() == id)
                     .expect(
                         "Command::get_arg_conflicts_with: \
                     The passed arg conflicts with an arg unknown to the cmd",
@@ -3527,7 +3527,11 @@ impl Command {
     fn get_subcommands_containing(&self, arg: &Arg) -> Vec<&Self> {
         let mut vec = std::vec::Vec::new();
         for idx in 0..self.subcommands.len() {
-            if self.subcommands[idx].args.args().any(|ar| ar.id == arg.id) {
+            if self.subcommands[idx]
+                .args
+                .args()
+                .any(|ar| ar.get_id() == arg.get_id())
+            {
                 vec.push(&self.subcommands[idx]);
                 vec.append(&mut self.subcommands[idx].get_subcommands_containing(arg));
             }
@@ -4107,7 +4111,7 @@ impl Command {
             let args_missing_help: Vec<Id> = self
                 .args
                 .args()
-                .filter(|arg| arg.help.is_none() && arg.long_help.is_none())
+                .filter(|arg| arg.get_help().is_none() && arg.get_long_help().is_none())
                 .map(|arg| arg.get_id().clone())
                 .collect();
 
@@ -4353,7 +4357,7 @@ impl Command {
     }
 
     pub(crate) fn find(&self, arg_id: &Id) -> Option<&Arg> {
-        self.args.args().find(|a| a.id == *arg_id)
+        self.args.args().find(|a| a.get_id() == arg_id)
     }
 
     #[inline]
@@ -4413,7 +4417,7 @@ impl Command {
 
     #[cfg(debug_assertions)]
     pub(crate) fn id_exists(&self, id: &Id) -> bool {
-        self.args.args().any(|x| x.id == *id) || self.groups.iter().any(|x| x.id == *id)
+        self.args.args().any(|x| x.get_id() == id) || self.groups.iter().any(|x| x.id == *id)
     }
 
     /// Iterate through the groups this arg is member of.
@@ -4443,7 +4447,7 @@ impl Command {
     pub(crate) fn required_graph(&self) -> ChildGraph<Id> {
         let mut reqs = ChildGraph::with_capacity(5);
         for a in self.args.args().filter(|a| a.is_required_set()) {
-            reqs.insert(a.id.clone());
+            reqs.insert(a.get_id().clone());
         }
         for group in &self.groups {
             if group.required {
@@ -4506,7 +4510,7 @@ impl Command {
                 for r in arg.requires.iter().filter_map(&func) {
                     if let Some(req) = self.find(&r) {
                         if !req.requires.is_empty() {
-                            r_vec.push(&req.id)
+                            r_vec.push(req.get_id())
                         }
                     }
                     args.push(r);
@@ -4571,7 +4575,7 @@ impl Command {
         // specified by the user is sent through. If hide_short_help is not included,
         // then items specified with hidden_short_help will also be hidden.
         let should_long = |v: &Arg| {
-            v.long_help.is_some()
+            v.get_long_help().is_some()
                 || v.is_hide_long_help_set()
                 || v.is_hide_short_help_set()
                 || v.get_possible_values()

src/builder/debug_asserts.rs

@@ -91,7 +91,7 @@ pub(crate) fn assert_app(cmd: &Command) {
         }
 
         // Name conflicts
-        if let Some((first, second)) = cmd.two_args_of(|x| x.id == arg.id) {
+        if let Some((first, second)) = cmd.two_args_of(|x| x.get_id() == arg.get_id()) {
             panic!(
             "Command {}: Argument names must be unique, but '{}' is in use by more than one argument or group{}",
             cmd.get_name(),
@@ -117,7 +117,7 @@ pub(crate) fn assert_app(cmd: &Command) {
 
         // Short conflicts
         if let Some(s) = arg.get_short() {
-            if let Some((first, second)) = cmd.two_args_of(|x| x.short == Some(s)) {
+            if let Some((first, second)) = cmd.two_args_of(|x| x.get_short() == Some(s)) {
                 panic!(
                     "Command {}: Short option names must be unique for each argument, \
                             but '-{}' is in use by both '{}' and '{}'{}",
@@ -133,7 +133,7 @@ pub(crate) fn assert_app(cmd: &Command) {
         // Index conflicts
         if let Some(idx) = arg.index {
             if let Some((first, second)) =
-                cmd.two_args_of(|x| x.is_positional() && x.index == Some(idx))
+                cmd.two_args_of(|x| x.is_positional() && x.get_index() == Some(idx))
             {
                 panic!(
                     "Command {}: Argument '{}' has the same index as '{}' \
@@ -242,13 +242,13 @@ pub(crate) fn assert_app(cmd: &Command) {
 
         if arg.is_last_set() {
             assert!(
-                arg.long.is_none(),
+                arg.get_long().is_none(),
                 "Command {}: Flags or Options cannot have last(true) set. '{}' has both a long and last(true) set.",
                     cmd.get_name(),
                 arg.get_id()
             );
             assert!(
-                arg.short.is_none(),
+                arg.get_short().is_none(),
                 "Command {}: Flags or Options cannot have last(true) set. '{}' has both a short and last(true) set.",
                     cmd.get_name(),
                 arg.get_id()
@@ -358,10 +358,12 @@ pub(crate) fn assert_app(cmd: &Command) {
 }
 
 fn duplicate_tip(cmd: &Command, first: &Arg, second: &Arg) -> &'static str {
-    if !cmd.is_disable_help_flag_set() && (first.id == Id::HELP || second.id == Id::HELP) {
+    if !cmd.is_disable_help_flag_set()
+        && (first.get_id() == Id::HELP || second.get_id() == Id::HELP)
+    {
         " (call `cmd.disable_help_flag(true)` to remove the auto-generated `--help`)"
     } else if !cmd.is_disable_version_flag_set()
-        && (first.id == Id::VERSION || second.id == Id::VERSION)
+        && (first.get_id() == Id::VERSION || second.get_id() == Id::VERSION)
     {
         " (call `cmd.disable_version_flag(true)` to remove the auto-generated `--version`)"
     } else {
@@ -546,7 +548,7 @@ fn _verify_positionals(cmd: &Command) -> bool {
     }
 
     // Next we verify that only the highest index has takes multiple arguments (if any)
-    let only_highest = |a: &Arg| a.is_multiple() && (a.index.unwrap_or(0) != highest_idx);
+    let only_highest = |a: &Arg| a.is_multiple() && (a.get_index().unwrap_or(0) != highest_idx);
     if cmd.get_positionals().any(only_highest) {
         // First we make sure if there is a positional that allows multiple values
         // the one before it (second to last) has one of these:
@@ -614,7 +616,7 @@ fn _verify_positionals(cmd: &Command) -> bool {
                          index than a required positional argument by two or more: {:?} \
                          index {:?}",
                     p.get_id(),
-                    p.index
+                    p.get_index()
                 );
             } else if p.is_required_set() && !p.is_last_set() {
                 // Args that .last(true) don't count since they can be required and have
@@ -643,7 +645,7 @@ fn _verify_positionals(cmd: &Command) -> bool {
                     "Found non-required positional argument with a lower \
                          index than a required positional argument: {:?} index {:?}",
                     p.get_id(),
-                    p.index
+                    p.get_index()
                 );
             } else if p.is_required_set() && !p.is_last_set() {
                 // Args that .last(true) don't count since they can be required and have
@@ -682,7 +684,7 @@ fn assert_arg(arg: &Arg) {
     // Self conflict
     // TODO: this check should be recursive
     assert!(
-        !arg.blacklist.iter().any(|x| *x == arg.id),
+        !arg.blacklist.iter().any(|x| x == arg.get_id()),
         "Argument '{}' cannot conflict with itself",
         arg.get_id(),
     );
@@ -731,9 +733,9 @@ fn assert_arg(arg: &Arg) {
             arg.is_takes_value_set(),
             "Argument '{}` is positional, it must take a value{}",
             arg.get_id(),
-            if arg.id == Id::HELP {
+            if arg.get_id() == Id::HELP {
                 " (`mut_arg` no longer works with implicit `--help`)"
-            } else if arg.id == Id::VERSION {
+            } else if arg.get_id() == Id::VERSION {
                 " (`mut_arg` no longer works with implicit `--version`)"
             } else {
                 ""

src/output/help_template.rs

@@ -494,7 +494,7 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> {
     fn long(&mut self, arg: &Arg) {
         debug!("HelpTemplate::long");
         if let Some(long) = arg.get_long() {
-            if arg.short.is_some() {
+            if arg.get_short().is_some() {
                 self.none(", ");
             }
             self.literal(format!("--{}", long));
@@ -516,7 +516,7 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> {
             let self_len = display_width(&arg.to_string());
             // Since we're writing spaces from the tab point we first need to know if we
             // had a long and short, or just short
-            let padding = if arg.long.is_some() {
+            let padding = if arg.get_long().is_some() {
                 // Only account 4 after the val
                 TAB_WIDTH
             } else {
@@ -969,7 +969,7 @@ fn option_sort_key(arg: &Arg) -> (usize, String) {
         x.to_string()
     } else {
         let mut s = '{'.to_string();
-        s.push_str(arg.id.as_str());
+        s.push_str(arg.get_id().as_str());
         s
     };
     (arg.get_display_order(), key)
@@ -1017,7 +1017,7 @@ fn replace_newline_var(styled: &mut StyledStr) {
 }
 
 fn longest_filter(arg: &Arg) -> bool {
-    arg.is_takes_value_set() || arg.long.is_some() || arg.short.is_none()
+    arg.is_takes_value_set() || arg.get_long().is_some() || arg.get_short().is_none()
 }
 
 #[cfg(test)]

src/output/usage.rs

@@ -170,7 +170,7 @@ impl<'cmd> Usage<'cmd> {
                 debug!("Usage::needs_options_tag:iter Option is required");
                 continue;
             }
-            for grp_s in self.cmd.groups_for_arg(&f.id) {
+            for grp_s in self.cmd.groups_for_arg(f.get_id()) {
                 debug!("Usage::needs_options_tag:iter:iter: grp_s={:?}", grp_s);
                 if self.cmd.get_groups().any(|g| g.id == grp_s && g.required) {
                     debug!("Usage::needs_options_tag:iter:iter: Group is required");

src/parser/arg_matcher.rs

@@ -25,8 +25,8 @@ impl ArgMatcher {
             matches: ArgMatches {
                 #[cfg(debug_assertions)]
                 valid_args: {
-                    let args = _cmd.get_arguments().map(|a| a.id.clone());
-                    let groups = _cmd.get_groups().map(|g| g.id.clone());
+                    let args = _cmd.get_arguments().map(|a| a.get_id().clone());
+                    let groups = _cmd.get_groups().map(|g| g.get_id().clone());
                     args.chain(groups).collect()
                 },
                 #[cfg(debug_assertions)]
@@ -130,7 +130,7 @@ impl ArgMatcher {
     }
 
     pub(crate) fn start_custom_arg(&mut self, arg: &Arg, source: ValueSource) {
-        let id = arg.id.clone();
+        let id = arg.get_id().clone();
         debug!(
             "ArgMatcher::start_custom_arg: id={:?}, source={:?}",
             id, source
@@ -153,7 +153,7 @@ impl ArgMatcher {
     }
 
     pub(crate) fn start_occurrence_of_arg(&mut self, arg: &Arg) {
-        let id = arg.id.clone();
+        let id = arg.get_id().clone();
         debug!("ArgMatcher::start_occurrence_of_arg: id={:?}", id);
         let ma = self.entry(id).or_insert(MatchedArg::new_arg(arg));
         debug_assert_eq!(ma.type_id(), Some(arg.get_value_parser().type_id()));
@@ -199,7 +199,7 @@ impl ArgMatcher {
         let num_pending = self
             .pending
             .as_ref()
-            .and_then(|p| (p.id == o.id).then(|| p.raw_vals.len()))
+            .and_then(|p| (p.id == *o.get_id()).then(|| p.raw_vals.len()))
             .unwrap_or(0);
         debug!(
             "ArgMatcher::needs_more_vals: o={}, pending={}",