clap-rs / clap

A full featured, fast Command Line Argument Parser for Rust
docs.rs/clap
Apache License 2.0
13.64k stars 1.02k forks source link

value_parser un-nests Option<Option<Foo>> multiple times #5536

Closed mwlon closed 1 week ago

mwlon commented 2 weeks ago

Please complete the following tasks

Rust Version

1.79

Clap Version

4.5.3

Minimal reproducible code

use clap::Parser;

fn parse_foo(s: &str) -> Option<usize> {
  if s == "auto" {
    None
  } else {
    usize::from_str(s).unwrap()
  }
}

#[derive(Parser)]
struct Opt {
  #[arg(long, value_parser=parse_foo, default_value="auto")]
  foo: Option<Option<usize>>
}

fn main() {
  Opt::parse();
}

Steps to reproduce the bug with the above code

cargo run -- --foo=auto

Actual Behaviour

something like

Mismatch between definition and access of `delta_encoding_order`. Could not downcast to TypeId { t: (8519994227001858441, 10522819541147869382) }, need to downcast to TypeId { t: (10946805347594816691, 17696603323637579145) }

Expected Behaviour

happy 0 exit code

Additional Context

It appears that clap derive expects parse_foo to just return usize instead of Option<usize>, and if we change it to do that, the above code compiles. This might make sense for singly-nested Option<usize>, but definitely seems wrong for Option<Option<usize>>. I assume this isn't the desired behavior; if it is, I think an explanation is warranted. I couldn't find any mention of how value_parser interacts with Option<> typing from the docs, but it's always possible I missed something.

Side question: if I want the above behavior where "auto" or no arg => None, int_string => Some(int), is there a better way to do it? I would prefer not to define an enum.

Debug Output

No response

epage commented 1 week ago

This isn't value_parser but our derive code which is inferring behavior based on types. You can workaround it by fully qualifying Option.

See #4626 for improving this.

mwlon commented 1 week ago

Thanks, I was able to work around it using that trick. I'll close this issue since #4626 probably covers it.