A required ArgGroup is satisfied by a default value

[eternaleye]

Rust Version

• 1.24.1

Affected Version of clap

• 2.31.1

Expected Behavior Summary

I'd expect that an ArgGroup with required(true) would fail unless the user explicitly passed an argument, even if one of the arguments sets a default_value. The documentation notes that this can be done by checking occurrences_of.

Actual Behavior Summary

If one argument of the group sets a default_value, the group is satisfied.

Steps to Reproduce the issue

• Define an App with two arguments, one of which takes a defaulted value
• Put the two arguments into a group with .required(true)
• Run the command without passing either

Sample Code or Link to Sample Code

extern crate clap;

use clap::{App,Arg,ArgGroup};

fn main() {
    App::new("example")
        .arg(Arg::with_name("foo")
            .long("foo")
            .takes_value(true))
        .arg(Arg::with_name("bar")
            .long("bar")
            .takes_value(true))
            .default_value("1"))
       .group(ArgGroup::with_name("baz")
            .args(&["foo", "bar"])
            .required(true))
        .get_matches();
}

[eternaleye]

Got into a discussion about this on IRC with @anp, pasting log:

tl;dr: I still feel that .required(true) on an ArgGroup shouldn't count default values as present, but adding a distinct .required_explicit(true) with that behavior is a possible alternative.

<anp> eternaleye: doesn't seem like a bug to me :P

<eternaleye> anp: The tool in question uses --output-tests to mean "output them to the default dir", --output-tests dir to mean "output them to a specific dir", and not passing it as "do not output tests"
<eternaleye> It also requires that at-least-one output option (of --output, --output-tests, and --synth-dump) is passed
<eternaleye> The only way to do this currently is to code a manual check that occurrences_of("output-tests") != 0
<eternaleye> But that doesn't allow using clap's logic for missing required arguments

<anp> yeah it seems to me that a default value for an argument means that it's specified
<anp> maybe there's a way for clap to provide a 0-or-1 arg version of the flag that doesn't use the default
<anp> the 0 arg version would trigger the "default value" behavior
<anp> but in application code
<anp> and then the required arg group setting would handle the occurences of stuff

<eternaleye> anp: I disagree, because the behavior you're arguing for is the same as not specifying the arg group at all :P
<eternaleye> Which is a bit silly IMO
<eternaleye> Since the whole point of an arg group is to require that at-least-one (or exactly-one) is passed

<anp> if its not inside an arg group, then having both default and required on an arg is redundant
<anp> bc it will always be specified

<eternaleye> But this is an arg group

<anp> but you shouldn't have different behavior just because you add an arg to a group
<anp> its not supposed to change how an argument is interpreted
<anp> just guarantee that one of the group is used

<eternaleye> I think you misunderstand clap arg groups

<anp> perhaps, its been a bit since i used them

<eternaleye> > Perhaps the most common use of ArgGroups is to require one and only one argument to be present out of a given set.
<eternaleye> This completely breaks down if ArgGroup is satisfied by a default
<eternaleye> Because now, it's illegal to pass anything except the one with the default :P
<eternaleye> That line's from the docs

<anp> yeah i'm reading them too
<anp> i think the appropriate change would be to panic if you try to construct an arg group with an arg that has a default
<anp> if it's a "exactly one" arg group

<eternaleye> The key thing here is the ability to query the arg group for what the user passed
<eternaleye> And then given that, look up the values for those args
<eternaleye> Which might be a default value, even if the user passed the --option

<anp> hm if i'm the application author
<anp> i would generally like to know what my arg rules resolve to
<anp> what the user passed is immaterial honestly
<anp> bc what comes to my application from clap should be sanitized, parsed, etc
<anp> and defaults are a part of that
<anp> from my application's perspective, a default value should be no different from the user specifying a value

<eternaleye> anp: But then you get relatively nasty commandlines, where you have to specify --tests separately from --tests-dir
<eternaleye> Instead of being able to pass --tests, or --tests=dir
<eternaleye> As a user, that's gross

