Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

requires is bypassed when other items from a mutually-exclusive group are present #4707

Open
2 tasks done
mkeeter opened this issue Feb 13, 2023 · 5 comments
Open
2 tasks done
Labels
A-validators Area: ArgMatches validation logi C-bug Category: Updating dependencies

Comments

@mkeeter
Copy link

mkeeter commented Feb 13, 2023

Please complete the following tasks

Rust Version

rustc 1.65.0 (897e37553 2022-11-02)

Clap Version

4.1.4

Minimal reproducible code

use clap::{ArgGroup, Parser};

#[derive(Parser, Debug)]
#[clap(group = ArgGroup::new("command").multiple(false))]
struct Args {
    #[clap(long, group = "command")]
    read: bool,

    #[clap(long, group = "command")]
    write: bool,

    #[clap(long, requires = "read")]
    show_hex: bool,
}

fn main() {
    let args = Args::parse();
    println!("{args:?}");
}

Steps to reproduce the bug with the above code

cargo run -- --write --show-hex

Actual Behaviour

This command runs without an error. This is surprising, because --show-hex is annotated with requires = "read", and we did not provide the --read argument.

Expected Behaviour

Clap should print an error indicating that the required argument --read is not present, e.g.

error: the following required arguments were not provided:
  --read

Usage: clap-test --read --show-hex

For more information, try '--help'.

Additional Context

If I remove either read or write from the command group, then everything behaves as expected. This suggests to me that Clap is accepting any member of the command group, instead of the one specifically named in the requires annotation.

Debug Output

