Mirrors/clap
2
0
Fork 0
mirror of https://github.com/clap-rs/clap synced 2024-12-12 13:52:34 +00:00
Code Issues Projects Releases Packages Wiki Activity

refactor(validator): Decouple parser (#3645)

Browse source 
* fix(validate): Consistent conflict behavior

We had two different implementations of conflicts
- Validating conflicts
- Allowing conflicts to override `required`
  - Missing members of a group conflicting with each other
  - Missing symmetric conflicts (A conflicts with B so B conflicts with
    A)

This consolidates their implementations which should fix overriding of
`required`.

* fix(validate): Overrides always ignore required

Before, if two arguments were required *and* overrode each other, then
`cmd --opt=1 --other=2` succeded but `cmd --other=2` failed despite
ignoring `--opt=1`.  Requiring `--opt=1` to be present but unavailable
doesn't help anyone and makes the behavior less predictable.

Now both commands will have the same behavior.

* refactor(parser): Pull out long-help determination

This isn't a parser policy but command-level policy.

* refactor(help): Make bool's meaning clearer

* refactor(help): Consolidate help errors

* refactor(help): Move help writing down a layer

* refactor(parser): Parser is solely responsible for populating ArgMatches

* refactor(validator): Decouple parser
This commit is contained in:
Ed Page 2022-04-21 18:14:17 -05:00 committed by GitHub
commit 8505c47e2e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 251 additions and 193 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/build/arg.rs
View file

@ -4339,6 +4339,8 @@ impl<'help> Arg<'help> {
/// **NOTE:** When an argument is overridden it is essentially as if it never was used, any
/// conflicts, requirements, etc. are evaluated **after** all "overrides" have been removed
///
/// **NOTE:** Overriding an argument implies they [conflict][Arg::conflicts_with`].
///
/// **WARNING:** Positional arguments and options which accept
/// [`Arg::multiple_occurrences`] cannot override themselves (or we
/// would never be able to advance to the next positional). If a positional
@ -4454,6 +4456,8 @@ impl<'help> Arg<'help> {
/// **NOTE:** When an argument is overridden it is essentially as if it never was used, any
/// conflicts, requirements, etc. are evaluated **after** all "overrides" have been removed
///
/// **NOTE:** Overriding an argument implies they [conflict][Arg::conflicts_with_all`].
///
/// # Examples
///
/// ```rust

57
src/build/command.rs
View file

@ -20,10 +20,12 @@ use crate::build::{arg::ArgProvider, Arg, ArgGroup, ArgPredicate};
use crate::error::ErrorKind;
use crate::error::Result as ClapResult;
use crate::mkeymap::MKeyMap;
use crate::output::fmt::Stream;
use crate::output::{fmt::Colorizer, Help, HelpWriter, Usage};
use crate::parse::{ArgMatcher, ArgMatches, Parser};
use crate::util::ChildGraph;
use crate::util::{color::ColorChoice, Id, Key};
use crate::PossibleValue;
use crate::{Error, INTERNAL_ERROR_MSG};
#[cfg(debug_assertions)]
@ -696,7 +698,7 @@ impl<'help> App<'help> {
self._build();
let color = self.get_color();
let mut c = Colorizer::new(false, color);
let mut c = Colorizer::new(Stream::Stdout, color);
let usage = Usage::new(self);
Help::new(HelpWriter::Buffer(&mut c), self, &usage, false).write_help()?;
c.print()
@ -721,7 +723,7 @@ impl<'help> App<'help> {
self._build();
let color = self.get_color();
let mut c = Colorizer::new(false, color);
let mut c = Colorizer::new(Stream::Stdout, color);
let usage = Usage::new(self);
Help::new(HelpWriter::Buffer(&mut c), self, &usage, true).write_help()?;
c.print()
@ -4675,6 +4677,57 @@ impl<'help> App<'help> {
pub(crate) fn get_display_order(&self) -> usize {
self.disp_ord.unwrap_or(999)
}
pub(crate) fn write_help_err(
&self,
mut use_long: bool,
stream: Stream,
) -> ClapResult<Colorizer> {
debug!(
"Parser::write_help_err: use_long={:?}, stream={:?}",
use_long && self.use_long_help(),
stream
);
use_long = use_long && self.use_long_help();
let usage = Usage::new(self);
let mut c = Colorizer::new(stream, self.color_help());
Help::new(HelpWriter::Buffer(&mut c), self, &usage, use_long).write_help()?;
Ok(c)
}
pub(crate) fn use_long_help(&self) -> bool {
debug!("Command::use_long_help");
// In this case, both must be checked. This allows the retention of
// original formatting, but also ensures that the actual -h or --help
// 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.is_hide_long_help_set()
|| v.is_hide_short_help_set()
|| cfg!(feature = "unstable-v4")
&& v.possible_vals.iter().any(PossibleValue::should_show_help)
};
// Subcommands aren't checked because we prefer short help for them, deferring to
// `cmd subcmd --help` for more.
self.get_long_about().is_some()
|| self.get_before_long_help().is_some()
|| self.get_after_long_help().is_some()
|| self.get_arguments().any(should_long)
}
// Should we color the help?
pub(crate) fn color_help(&self) -> ColorChoice {
#[cfg(feature = "color")]
if self.is_disable_colored_help_set() {
return ColorChoice::Never;
}
self.get_color()
}
}
impl<'help> Default for App<'help> {

19
src/error/mod.rs
View file

@ -15,6 +15,7 @@ use std::{
use crate::{
build::Arg,
output::fmt::Colorizer,
output::fmt::Stream,
parse::features::suggestions,
util::{color::ColorChoice, safe_exit, SUCCESS_CODE, USAGE_CODE},
AppSettings, Command,
@ -97,10 +98,14 @@ impl Error {
/// Should the message be written to `stdout` or not?
#[inline]
pub fn use_stderr(&self) -> bool {
!matches!(
self.kind(),
ErrorKind::DisplayHelp | ErrorKind::DisplayVersion
)
self.stream() == Stream::Stderr
}
pub(crate) fn stream(&self) -> Stream {
match self.kind() {
ErrorKind::DisplayHelp | ErrorKind::DisplayVersion => Stream::Stdout,
_ => Stream::Stderr,
}
}
/// Prints the error and exits.
@ -608,7 +613,7 @@ impl Error {
if let Some(message) = self.inner.message.as_ref() {
message.formatted()
} else {
let mut c = Colorizer::new(self.use_stderr(), self.inner.color_when);
let mut c = Colorizer::new(self.stream(), self.inner.color_when);
start_error(&mut c);
@ -1090,7 +1095,7 @@ impl Message {
fn format(&mut self, cmd: &Command, usage: String) {
match self {
Message::Raw(s) => {
let mut c = Colorizer::new(true, cmd.get_color());
let mut c = Colorizer::new(Stream::Stderr, cmd.get_color());
let mut message = String::new();
std::mem::swap(s, &mut message);
@ -1107,7 +1112,7 @@ impl Message {
fn formatted(&self) -> Cow<Colorizer> {
match self {
Message::Raw(s) => {
let mut c = Colorizer::new(true, ColorChoice::Never);
let mut c = Colorizer::new(Stream::Stderr, ColorChoice::Never);
start_error(&mut c);
c.none(s);
Cow::Owned(c)

2
src/macros.rs
View file

@ -933,7 +933,7 @@ macro_rules! debug {
($($arg:tt)*) => ({
let prefix = format!("[{:>w$}] \t", module_path!(), w = 28);
let body = format!($($arg)*);
let mut color = $crate::output::fmt::Colorizer::new(true, $crate::ColorChoice::Auto);
let mut color = $crate::output::fmt::Colorizer::new($crate::output::fmt::Stream::Stderr, $crate::ColorChoice::Auto);
color.hint(prefix);
color.hint(body);
color.none("\n");

49
src/output/fmt.rs
View file

@ -5,9 +5,15 @@ use std::{
io::{self, Write},
};
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub(crate) enum Stream {
Stdout,
Stderr,
}
#[derive(Clone, Debug)]
pub(crate) struct Colorizer {
use_stderr: bool,
stream: Stream,
#[allow(unused)]
color_when: ColorChoice,
pieces: Vec<(String, Style)>,
@ -15,9 +21,9 @@ pub(crate) struct Colorizer {
impl Colorizer {
#[inline(never)]
pub(crate) fn new(use_stderr: bool, color_when: ColorChoice) -> Self {
pub(crate) fn new(stream: Stream, color_when: ColorChoice) -> Self {
Colorizer {
use_stderr,
stream,
color_when,
pieces: vec![],
}
@ -58,14 +64,13 @@ impl Colorizer {
let color_when = match self.color_when {
ColorChoice::Always => DepColorChoice::Always,
ColorChoice::Auto if is_a_tty(self.use_stderr) => DepColorChoice::Auto,
ColorChoice::Auto if is_a_tty(self.stream) => DepColorChoice::Auto,
_ => DepColorChoice::Never,
};
let writer = if self.use_stderr {
BufferWriter::stderr(color_when)
} else {
BufferWriter::stdout(color_when)
let writer = match self.stream {
Stream::Stdout => BufferWriter::stderr(color_when),
Stream::Stderr => BufferWriter::stdout(color_when),
};
let mut buffer = writer.buffer();
@ -101,14 +106,17 @@ impl Colorizer {
pub(crate) fn print(&self) -> io::Result<()> {
// [e]println can't be used here because it panics
// if something went wrong. We don't want that.
if self.use_stderr {
let stderr = std::io::stderr();
let mut stderr = stderr.lock();
write!(stderr, "{}", self)
} else {
let stdout = std::io::stdout();
let mut stdout = stdout.lock();
write!(stdout, "{}", self)
match self.stream {
Stream::Stdout => {
let stdout = std::io::stdout();
let mut stdout = stdout.lock();
write!(stdout, "{}", self)
}
Stream::Stderr => {
let stderr = std::io::stderr();
let mut stderr = stderr.lock();
write!(stderr, "{}", self)
}
}
}
}
@ -140,11 +148,10 @@ impl Default for Style {
}
#[cfg(feature = "color")]
fn is_a_tty(stderr: bool) -> bool {
let stream = if stderr {
atty::Stream::Stderr
} else {
atty::Stream::Stdout
fn is_a_tty(stream: Stream) -> bool {
let stream = match stream {
Stream::Stdout => atty::Stream::Stdout,
Stream::Stderr => atty::Stream::Stderr,
};
atty::is(stream)

89
src/parse/parser.rs
View file

@ -1,6 +1,6 @@
// Std
use std::{
cell::{Cell, RefCell},
cell::Cell,
ffi::{OsStr, OsString},
};
@ -8,21 +8,21 @@ use std::{
use clap_lex::RawOsStr;
// Internal
use crate::build::AppSettings as AS;
use crate::build::{Arg, Command};
use crate::error::Error as ClapError;
use crate::error::Result as ClapResult;
use crate::mkeymap::KeyType;
use crate::output::{fmt::Colorizer, Help, HelpWriter, Usage};
use crate::output::fmt::Stream;
use crate::output::{fmt::Colorizer, Usage};
use crate::parse::features::suggestions;
use crate::parse::{ArgMatcher, SubCommand};
use crate::parse::{Validator, ValueSource};
use crate::util::{color::ColorChoice, Id};
use crate::{build::AppSettings as AS, PossibleValue};
use crate::util::Id;
use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8};
pub(crate) struct Parser<'help, 'cmd> {
pub(crate) cmd: &'cmd mut Command<'help>,
pub(crate) overridden: RefCell<Vec<Id>>,
seen: Vec<Id>,
cur_idx: Cell<usize>,
/// Index of the previous flag subcommand in a group of flags.
@ -37,23 +37,12 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
pub(crate) fn new(cmd: &'cmd mut Command<'help>) -> Self {
Parser {
cmd,
overridden: Default::default(),
seen: Vec::new(),
cur_idx: Cell::new(0),
flag_subcmd_at: None,
flag_subcmd_skip: 0,
}
}
// Should we color the help?
pub(crate) fn color_help(&self) -> ColorChoice {
#[cfg(feature = "color")]
if self.cmd.is_disable_colored_help_set() {
return ColorChoice::Never;
}
self.cmd.get_color()
}
}
// Parsing Methods
@ -259,7 +248,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
))
}
ParseResult::HelpFlag => {
return Err(self.help_err(true));
return Err(self.help_err(true, Stream::Stdout));
}
ParseResult::VersionFlag => {
return Err(self.version_err(true));
@ -342,7 +331,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
));
}
ParseResult::HelpFlag => {
return Err(self.help_err(false));
return Err(self.help_err(false, Stream::Stdout));
}
ParseResult::VersionFlag => {
return Err(self.version_err(false));
@ -459,7 +448,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
matches: sc_m.into_inner(),
});
return Validator::new(self).validate(parse_state, matcher, trailing_values);
#[cfg(feature = "env")]
self.add_env(matcher, trailing_values)?;
self.add_defaults(matcher, trailing_values);
return Validator::new(self.cmd).validate(parse_state, matcher);
} else {
// Start error processing
return Err(self.match_arg_error(&arg_os, valid_arg_found, trailing_values));
@ -476,7 +468,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
self.parse_subcommand(&sc_name, matcher, raw_args, args_cursor, keep_state)?;
}
Validator::new(self).validate(parse_state, matcher, trailing_values)
#[cfg(feature = "env")]
self.add_env(matcher, trailing_values)?;
self.add_defaults(matcher, trailing_values);
Validator::new(self.cmd).validate(parse_state, matcher)
}
fn match_arg_error(
@ -647,7 +642,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
let parser = Parser::new(&mut sc);
Err(parser.help_err(true))
Err(parser.help_err(true, Stream::Stdout))
}
fn is_new_arg(&self, next: &clap_lex::ParsedArg<'_>, current_positional: &Arg) -> bool {
@ -807,28 +802,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
None
}
fn use_long_help(&self) -> bool {
debug!("Parser::use_long_help");
// In this case, both must be checked. This allows the retention of
// original formatting, but also ensures that the actual -h or --help
// 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.is_hide_long_help_set()
|| v.is_hide_short_help_set()
|| cfg!(feature = "unstable-v4")
&& v.possible_vals.iter().any(PossibleValue::should_show_help)
};
// Subcommands aren't checked because we prefer short help for them, deferring to
// `cmd subcmd --help` for more.
self.cmd.get_long_about().is_some()
|| self.cmd.get_before_long_help().is_some()
|| self.cmd.get_after_long_help().is_some()
|| self.cmd.get_arguments().any(should_long)
}
fn parse_long_arg(
&mut self,
matcher: &mut ArgMatcher,
@ -1248,7 +1221,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
for override_id in &arg.overrides {
debug!("Parser::remove_overrides:iter:{:?}: removing", override_id);
matcher.remove(override_id);
self.overridden.borrow_mut().push(override_id.clone());
}
// Override anything that can override us
@ -1263,7 +1235,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
for overrider_id in transitive {
debug!("Parser::remove_overrides:iter:{:?}: removing", overrider_id);
matcher.remove(overrider_id);
self.overridden.borrow_mut().push(overrider_id.clone());
}
}
@ -1443,7 +1414,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
// Early return on `HelpFlag` or `VersionFlag`.
match self.check_for_help_and_version_str(&val) {
Some(ParseResult::HelpFlag) => {
return Err(self.help_err(true));
return Err(self.help_err(true, Stream::Stdout));
}
Some(ParseResult::VersionFlag) => {
return Err(self.version_err(true));
@ -1533,26 +1504,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
)
}
pub(crate) fn write_help_err(&self) -> ClapResult<Colorizer> {
let usage = Usage::new(self.cmd);
let mut c = Colorizer::new(true, self.color_help());
Help::new(HelpWriter::Buffer(&mut c), self.cmd, &usage, false).write_help()?;
Ok(c)
}
fn help_err(&self, mut use_long: bool) -> ClapError {
debug!(
"Parser::help_err: use_long={:?}",
use_long && self.use_long_help()
);
use_long = use_long && self.use_long_help();
let usage = Usage::new(self.cmd);
let mut c = Colorizer::new(false, self.color_help());
match Help::new(HelpWriter::Buffer(&mut c), self.cmd, &usage, use_long).write_help() {
Err(e) => e.into(),
_ => ClapError::display_help(self.cmd, c),
fn help_err(&self, use_long: bool, stream: Stream) -> ClapError {
match self.cmd.write_help_err(use_long, stream) {
Ok(c) => ClapError::display_help(self.cmd, c),
Err(e) => e,
}
}
@ -1560,7 +1515,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
debug!("Parser::version_err");
let msg = self.cmd._render_version(use_long);
let mut c = Colorizer::new(false, self.color_help());
let mut c = Colorizer::new(Stream::Stdout, self.cmd.color_help());
c.none(msg);
ClapError::display_version(self.cmd, c)
}

186
src/parse/validator.rs
View file

@ -1,41 +1,37 @@
// Internal
use crate::build::{AppSettings, Arg, ArgPredicate, Command, PossibleValue};
use crate::error::{Error, Result as ClapResult};
use crate::output::fmt::Stream;
use crate::output::Usage;
use crate::parse::{ArgMatcher, MatchedArg, ParseState, Parser};
use crate::parse::{ArgMatcher, MatchedArg, ParseState};
use crate::util::ChildGraph;
use crate::util::Id;
use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8};
pub(crate) struct Validator<'help, 'cmd, 'parser> {
p: &'parser mut Parser<'help, 'cmd>,
pub(crate) struct Validator<'help, 'cmd> {
cmd: &'cmd Command<'help>,
required: ChildGraph<Id>,
}
impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
pub(crate) fn new(p: &'parser mut Parser<'help, 'cmd>) -> Self {
let required = p.cmd.required_graph();
Validator { p, required }
impl<'help, 'cmd> Validator<'help, 'cmd> {
pub(crate) fn new(cmd: &'cmd Command<'help>) -> Self {
let required = cmd.required_graph();
Validator { cmd, required }
}
pub(crate) fn validate(
&mut self,
parse_state: ParseState,
matcher: &mut ArgMatcher,
trailing_values: bool,
) -> ClapResult<()> {
debug!("Validator::validate");
let mut conflicts = Conflicts::new();
let has_subcmd = matcher.subcommand_name().is_some();
#[cfg(feature = "env")]
self.p.add_env(matcher, trailing_values)?;
self.p.add_defaults(matcher, trailing_values);
if let ParseState::Opt(a) = parse_state {
debug!("Validator::validate: needs_val_of={:?}", a);
let o = &self.p.cmd[&a];
let o = &self.cmd[&a];
let should_err = if let Some(v) = matcher.args.get(&o.id) {
v.all_val_groups_empty() && !(o.min_vals.is_some() && o.min_vals.unwrap() == 0)
} else {
@ -43,53 +39,52 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
};
if should_err {
return Err(Error::empty_value(
self.p.cmd,
self.cmd,
&o.possible_vals
.iter()
.filter(|pv| !pv.is_hide_set())
.map(PossibleValue::get_name)
.collect::<Vec<_>>(),
o,
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
));
}
}
if !has_subcmd && self.p.cmd.is_arg_required_else_help_set() {
if !has_subcmd && self.cmd.is_arg_required_else_help_set() {
let num_user_values = matcher
.arg_names()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.count();
if num_user_values == 0 {
let message = self.p.write_help_err()?;
return Err(Error::display_help_error(self.p.cmd, message));
let message = self.cmd.write_help_err(false, Stream::Stderr)?;
return Err(Error::display_help_error(self.cmd, message));
}
}
#[allow(deprecated)]
if !has_subcmd && self.p.cmd.is_subcommand_required_set() {
if !has_subcmd && self.cmd.is_subcommand_required_set() {
let bn = self
.p
.cmd
.get_bin_name()
.unwrap_or_else(|| self.p.cmd.get_name());
.unwrap_or_else(|| self.cmd.get_name());
return Err(Error::missing_subcommand(
self.p.cmd,
self.cmd,
bn.to_string(),
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
));
} else if !has_subcmd && self.p.cmd.is_set(AppSettings::SubcommandRequiredElseHelp) {
} else if !has_subcmd && self.cmd.is_set(AppSettings::SubcommandRequiredElseHelp) {
debug!("Validator::new::get_matches_with: SubcommandRequiredElseHelp=true");
let message = self.p.write_help_err()?;
return Err(Error::display_help_error(self.p.cmd, message));
let message = self.cmd.write_help_err(false, Stream::Stderr)?;
return Err(Error::display_help_error(self.cmd, message));
}
self.validate_conflicts(matcher)?;
if !(self.p.cmd.is_subcommand_negates_reqs_set() && has_subcmd) {
self.validate_required(matcher)?;
self.validate_conflicts(matcher, &mut conflicts)?;
if !(self.cmd.is_subcommand_negates_reqs_set() && has_subcmd) {
self.validate_required(matcher, &mut conflicts)?;
}
self.validate_matched_args(matcher)?;
@ -110,8 +105,8 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
val
);
return Err(Error::invalid_utf8(
self.p.cmd,
Usage::new(self.p.cmd)
self.cmd,
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
));
@ -131,14 +126,14 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
.arg_names()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.filter(|&n| {
self.p.cmd.find(n).map_or(true, |a| {
self.cmd.find(n).map_or(true, |a| {
!(a.is_hide_set() || self.required.contains(&a.id))
})
})
.cloned()
.collect();
return Err(Error::invalid_value(
self.p.cmd,
self.cmd,
val_str.into_owned(),
&arg.possible_vals
.iter()
@ -146,7 +141,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
.map(PossibleValue::get_name)
.collect::<Vec<_>>(),
arg,
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&used),
));
@ -155,14 +150,14 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
if arg.is_forbid_empty_values_set() && val.is_empty() && matcher.contains(&arg.id) {
debug!("Validator::validate_arg_values: illegal empty val found");
return Err(Error::empty_value(
self.p.cmd,
self.cmd,
&arg.possible_vals
.iter()
.filter(|pv| !pv.is_hide_set())
.map(PossibleValue::get_name)
.collect::<Vec<_>>(),
arg,
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
));
@ -178,7 +173,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
val.to_string_lossy().into_owned(),
e,
)
.with_cmd(self.p.cmd));
.with_cmd(self.cmd));
} else {
debug!("good");
}
@ -193,7 +188,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
val.to_string_lossy().into(),
e,
)
.with_cmd(self.p.cmd));
.with_cmd(self.cmd));
} else {
debug!("good");
}
@ -202,19 +197,22 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
Ok(())
}
fn validate_conflicts(&self, matcher: &ArgMatcher) -> ClapResult<()> {
fn validate_conflicts(
&mut self,
matcher: &ArgMatcher,
conflicts: &mut Conflicts,
) -> ClapResult<()> {
debug!("Validator::validate_conflicts");
self.validate_exclusive(matcher)?;
let mut conflicts = Conflicts::new();
for arg_id in matcher
.arg_names()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.filter(|arg_id| self.p.cmd.find(arg_id).is_some())
.filter(|arg_id| self.cmd.find(arg_id).is_some())
{
debug!("Validator::validate_conflicts::iter: id={:?}", arg_id);
let conflicts = conflicts.gather_conflicts(self.p.cmd, matcher, arg_id);
let conflicts = conflicts.gather_conflicts(self.cmd, matcher, arg_id);
self.build_conflict_err(arg_id, &conflicts, matcher)?;
}
@ -229,8 +227,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
.arg_names()
.filter_map(|name| {
debug!("Validator::validate_exclusive:iter:{:?}", name);
self.p
.cmd
self.cmd
.find(name)
// Find `arg`s which are exclusive but also appear with other args.
.filter(|&arg| arg.is_exclusive_set() && args_count > 1)
@ -238,10 +235,10 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
// Throw an error for the first conflict found.
.try_for_each(|arg| {
Err(Error::argument_conflict(
self.p.cmd,
self.cmd,
arg,
Vec::new(),
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
))
@ -263,24 +260,24 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
let conflicts = conflict_ids
.iter()
.flat_map(|c_id| {
if self.p.cmd.find_group(c_id).is_some() {
self.p.cmd.unroll_args_in_group(c_id)
if self.cmd.find_group(c_id).is_some() {
self.cmd.unroll_args_in_group(c_id)
} else {
vec![c_id.clone()]
}
})
.filter_map(|c_id| {
seen.insert(c_id.clone()).then(|| {
let c_arg = self.p.cmd.find(&c_id).expect(INTERNAL_ERROR_MSG);
let c_arg = self.cmd.find(&c_id).expect(INTERNAL_ERROR_MSG);
c_arg.to_string()
})
})
.collect();
let former_arg = self.p.cmd.find(name).expect(INTERNAL_ERROR_MSG);
let former_arg = self.cmd.find(name).expect(INTERNAL_ERROR_MSG);
let usg = self.build_conflict_err_usage(matcher, conflict_ids);
Err(Error::argument_conflict(
self.p.cmd, former_arg, conflicts, usg,
self.cmd, former_arg, conflicts, usg,
))
}
@ -293,13 +290,13 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
.collect();
let required: Vec<Id> = used_filtered
.iter()
.filter_map(|key| self.p.cmd.find(key))
.filter_map(|key| self.cmd.find(key))
.flat_map(|arg| arg.requires.iter().map(|item| &item.1))
.filter(|key| !used_filtered.contains(key) && !conflicting_keys.contains(key))
.chain(used_filtered.iter())
.cloned()
.collect();
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&required)
}
@ -311,16 +308,16 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
{
debug!("Validator::gather_requires:iter:{:?}", name);
if let Some(arg) = self.p.cmd.find(name) {
if let Some(arg) = self.cmd.find(name) {
let is_relevant = |(val, req_arg): &(ArgPredicate<'_>, Id)| -> Option<Id> {
let required = matcher.check_explicit(&arg.id, *val);
required.then(|| req_arg.clone())
};
for req in self.p.cmd.unroll_arg_requires(is_relevant, &arg.id) {
for req in self.cmd.unroll_arg_requires(is_relevant, &arg.id) {
self.required.insert(req);
}
} else if let Some(g) = self.p.cmd.find_group(name) {
} else if let Some(g) = self.cmd.find_group(name) {
debug!("Validator::gather_requires:iter:{:?}:group", name);
for r in &g.requires {
self.required.insert(r.clone());
@ -337,7 +334,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
name,
ma.vals_flatten()
);
if let Some(arg) = self.p.cmd.find(name) {
if let Some(arg) = self.cmd.find(name) {
self.validate_arg_num_vals(arg, ma)?;
self.validate_arg_values(arg, ma, matcher)?;
self.validate_arg_num_occurs(arg, ma)?;
@ -357,9 +354,9 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
if ma.get_occurrences() > 1 && !a.is_multiple_occurrences_set() && !a.is_positional() {
// Not the first time, and we don't allow multiples
return Err(Error::unexpected_multiple_usage(
self.p.cmd,
self.cmd,
a,
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
));
@ -372,11 +369,11 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
let occurs = ma.get_occurrences() as usize;
if occurs > max_occurs {
return Err(Error::too_many_occurrences(
self.p.cmd,
self.cmd,
a,
max_occurs,
occurs,
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
));
@ -399,7 +396,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
if should_err {
debug!("Validator::validate_arg_num_vals: Sending error WrongNumberOfValues");
return Err(Error::wrong_number_of_values(
self.p.cmd,
self.cmd,
a,
num,
if a.is_multiple_occurrences_set() {
@ -407,7 +404,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
} else {
total_num
},
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
));
@ -418,7 +415,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
if ma.num_vals() > num {
debug!("Validator::validate_arg_num_vals: Sending error TooManyValues");
return Err(Error::too_many_values(
self.p.cmd,
self.cmd,
ma.vals_flatten()
.last()
.expect(INTERNAL_ERROR_MSG)
@ -426,7 +423,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
.expect(INVALID_UTF8)
.to_string(),
a.to_string(),
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
));
@ -437,11 +434,11 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
if ma.num_vals() < num && num != 0 {
debug!("Validator::validate_arg_num_vals: Sending error TooFewValues");
return Err(Error::too_few_values(
self.p.cmd,
self.cmd,
a,
num,
ma.num_vals(),
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
));
@ -454,14 +451,14 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
// Issue 1105 (https://github.com/clap-rs/clap/issues/1105)
if a.is_takes_value_set() && !min_vals_zero && ma.all_val_groups_empty() {
return Err(Error::empty_value(
self.p.cmd,
self.cmd,
&a.possible_vals
.iter()
.filter(|pv| !pv.is_hide_set())
.map(PossibleValue::get_name)
.collect::<Vec<_>>(),
a,
Usage::new(self.p.cmd)
Usage::new(self.cmd)
.required(&self.required)
.create_usage_with_title(&[]),
));
@ -469,21 +466,24 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
Ok(())
}
fn validate_required(&mut self, matcher: &ArgMatcher) -> ClapResult<()> {
fn validate_required(
&mut self,
matcher: &ArgMatcher,
conflicts: &mut Conflicts,
) -> ClapResult<()> {
debug!("Validator::validate_required: required={:?}", self.required);
self.gather_requires(matcher);
for arg_or_group in self.required.iter().filter(|r| !matcher.contains(r)) {
debug!("Validator::validate_required:iter:aog={:?}", arg_or_group);
if let Some(arg) = self.p.cmd.find(arg_or_group) {
if let Some(arg) = self.cmd.find(arg_or_group) {
debug!("Validator::validate_required:iter: This is an arg");
if !self.is_missing_required_ok(arg, matcher) {
if !self.is_missing_required_ok(arg, matcher, conflicts) {
return self.missing_required_error(matcher, vec![]);
}
} else if let Some(group) = self.p.cmd.find_group(arg_or_group) {
} else if let Some(group) = self.cmd.find_group(arg_or_group) {
debug!("Validator::validate_required:iter: This is a group");
if !self
.p
.cmd
.unroll_args_in_group(&group.id)
.iter()
@ -495,7 +495,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
}
// Validate the conditionally required args
for a in self.p.cmd.get_arguments() {
for a in self.cmd.get_arguments() {
for (other, val) in &a.r_ifs {
if matcher.check_explicit(other, ArgPredicate::Equals(std::ffi::OsStr::new(*val)))
&& !matcher.contains(&a.id)
@ -517,27 +517,20 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
Ok(())
}
fn is_missing_required_ok(&self, a: &Arg<'help>, matcher: &ArgMatcher) -> bool {
fn is_missing_required_ok(
&self,
a: &Arg<'help>,
matcher: &ArgMatcher,
conflicts: &mut Conflicts,
) -> bool {
debug!("Validator::is_missing_required_ok: {}", a.name);
self.validate_arg_conflicts(a, matcher) || self.p.overridden.borrow().contains(&a.id)
}
fn validate_arg_conflicts(&self, a: &Arg<'help>, matcher: &ArgMatcher) -> bool {
debug!("Validator::validate_arg_conflicts: a={:?}", a.name);
a.blacklist.iter().any(|conf| {
matcher.contains(conf)
|| self
.p
.cmd
.find_group(conf)
.map_or(false, |g| g.args.iter().any(|arg| matcher.contains(arg)))
})
let conflicts = conflicts.gather_conflicts(self.cmd, matcher, &a.id);
!conflicts.is_empty()
}
fn validate_required_unless(&self, matcher: &ArgMatcher) -> ClapResult<()> {
debug!("Validator::validate_required_unless");
let failed_args: Vec<_> = self
.p
.cmd
.get_arguments()
.filter(|&a| {
@ -571,7 +564,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
self.required
);
let usg = Usage::new(self.p.cmd).required(&self.required);
let usg = Usage::new(self.cmd).required(&self.required);
let req_args = usg.get_required_usage_from(&incl, Some(matcher), true);
@ -585,8 +578,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.filter(|n| {
// Filter out the args we don't want to specify.
self.p
.cmd
self.cmd
.find(n)
.map_or(true, |a| !a.is_hide_set() && !self.required.contains(&a.id))
})
@ -595,7 +587,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> {
.collect();
Err(Error::missing_required_argument(
self.p.cmd,
self.cmd,
req_args,
usg.create_usage_with_title(&used),
))
@ -654,6 +646,10 @@ impl Conflicts {
}
}
}
// Overrides are implicitly conflicts
conf.extend(arg.overrides.iter().cloned());
conf
} else if let Some(group) = cmd.find_group(arg_id) {
group.conflicts.clone()

38
tests/builder/app_settings.rs
View file

@ -1028,6 +1028,44 @@ fn aaos_opts_w_other_overrides_rev_2() {
assert_eq!(m.value_of("other"), Some("val"));
}
#[test]
fn aaos_opts_w_override_as_conflict_1() {
// opts with other overrides, rev
let res = Command::new("posix")
.args_override_self(true)
.arg(
arg!(--opt <val> "some option")
.required(true)
.overrides_with("other"),
)
.arg(arg!(--other <val> "some other option").required(true))
.try_get_matches_from(vec!["", "--opt=some"]);
assert!(res.is_ok(), "{}", res.unwrap_err());
let m = res.unwrap();
assert!(m.is_present("opt"));
assert!(!m.is_present("other"));
assert_eq!(m.value_of("opt"), Some("some"));
}
#[test]
fn aaos_opts_w_override_as_conflict_2() {
// opts with other overrides, rev
let res = Command::new("posix")
.args_override_self(true)
.arg(
arg!(--opt <val> "some option")
.required(true)
.overrides_with("other"),
)
.arg(arg!(--other <val> "some other option").required(true))
.try_get_matches_from(vec!["", "--other=some"]);
assert!(res.is_ok(), "{}", res.unwrap_err());
let m = res.unwrap();
assert!(!m.is_present("opt"));
assert!(m.is_present("other"));
assert_eq!(m.value_of("other"), Some("some"));
}
#[test]
fn aaos_opts_mult() {
// opts with multiple