<anp> i think that constraint is better expressed as min_values/max_values on the arg
<anp> than a default
<anp> and in your validator/parser for the arg you handle the default case there

<eternaleye> But then you lose --help printing the default value
<eternaleye> Which is really nice
<eternaleye> I really, really want to stay entirely declarative

<anp> you can change the help output for that arg

<eternaleye> Because going imperative loses features that help the user
<eternaleye> anp: Yes, and now it's not DRY
<eternaleye> And the help output can desync from the actual default

<anp> not if you make it a const

<eternaleye> anp: And then I have to dick around with concat!(), great

<anp> how so?
<anp> paths are relative to the working dir no?

<eternaleye> Because the help output is Put generated tests into this directory [default: tests]
<eternaleye> I'd need to concat!("Put generated tests into this directory [default: ", TEST_DIR, "]")

<anp> help text that doesn't seem too bad, i do wish there were a const_fn format macro

<eternaleye> And if clap ever changed its format, then it's out of sync there, and... ugh
<eternaleye> It's just nasty

<anp> anyways, dinner time

<eternaleye> Another option is a .required_explicit(true)

<anp> still think it should be a panic to add a defaulted arg to a group
<anp> yeah that would be a good addition imo

<eternaleye> Which is like .required(true) for arg groups, but checks that the user passed it
<eternaleye> Mind if I tack this log onto the issue?

<anp> eternaleye: not at all!

[kbknapp]

Thanks for all the details! Yeah, unfortunately I don't want to change the behavior as a default value is a value. However, I can add require_explicit to the issue tracker if someone would like to take a stab at adding it.

I can take a look at adding it once v3 is out. I'd like to think on the ramifications of require_explicit too to make sure the naming and implications are all correct too though. If someone wants to take a stab at this, please provide a spec for how you see this working, including how it interacts with things like ENV vars, etc.

[CreepySkeleton]

This sounds like a logic problem to me: having a default value implies that the arg or group is NOT required - why the hell would it have a default value otherwise? If a default value was specified, it's implied that there's circumstance this default value would be used in, but if the arg was also required, than the default value would never be used - user would be forced to supply their own value. Having default_value("some") with required(true) is simply meaningless; it's a bug.

I think we should panic (maybe only in debug?) if both have been detected on one arg/group.

[pksunkara]

This is how a normal Arg behaves too. If you specify required(true) and default_value it will still pass when no value is provided.

[pksunkara]

@CreepySkeleton Is this issue fixed? I remember you fixing something about this.

[eternaleye]

@CreepySkeleton I disagree specifically in the case of ArgGroup, though I agree for Arg.

I gave a specific use case in my second comment, which is inexpressible with Clap's current behavior, as far as I can tell:

<eternaleye> anp: The tool in question uses --output-tests to mean "output them to the default dir", --output-tests dir to mean "output them to a specific dir", and not passing it as "do not output tests"
<eternaleye> It also requires that at-least-one output option (of --output, --output-tests, and --synth-dump) is passed
<eternaleye> The only way to do this currently is to code a manual check that occurrences_of("output-tests") != 0
<eternaleye> But that doesn't allow using clap's logic for missing required arguments

This is a use case that arose from me trying to re-implement an existing tool in Rust using Clap, and it is (so far as I can tell) an eminently sensible behavior. The issue here is perhaps the conflation of "a default value for an optional argument to a flag" with "a default value if a flag is not passed at all", where the latter should satisfy an ArgGroup and the former should not, but the desired behavior that Clap makes inexpressible doesn't seem like a logic error to me.

[CreepySkeleton]

a default value if a flag is not passed at all

Gotcha. And this is exactly why we're introducing default_missing_value - use the default if the flag's been passed, but otherwise don't. I think it allows for the use cause you want.

[rivy]

@eternaleye , to read up the impetus and semantics of the new default_missing_value, see PRs #1468, #1507, #1587.
I believe it was ultimately merged into the v3 branch.

[pksunkara]

Closing this since the needed API is already added.

[CreepySkeleton]

@pksunkara This issue is a reminder to add a check:

I think we should panic (maybe only in debug?) if both have been detected on one arg/group.