clap-test git:(master) ✗ cargo run -- --write --show-hex
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/clap-test --write --show-hex`
[      clap::builder::command] 	Command::_do_parse
[      clap::builder::command] 	Command::_build: name="clap-test"
[      clap::builder::command] 	Command::_propagate:clap-test
[      clap::builder::command] 	Command::_check_help_and_version:clap-test expand_help_tree=false
[      clap::builder::command] 	Command::long_help_exists
[      clap::builder::command] 	Command::_check_help_and_version: Building default --help
[      clap::builder::command] 	Command::_propagate_global_args:clap-test
[clap::builder::debug_asserts] 	Command::_debug_asserts
[clap::builder::debug_asserts] 	Arg::_debug_asserts:read
[clap::builder::debug_asserts] 	Arg::_debug_asserts:write
[clap::builder::debug_asserts] 	Arg::_debug_asserts:show_hex
[clap::builder::debug_asserts] 	Arg::_debug_asserts:help
[clap::builder::debug_asserts] 	Command::_verify_positionals
[        clap::parser::parser] 	Parser::get_matches_with
[        clap::parser::parser] 	Parser::get_matches_with: Begin parsing 'RawOsStr("--write")' ([45, 45, 119, 114, 105, 116, 101])
[        clap::parser::parser] 	Parser::possible_subcommand: arg=Ok("--write")
[        clap::parser::parser] 	Parser::get_matches_with: sc=None
[        clap::parser::parser] 	Parser::parse_long_arg
[        clap::parser::parser] 	Parser::parse_long_arg: Does it contain '='...
[        clap::parser::parser] 	Parser::parse_long_arg: Found valid arg or flag '--write'
[        clap::parser::parser] 	Parser::parse_long_arg("write"): Presence validated
[        clap::parser::parser] 	Parser::react action=SetTrue, identifier=Some(Long), source=CommandLine
[        clap::parser::parser] 	Parser::react: has default_missing_vals
[        clap::parser::parser] 	Parser::remove_overrides: id="write"
[   clap::parser::arg_matcher] 	ArgMatcher::start_custom_arg: id="write", source=CommandLine
[      clap::builder::command] 	Command::groups_for_arg: id="write"
[   clap::parser::arg_matcher] 	ArgMatcher::start_custom_arg: id="Args", source=CommandLine
[   clap::parser::arg_matcher] 	ArgMatcher::start_custom_arg: id="command", source=CommandLine
[        clap::parser::parser] 	Parser::push_arg_values: ["true"]
[        clap::parser::parser] 	Parser::add_single_val_to_arg: cur_idx:=1
[        clap::parser::parser] 	Parser::get_matches_with: After parse_long_arg ValuesDone
[        clap::parser::parser] 	Parser::get_matches_with: Begin parsing 'RawOsStr("--show-hex")' ([45, 45, 115, 104, 111, 119, 45, 104, 101, 120])
[        clap::parser::parser] 	Parser::possible_subcommand: arg=Ok("--show-hex")
[        clap::parser::parser] 	Parser::get_matches_with: sc=None
[        clap::parser::parser] 	Parser::parse_long_arg
[        clap::parser::parser] 	Parser::parse_long_arg: Does it contain '='...
[        clap::parser::parser] 	Parser::parse_long_arg: Found valid arg or flag '--show-hex'
[        clap::parser::parser] 	Parser::parse_long_arg("show-hex"): Presence validated
[        clap::parser::parser] 	Parser::react action=SetTrue, identifier=Some(Long), source=CommandLine
[        clap::parser::parser] 	Parser::react: has default_missing_vals
[        clap::parser::parser] 	Parser::remove_overrides: id="show_hex"
[   clap::parser::arg_matcher] 	ArgMatcher::start_custom_arg: id="show_hex", source=CommandLine
[      clap::builder::command] 	Command::groups_for_arg: id="show_hex"
[   clap::parser::arg_matcher] 	ArgMatcher::start_custom_arg: id="Args", source=CommandLine
[        clap::parser::parser] 	Parser::push_arg_values: ["true"]
[        clap::parser::parser] 	Parser::add_single_val_to_arg: cur_idx:=2
[        clap::parser::parser] 	Parser::get_matches_with: After parse_long_arg ValuesDone
[        clap::parser::parser] 	Parser::add_defaults
[        clap::parser::parser] 	Parser::add_defaults:iter:read:
[        clap::parser::parser] 	Parser::add_default_value: doesn't have conditional defaults
[        clap::parser::parser] 	Parser::add_default_value:iter:read: has default vals
[        clap::parser::parser] 	Parser::add_default_value:iter:read: wasn't used
[        clap::parser::parser] 	Parser::react action=SetTrue, identifier=None, source=DefaultValue
[   clap::parser::arg_matcher] 	ArgMatcher::start_custom_arg: id="read", source=DefaultValue
[        clap::parser::parser] 	Parser::push_arg_values: ["false"]
[        clap::parser::parser] 	Parser::add_single_val_to_arg: cur_idx:=3
[        clap::parser::parser] 	Parser::add_defaults:iter:write:
[        clap::parser::parser] 	Parser::add_default_value: doesn't have conditional defaults
[        clap::parser::parser] 	Parser::add_default_value:iter:write: has default vals
[        clap::parser::parser] 	Parser::add_default_value:iter:write: was used
[        clap::parser::parser] 	Parser::add_defaults:iter:show_hex:
[        clap::parser::parser] 	Parser::add_default_value: doesn't have conditional defaults
[        clap::parser::parser] 	Parser::add_default_value:iter:show_hex: has default vals
[        clap::parser::parser] 	Parser::add_default_value:iter:show_hex: was used
[        clap::parser::parser] 	Parser::add_defaults:iter:help:
[        clap::parser::parser] 	Parser::add_default_value: doesn't have conditional defaults
[        clap::parser::parser] 	Parser::add_default_value:iter:help: doesn't have default vals
[     clap::parser::validator] 	Validator::validate
[      clap::builder::command] 	Command::groups_for_arg: id="write"
[     clap::parser::validator] 	Conflicts::gather_direct_conflicts id="write", conflicts=["read"]
[     clap::parser::validator] 	Conflicts::gather_direct_conflicts id="Args", conflicts=[]
[     clap::parser::validator] 	Conflicts::gather_direct_conflicts id="command", conflicts=[]
[      clap::builder::command] 	Command::groups_for_arg: id="show_hex"
[     clap::parser::validator] 	Conflicts::gather_direct_conflicts id="show_hex", conflicts=[]
[     clap::parser::validator] 	Validator::validate_conflicts
[     clap::parser::validator] 	Validator::validate_exclusive
[     clap::parser::validator] 	Validator::validate_exclusive:iter:"write"
[     clap::parser::validator] 	Validator::validate_exclusive:iter:"Args"
[     clap::parser::validator] 	Validator::validate_exclusive:iter:"command"
[     clap::parser::validator] 	Validator::validate_exclusive:iter:"show_hex"
[     clap::parser::validator] 	Validator::validate_conflicts::iter: id="write"
[     clap::parser::validator] 	Conflicts::gather_conflicts: arg="write"
[     clap::parser::validator] 	Conflicts::gather_conflicts: conflicts=[]
[     clap::parser::validator] 	Validator::validate_conflicts::iter: id="show_hex"
[     clap::parser::validator] 	Conflicts::gather_conflicts: arg="show_hex"
[     clap::parser::validator] 	Conflicts::gather_conflicts: conflicts=[]
[     clap::parser::validator] 	Validator::validate_required: required=ChildGraph([])
[     clap::parser::validator] 	Validator::gather_requires
[     clap::parser::validator] 	Validator::gather_requires:iter:"write"
[     clap::parser::validator] 	Validator::gather_requires:iter:"Args"
[     clap::parser::validator] 	Validator::gather_requires:iter:"Args":group
[     clap::parser::validator] 	Validator::gather_requires:iter:"command"
[     clap::parser::validator] 	Validator::gather_requires:iter:"command":group
[     clap::parser::validator] 	Validator::gather_requires:iter:"show_hex"
[     clap::parser::validator] 	Validator::validate_required: is_exclusive_present=false
[     clap::parser::validator] 	Validator::validate_required:iter:aog="read"
[     clap::parser::validator] 	Validator::validate_required:iter: This is an arg
[     clap::parser::validator] 	Validator::is_missing_required_ok: read
[     clap::parser::validator] 	Conflicts::gather_conflicts: arg="read"
[      clap::builder::command] 	Command::groups_for_arg: id="read"
[     clap::parser::validator] 	Conflicts::gather_direct_conflicts id="read", conflicts=["write"]
[     clap::parser::validator] 	Conflicts::gather_conflicts: conflicts=["write", "write"]
[     clap::parser::validator] 	Validator::is_missing_required_ok: true (self)
[   clap::parser::arg_matcher] 	ArgMatcher::get_global_values: global_arg_vec=[]
Args { read: false, write: true, show_hex: true }
@mkeeter mkeeter added the C-bug Category: Updating dependencies label Feb 13, 2023
@epage
Copy link
Member

epage commented Feb 13, 2023

I think this is a duplicate of #4520. If not, let us know in what way it is different and we can re-open

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Feb 13, 2023
@mkeeter
Copy link
Author

mkeeter commented Feb 13, 2023

Hmmm, I had found that issue when searching, but I'm not sure if it's the same thing.

#4520 is triggered when you use requires and conflicts_with together; this issue is when you use requires and group together.

It may be that it's the same issue under the hood, but I don't know enough about Clap internals to say for sure.

@epage epage reopened this Feb 13, 2023
@epage
Copy link
Member

epage commented Feb 13, 2023

They are both forms of conflicts but I've re-opened this for the sake of making sure we have tests for both use cases.

@epage epage added A-parsing Area: Parser's logic and needs it changed somehow. A-validators Area: ArgMatches validation logi and removed A-parsing Area: Parser's logic and needs it changed somehow. labels Feb 13, 2023
@TD-Sky
Copy link

TD-Sky commented Jul 21, 2023

Same problem.

@epage epage changed the title requires will accept any item from a group requires is bypassed when other items from a mutually-exclusive group are present Jul 21, 2023
@Xophmeister
Copy link

I had the same problem, but I was able to sort-of solve it by lifting the branch that admits options into its own struct. Something like this:

use clap::{ArgGroup, Args, Parser};

#[derive(Args, Debug)]
struct Foo {
    #[arg(long)]
    foo: String,

    #[arg(long, requires = "foo")]
    quux: Option<String>,
}

#[derive(Debug, Parser)]
#[command(group = ArgGroup::new("mx")
    .multiple(false)
    .required(true)
    .args(&["foo", "bar"])
)]
struct Cli {
    #[command(flatten)]
    foo_w_option: Option<Foo>,

    #[arg(long)]
    bar: Option<String>,
}

fn main() {
    let args = Cli::parse();
    println!("{args:?}");
}

I say "sort-of" solve it because, while the parser correctly forbids the --bar X --quux Y case, the error message is not very clear:

error: The following required argument was not provided: foo

Usage: clap-mx-group-w-option [OPTIONS] <--foo <FOO>|--bar <BAR>>

For more information, try '--help'.

This is actually OK -- although I would expect to see <--foo <FOO> [--quux <QUUX>]|--bar <BAR>> -- but in my actual use-case, I'm using subcommands and including positional arguments in the mutual exclusivity group. This reverts the usage text to the top-level, rather than the subcommand's usage text. (I don't know if that's worth an issue in its own right...)

Xophmeister added a commit to tweag/topiary that referenced this issue Aug 17, 2023
Xophmeister added a commit to tweag/topiary that referenced this issue Aug 18, 2023
* Note about not using infer_subcommands

* --tolerate-parsing-errors doesn't make sense for visualisation

* Separate out input source types so we can create a unified interface

* Fallback to the given output path if canonicalisation fails

Resolves #588

* We're going to need an InputFile, too

* WIP: InputFile type

* Correct blunder regarding --query

Note that clap doesn't support (--foo [--bar] | --quux) groups very
cleanly; it was a bit of a hack to get this to work, with the result
being the error text being a bit off when an illegal combination is
attempted. I've attempted to compensate for this by making the long help
text quite explicit.

Also updated clap, which contains the fix for clap-rs/clap#5022

* Missed change to README

* Add note RE clap-rs/clap#4707 workaround

* Machinery to unify inputs + downstream use to reimplement visualisation

* Don't flatten-away errors

* Don't open the input file until we need to read from it

* Into InputFrom should be from &AtLeastOneInput

* Add logging

* Moar logging!1!!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-validators Area: ArgMatches validation logi C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

4 participants