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

required conflicts with required_unless* #4715

Closed
2 tasks done
Ciubix8513 opened this issue Feb 18, 2023 · 4 comments
Closed
2 tasks done

required conflicts with required_unless* #4715

Ciubix8513 opened this issue Feb 18, 2023 · 4 comments
Labels
C-bug Category: Updating dependencies

Comments

@Ciubix8513
Copy link

Please complete the following tasks

Rust Version

1.67.1

Clap Version

4.1.6

Minimal reproducible code

use clap::{Parser};

#[derive(Parser,Debug)]
struct Args{
    #[arg(short)]
    a: bool,
    #[arg(required_unless_present("a"))]
    test: String, 
}

fn main() {
    let args = Args::parse();
}

Steps to reproduce the bug with the above code

cargo run

Actual Behaviour

'main' panicked at 'Argument test: required conflicts with required_unless*', /home/luna/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-4.1.6/src/builder/debug_asserts.rs:186:13

Expected Behaviour

required_unless* should automatically infer remove required and the code example should run normally

Additional Context

Currently to fix this issue you need to add default_value to the arg definition, and then it works as expected

Debug Output

No response

@Ciubix8513 Ciubix8513 added the C-bug Category: Updating dependencies label Feb 18, 2023
@Ciubix8513 Ciubix8513 changed the title required conflicts with required_unless* required conflicts with required_unless* Feb 18, 2023
@epage
Copy link
Member

epage commented Feb 19, 2023

Even if we resolved things to prevent that assert, it would then fail at runtime. If the "unless" case happens, clap will have nothing to put into that field and will report an error.

You either need to make the argument optional by changing the type from String to Option<String> or provide a default.

@Ciubix8513
Copy link
Author

Yea, that makes sense.
But wouldn't it be possible that the compiler gives an error saying that required_unless* requires the argument to be Option<> or have a defualt value?
I'm not quite sure if that's possible (I'm pretty new to rust), but that would make it less confusing, to people who are not as familiar with Clap or Rust.

@epage
Copy link
Member

epage commented Feb 19, 2023

Yes, ideally, more of our checks would be done at compile-time. A couple of challenges with that

  • We support merging structs and the compiler doesn't tell us anything about the other struct
  • There are a lot of corner cases for how these arguments interact that it would be complex and brittle, especially because...
  • This would be hard to maintain as we'd have to have a copy of all of the rules available at both compile time and runtime.

See also #3133

@epage
Copy link
Member

epage commented Feb 19, 2023

As it seems this direct issue is resolved, I'm going to go ahead and close thos

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Feb 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

2 participants