diff --git a/examples/derive_ref/custom-bool.md b/examples/derive_ref/custom-bool.md index f8a5bd86b20..3586af567eb 100644 --- a/examples/derive_ref/custom-bool.md +++ b/examples/derive_ref/custom-bool.md @@ -26,7 +26,7 @@ error: The following required arguments were not provided: Usage: - custom-bool[EXE] [OPTIONS] --foo + custom-bool[EXE] --foo For more information try --help diff --git a/examples/tutorial_builder/04_03_relations.md b/examples/tutorial_builder/04_03_relations.md index 0ff757684ec..aad5c7cb192 100644 --- a/examples/tutorial_builder/04_03_relations.md +++ b/examples/tutorial_builder/04_03_relations.md @@ -25,7 +25,7 @@ error: The following required arguments were not provided: <--set-ver |--major|--minor|--patch> Usage: - 04_03_relations[EXE] [OPTIONS] <--set-ver |--major|--minor|--patch> [INPUT_FILE] + 04_03_relations[EXE] <--set-ver |--major|--minor|--patch> For more information try --help diff --git a/examples/tutorial_derive/04_03_relations.md b/examples/tutorial_derive/04_03_relations.md index dba580b5ebd..b4a69aacff4 100644 --- a/examples/tutorial_derive/04_03_relations.md +++ b/examples/tutorial_derive/04_03_relations.md @@ -25,7 +25,7 @@ error: The following required arguments were not provided: <--set-ver |--major|--minor|--patch> Usage: - 04_03_relations_derive[EXE] [OPTIONS] <--set-ver |--major|--minor|--patch> [INPUT_FILE] + 04_03_relations_derive[EXE] <--set-ver |--major|--minor|--patch> For more information try --help diff --git a/src/output/usage.rs b/src/output/usage.rs index a509c8fe14c..2fe2f36a5ec 100644 --- a/src/output/usage.rs +++ b/src/output/usage.rs @@ -365,7 +365,7 @@ impl<'cmd> Usage<'cmd> { &required_owned }; - let mut unrolled_reqs = FlatSet::new(); + let mut unrolled_reqs = Vec::new(); for a in required.iter() { let is_relevant = |(val, req_arg): &(ArgPredicate, Id)| -> Option { let required = match val { @@ -384,11 +384,11 @@ impl<'cmd> Usage<'cmd> { for aa in self.cmd.unroll_arg_requires(is_relevant, a) { // if we don't check for duplicates here this causes duplicate error messages // see https://github.com/clap-rs/clap/issues/2770 - unrolled_reqs.insert(aa); + unrolled_reqs.push(aa); } // always include the required arg itself. it will not be enumerated // by unroll_requirements_for_arg. - unrolled_reqs.insert(a.clone()); + unrolled_reqs.push(a.clone()); } debug!( "Usage::get_required_usage_from: unrolled_reqs={:?}", @@ -396,30 +396,9 @@ impl<'cmd> Usage<'cmd> { ); let mut required_groups_members = FlatSet::new(); - let mut required_opts = FlatSet::new(); let mut required_groups = FlatSet::new(); - let mut required_positionals = FlatSet::new(); for req in unrolled_reqs.iter().chain(incls.iter()) { - if let Some(arg) = self.cmd.find(req) { - let is_present = matcher - .map(|m| m.check_explicit(req, &ArgPredicate::IsPresent)) - .unwrap_or(false); - debug!( - "Usage::get_required_usage_from:iter:{:?} arg is_present={}", - req, is_present - ); - if !is_present { - if arg.is_positional() { - if incl_last || !arg.is_last_set() { - required_positionals - .insert((arg.index.unwrap(), arg.stylized(Some(true)))); - } - } else { - required_opts.insert(arg.stylized(Some(true))); - } - } - } else { - debug_assert!(self.cmd.find_group(req).is_some()); + if self.cmd.find_group(req).is_some() { let group_members = self.cmd.unroll_args_in_group(req); let is_present = matcher .map(|m| { @@ -432,31 +411,57 @@ impl<'cmd> Usage<'cmd> { "Usage::get_required_usage_from:iter:{:?} group is_present={}", req, is_present ); - if !is_present { - let elem = self.cmd.format_group(req); - required_groups.insert(elem); - required_groups_members.extend( - group_members - .iter() - .flat_map(|id| self.cmd.find(id)) - .map(|arg| arg.stylized(Some(true))), - ); + if is_present { + continue; } + + let elem = self.cmd.format_group(req); + required_groups.insert(elem); + required_groups_members.extend(group_members); + } else { + debug_assert!(self.cmd.find(req).is_some()); } } - let mut ret_val = Vec::new(); + let mut required_opts = FlatSet::new(); + let mut required_positionals = FlatSet::new(); + for req in unrolled_reqs.iter().chain(incls.iter()) { + if let Some(arg) = self.cmd.find(req) { + if required_groups_members.contains(arg.get_id()) { + continue; + } - required_opts.retain(|arg| !required_groups_members.contains(arg)); - ret_val.extend(required_opts); + let is_present = matcher + .map(|m| m.check_explicit(req, &ArgPredicate::IsPresent)) + .unwrap_or(false); + debug!( + "Usage::get_required_usage_from:iter:{:?} arg is_present={}", + req, is_present + ); + if is_present { + continue; + } - ret_val.extend(required_groups); + let stylized = arg.stylized(Some(true)); + if arg.is_positional() { + if !arg.is_last_set() || incl_last { + let index = arg.index.unwrap(); + required_positionals.insert((index, stylized)); + } + } else { + required_opts.insert(stylized); + } + } else { + debug_assert!(self.cmd.find_group(req).is_some()); + } + } + let mut ret_val = Vec::new(); + ret_val.extend(required_opts); + ret_val.extend(required_groups); required_positionals.sort_by_key(|(ind, _)| *ind); // sort by index for (_, p) in required_positionals { - if !required_groups_members.contains(&p) { - ret_val.push(p); - } + ret_val.push(p); } debug!("Usage::get_required_usage_from: ret_val={:?}", ret_val); diff --git a/src/parser/validator.rs b/src/parser/validator.rs index 1a8bbdce7f8..4703cb504d5 100644 --- a/src/parser/validator.rs +++ b/src/parser/validator.rs @@ -240,6 +240,9 @@ impl<'cmd> Validator<'cmd> { debug!("Validator::validate_required: required={:?}", self.required); self.gather_requires(matcher); + let mut missing_required = Vec::new(); + let mut highest_index = 0; + let is_exclusive_present = matcher .arg_ids() .filter(|arg_id| matcher.check_explicit(arg_id, &ArgPredicate::IsPresent)) @@ -263,7 +266,14 @@ impl<'cmd> Validator<'cmd> { if let Some(arg) = self.cmd.find(arg_or_group) { debug!("Validator::validate_required:iter: This is an arg"); if !is_exclusive_present && !self.is_missing_required_ok(arg, matcher, conflicts) { - return self.missing_required_error(matcher, vec![]); + debug!( + "Validator::validate_required:iter: Missing {:?}", + arg.get_id() + ); + missing_required.push(arg.get_id().clone()); + if !arg.is_last_set() { + highest_index = highest_index.max(arg.get_index().unwrap_or(0)); + } } } else if let Some(group) = self.cmd.find_group(arg_or_group) { debug!("Validator::validate_required:iter: This is a group"); @@ -273,33 +283,82 @@ impl<'cmd> Validator<'cmd> { .iter() .any(|a| matcher.check_explicit(a, &ArgPredicate::IsPresent)) { - return self.missing_required_error(matcher, vec![]); + debug!( + "Validator::validate_required:iter: Missing {:?}", + group.get_id() + ); + missing_required.push(group.get_id().clone()); } } } // Validate the conditionally required args - for a in self.cmd.get_arguments() { + for a in self + .cmd + .get_arguments() + .filter(|a| !matcher.check_explicit(a.get_id(), &ArgPredicate::IsPresent)) + { + let mut required = false; + for (other, val) in &a.r_ifs { - if matcher.check_explicit(other, &ArgPredicate::Equals(val.into())) - && !matcher.check_explicit(&a.id, &ArgPredicate::IsPresent) - { - return self.missing_required_error(matcher, vec![a.id.clone()]); + if matcher.check_explicit(other, &ArgPredicate::Equals(val.into())) { + debug!( + "Validator::validate_required:iter: Missing {:?}", + a.get_id() + ); + required = true; } } let match_all = a.r_ifs_all.iter().all(|(other, val)| { matcher.check_explicit(other, &ArgPredicate::Equals(val.into())) }); - if match_all - && !a.r_ifs_all.is_empty() - && !matcher.check_explicit(&a.id, &ArgPredicate::IsPresent) + if match_all && !a.r_ifs_all.is_empty() { + debug!( + "Validator::validate_required:iter: Missing {:?}", + a.get_id() + ); + required = true; + } + + if (!a.r_unless.is_empty() || !a.r_unless_all.is_empty()) + && self.fails_arg_required_unless(a, matcher) + { + debug!( + "Validator::validate_required:iter: Missing {:?}", + a.get_id() + ); + required = true; + } + + if required { + missing_required.push(a.get_id().clone()); + if !a.is_last_set() { + highest_index = highest_index.max(a.get_index().unwrap_or(0)); + } + } + } + + // For display purposes, include all of the preceding positional arguments + if !self.cmd.is_allow_missing_positional_set() { + for pos in self + .cmd + .get_positionals() + .filter(|a| !matcher.check_explicit(a.get_id(), &ArgPredicate::IsPresent)) { - return self.missing_required_error(matcher, vec![a.id.clone()]); + if pos.get_index() < Some(highest_index) { + debug!( + "Validator::validate_required:iter: Missing {:?}", + pos.get_id() + ); + missing_required.push(pos.get_id().clone()); + } } } - self.validate_required_unless(matcher)?; + if !missing_required.is_empty() { + self.missing_required_error(matcher, missing_required)?; + } Ok(()) } @@ -315,25 +374,6 @@ impl<'cmd> Validator<'cmd> { !conflicts.is_empty() } - fn validate_required_unless(&self, matcher: &ArgMatcher) -> ClapResult<()> { - debug!("Validator::validate_required_unless"); - let failed_args: Vec<_> = self - .cmd - .get_arguments() - .filter(|&a| { - (!a.r_unless.is_empty() || !a.r_unless_all.is_empty()) - && !matcher.check_explicit(&a.id, &ArgPredicate::IsPresent) - && self.fails_arg_required_unless(a, matcher) - }) - .map(|a| a.id.clone()) - .collect(); - if failed_args.is_empty() { - Ok(()) - } else { - self.missing_required_error(matcher, failed_args) - } - } - // Failing a required unless means, the arg's "unless" wasn't present, and neither were they fn fails_arg_required_unless(&self, a: &Arg, matcher: &ArgMatcher) -> bool { debug!("Validator::fails_arg_required_unless: a={:?}", a.get_id()); diff --git a/src/util/flat_map.rs b/src/util/flat_map.rs index 181950829cf..07e418bf28b 100644 --- a/src/util/flat_map.rs +++ b/src/util/flat_map.rs @@ -1,3 +1,5 @@ +#![allow(dead_code)] + use std::borrow::Borrow; /// Flat (Vec) backed map diff --git a/src/util/flat_set.rs b/src/util/flat_set.rs index db4381710b8..3e0b23dae35 100644 --- a/src/util/flat_set.rs +++ b/src/util/flat_set.rs @@ -1,3 +1,5 @@ +#![allow(dead_code)] + use std::borrow::Borrow; /// Flat (Vec) backed set diff --git a/tests/builder/default_vals.rs b/tests/builder/default_vals.rs index 6c32e614e94..3f04b6b9657 100644 --- a/tests/builder/default_vals.rs +++ b/tests/builder/default_vals.rs @@ -732,7 +732,7 @@ fn default_vals_donnot_show_in_smart_usage() { Usage: - bug [OPTIONS] + bug For more information try --help ", diff --git a/tests/builder/require.rs b/tests/builder/require.rs index 6aeca5d9b1f..fd9304597cb 100644 --- a/tests/builder/require.rs +++ b/tests/builder/require.rs @@ -33,10 +33,11 @@ For more information try --help static MISSING_REQ: &str = "error: The following required arguments were not provided: --long-option-2 + Usage: - clap-test --long-option-2 -F + clap-test --long-option-2 -F For more information try --help "; @@ -141,7 +142,7 @@ static POSITIONAL_REQ: &str = "error: The following required arguments were not Usage: - clap-test [ARGS] + clap-test For more information try --help "; @@ -160,7 +161,7 @@ static POSITIONAL_REQ_IF_NO_VAL: &str = "error: The following required arguments Usage: - clap-test [ARGS] + clap-test For more information try --help ";