From 217b111bd7338d47e42087855145b2951479325d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 29 Jul 2020 09:04:44 +1000 Subject: [PATCH 1/5] refactor: Replace `it.find(...).is_none()` with `!it.any(...)`. --- clap_generate/src/generators/mod.rs | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/clap_generate/src/generators/mod.rs b/clap_generate/src/generators/mod.rs index ef1e080ba60..2dfce022b77 100644 --- a/clap_generate/src/generators/mod.rs +++ b/clap_generate/src/generators/mod.rs @@ -140,19 +140,12 @@ pub trait Generator { .flatten() .collect(); - if shorts_and_visible_aliases - .iter() - .find(|x| **x == 'h') - .is_none() - { + if !shorts_and_visible_aliases.iter().any(|&x| x == 'h') { shorts_and_visible_aliases.push('h'); } if !p.is_set(AppSettings::DisableVersion) - && shorts_and_visible_aliases - .iter() - .find(|x| **x == 'V') - .is_none() + && !shorts_and_visible_aliases.iter().any(|&x| x == 'V') { shorts_and_visible_aliases.push('V'); } @@ -176,13 +169,11 @@ pub trait Generator { }) .collect(); - if longs.iter().find(|x| **x == "help").is_none() { + if !longs.iter().any(|x| *x == "help") { longs.push(String::from("help")); } - if !p.is_set(AppSettings::DisableVersion) - && longs.iter().find(|x| **x == "version").is_none() - { + if !p.is_set(AppSettings::DisableVersion) && !longs.iter().any(|x| *x == "version") { longs.push(String::from("version")); } @@ -196,7 +187,7 @@ pub trait Generator { let mut flags: Vec<_> = p.get_flags_no_heading().cloned().collect(); - if flags.iter().find(|x| x.get_name() == "help").is_none() { + if !flags.iter().any(|x| x.get_name() == "help") { flags.push( Arg::new("help") .short('h') @@ -206,7 +197,7 @@ pub trait Generator { } if !p.is_set(AppSettings::DisableVersion) - && flags.iter().find(|x| x.get_name() == "version").is_none() + && !flags.iter().any(|x| x.get_name() == "version") { flags.push( Arg::new("version") From d8c775eb26027618c24245af2ba8291a44ccabee Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 29 Jul 2020 09:37:47 +1000 Subject: [PATCH 2/5] refactor: Replace `it.filter(...).find(...)` with `it.find(...)`. --- src/parse/parser.rs | 3 +-- src/parse/validator.rs | 9 +++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 3dcd75cbafe..16cc70561f1 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -626,8 +626,7 @@ impl<'help, 'app> Parser<'help, 'app> { .args .args .iter() - .filter(|a| a.is_positional()) - .find(|p| p.index == Some(pos_counter as u64)) + .find(|a| a.is_positional() && a.index == Some(pos_counter as u64)) { if p.is_set(ArgSettings::Last) && !self.is_set(AS::TrailingValues) { return Err(ClapError::unknown_argument( diff --git a/src/parse/validator.rs b/src/parse/validator.rs index c8b86c02d54..33bc32840e6 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -213,8 +213,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> { .app .groups .iter() - .filter(|g| !g.multiple) - .find(|g| &g.id == name) + .find(|g| !g.multiple && &g.id == name) { let conf_with_self = self .p @@ -311,8 +310,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> { .app .groups .iter() - .filter(|g| !g.multiple) - .find(|g| g.id == grp) + .find(|g| !g.multiple && g.id == grp) { // for g_arg in self.p.app.unroll_args_in_group(&g.name) { // if &g_arg != name { @@ -326,8 +324,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> { .app .groups .iter() - .filter(|g| !g.multiple) - .find(|grp| grp.id == *name) + .find(|g| !g.multiple && g.id == *name) { debug!("Validator::gather_conflicts:iter:{:?}:group", name); self.c.insert(g.id.clone()); From be535e28cf7da89a5c92f5f54d099dba4602dfde Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 29 Jul 2020 09:42:20 +1000 Subject: [PATCH 3/5] refactor: Avoid unnecessary uses of `enumerate()`. --- src/util/argstr.rs | 10 +--------- src/util/graph.rs | 7 +------ 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/util/argstr.rs b/src/util/argstr.rs index c12f141bccf..e98c3c39379 100644 --- a/src/util/argstr.rs +++ b/src/util/argstr.rs @@ -76,15 +76,7 @@ impl<'a> ArgStr<'a> { pub(crate) fn trim_start_n_matches(&self, n: usize, ch: u8) -> ArgStr { assert!(ch.is_ascii()); - let i = self - .0 - .iter() - .take(n) - .take_while(|c| **c == ch) - .enumerate() - .last() - .map(|(i, _)| i + 1) - .unwrap_or(0); + let i = self.0.iter().take(n).take_while(|c| **c == ch).count(); self.split_at_unchecked(i).1 } diff --git a/src/util/graph.rs b/src/util/graph.rs index 183b2d9d378..c01563324d8 100644 --- a/src/util/graph.rs +++ b/src/util/graph.rs @@ -30,12 +30,7 @@ where self.0.push(Child::new(req)); idx } else { - self.0 - .iter() - .enumerate() - .find(|(_, e)| e.id == req) - .map(|(i, _)| i) - .unwrap() + self.0.iter().position(|e| e.id == req).unwrap() } } From 7fb397d905b7151d69407d3a70b490aa4e541dbc Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 29 Jul 2020 09:48:29 +1000 Subject: [PATCH 4/5] refactor: Simplify some `it.any(...)` calls. --- src/build/app/mod.rs | 3 +-- src/parse/matches/matched_arg.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 311dbece455..8681d3a99cd 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -2549,8 +2549,7 @@ impl<'help> App<'help> { pub(crate) fn has_visible_subcommands(&self) -> bool { self.subcommands .iter() - .filter(|sc| sc.name != "help") - .any(|sc| !sc.is_set(AppSettings::Hidden)) + .any(|sc| sc.name != "help" && !sc.is_set(AppSettings::Hidden)) } /// Check if this subcommand can be referred to as `name`. In other words, diff --git a/src/parse/matches/matched_arg.rs b/src/parse/matches/matched_arg.rs index 67a3dd78510..e9286bed475 100644 --- a/src/parse/matches/matched_arg.rs +++ b/src/parse/matches/matched_arg.rs @@ -35,7 +35,6 @@ impl MatchedArg { pub(crate) fn contains_val(&self, val: &str) -> bool { self.vals .iter() - .map(OsString::as_os_str) - .any(|v| v == OsStr::new(val)) + .any(|v| OsString::as_os_str(v) == OsStr::new(val)) } } From ed46e8962c2cb71b0ea2b5921b1b0186fe369471 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 29 Jul 2020 10:03:22 +1000 Subject: [PATCH 5/5] refactor: Combine two large and very similar expressions. --- src/output/usage.rs | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/src/output/usage.rs b/src/output/usage.rs index 6a6b0e52d4a..137c0e54cf1 100644 --- a/src/output/usage.rs +++ b/src/output/usage.rs @@ -391,28 +391,17 @@ impl<'help, 'app, 'parser> Usage<'help, 'app, 'parser> { .flat_map(|g| self.p.app.unroll_args_in_group(&g.id)) .collect::>(); - let pmap = if let Some(m) = matcher { - unrolled_reqs - .iter() - .chain(incls.iter()) - .filter(|a| self.p.app.get_positionals().any(|p| &&p.id == a)) - .filter(|&pos| !m.contains(pos)) - .filter_map(|pos| self.p.app.find(pos)) - .filter(|&pos| incl_last || !pos.is_set(ArgSettings::Last)) - .filter(|pos| !args_in_groups.contains(&pos.id)) - .map(|pos| (pos.index.unwrap(), pos)) - .collect::>() // sort by index - } else { - unrolled_reqs - .iter() - .chain(incls.iter()) - .filter(|a| self.p.app.get_positionals().any(|p| &&p.id == a)) - .filter_map(|pos| self.p.app.find(pos)) - .filter(|&pos| incl_last || !pos.is_set(ArgSettings::Last)) - .filter(|pos| !args_in_groups.contains(&pos.id)) - .map(|pos| (pos.index.unwrap(), pos)) - .collect::>() // sort by index - }; + let pmap = unrolled_reqs + .iter() + .chain(incls.iter()) + .filter(|a| self.p.app.get_positionals().any(|p| &&p.id == a)) + .filter(|&pos| matcher.map_or(true, |m| !m.contains(pos))) + .filter_map(|pos| self.p.app.find(pos)) + .filter(|&pos| incl_last || !pos.is_set(ArgSettings::Last)) + .filter(|pos| !args_in_groups.contains(&pos.id)) + .map(|pos| (pos.index.unwrap(), pos)) + .collect::>(); // sort by index + for p in pmap.values() { debug!("Usage::get_required_usage_from:iter:{:?}", p.id); if args_in_groups.is_empty() || !args_in_groups.contains(&p.id) {