Using default_value_ifs with required returns error about argument not being provided when default is matched sometime after 3.1.12

[dsherret]

Please complete the following tasks

• I have searched the discussions
• I have searched the open and rejected issues

Rust Version

1.61

Clap Version

3.2.5

Minimal reproducible code

#[test]
fn default_ifs_arg_required_not_present() {
    let r = Command::new("df")
        .arg(arg!(--opt <FILE> "some arg").required(false))
        .arg(
            arg!([arg] "some arg")
                .default_value_ifs(&[
                    ("opt", Some("some"), Some("default")),
                ])
                .required(true),
        )
        .try_get_matches_from(vec!["", "--opt=some"]);
    assert!(r.is_ok(), "{}", r.unwrap_err());
    let m = r.unwrap();
    assert!(m.contains_id("arg"));
    assert_eq!(
        m.get_one::<String>("arg").map(|v| v.as_str()).unwrap(),
        "default"
    );
}

Steps to reproduce the bug with the above code

1. Add this to tests/builder/default_vals.rs in the clap repo.
2. cargo test default_ifs_arg_required_not_present

Actual Behaviour

error: The following required arguments were not provided:

Expected Behaviour

Should parse.

Additional Context

This error started occuring sometime after 3.1.12

Debug Output

No response

[epage]

That was broken with #3793 which was released in v3.2.0.

This is a tricky one.

This was not a documented or tested use case and in general our various validation rules should be for non-default arguments (we've been fine making similar changes across minor releases). We were also needing to allow default_value and required to play nicely for the new actions like ArgAction::SetTrue which was needed by the derive API (see #3786). At this point, a lot depends on this combination and can't be trivially backed out.

On the other hand, this broke you.

We'll need to do some investigation on this to see if there is a way to make both cases work. Otherwise we'll either need to

• Say this is ok and leave it as-is
• Yank the 3.2 releases and release a 4.0 and push back our other changes to a 5.0.

[epage]

Closing this as the new behavior is the expected behavior and the concern was with existing users which only means 3.x users. At this point, making a change to something like this would be too disruptive to v3-master branch

[crowlKats]

@epage Deno actually relied on this behaviour (deno run --v8-flags=--help would be allowed but any other value for the v8-flags flag would require also a file to be specified as a positional final argument), but only now figured out (and only recently even really noticed) why it hasn't worked properly since we upgraded clap past v3.2 (ref denoland/deno#20145).
Is there any interest in revisiting this behaviour or adding capability to enable it?

[epage]

Considering the existing behavior is the expected behavior, it would require v5 for us to break it. I also don't see us likely to change course on this which would require new validation logic. In general, I've been trying to hold off on new validation logic because there is a long tail of feature requests for it and would go counter to our build-time / binary size goals since it'd be all-or-nothing. Instead, I've been holding out for #3476 which will allow us and users to have more validation options without impacting build-time and binary size as much. Unfortunately, #3476 doesn't feel like enough of a priority atm for me to set aside the other stuff I'm doing to focus on it.