search

clap-rs / clap

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

For cli option with custom types, `impl FromStr` with `FromStr::Err` set to `()` won't work #5360

Open SteveLauC opened 4 months ago

SteveLauC commented 4 months ago

Please complete the following tasks

Rust Version

rustc 1.76.0 (07dca489a 2024-02-04)

Clap Version

4.5.1

Minimal reproducible code

use std::str::FromStr;
use clap::Parser;

#[derive(Parser)]
struct Cli {
    #[arg(short, long)]
    foo: Foo,
}

#[derive(Clone)]
struct Foo;

impl FromStr for Foo {
    type Err = ();
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        todo!()
    }
}

fn main() {}

Steps to reproduce the bug with the above code

With the above code, run cargo build

Actual Behaviour

   Compiling rust v0.1.0 (/home/steve/Documents/workspace/rust)
error[E0599]: the method `value_parser` exists for reference `&&&&&&_AutoValueParser<Foo>`, but its trait bounds were not satisfied
    --> src/main.rs:17:5
     |
5    | struct Foo;
     | ----------
     | |
     | doesn't satisfy `Foo: From<&'s std::ffi::OsStr>`
     | doesn't satisfy `Foo: From<&'s str>`
     | doesn't satisfy `Foo: From<OsString>`
     | doesn't satisfy `Foo: From<std::string::String>`
     | doesn't satisfy `Foo: ValueEnum`
     | doesn't satisfy `Foo: ValueParserFactory`
...
17   |     #[arg(short, long)]
     |     ^ method cannot be called on `&&&&&&_AutoValueParser<Foo>` due to unsatisfied trait bounds
     |
    ::: /home/steve/.cargo/registry/src/index.crates.io-6f17d22bba15001f/clap_builder-4.5.1/src/builder/value_parser.rs:2462:1
     |
2462 | pub struct _AutoValueParser<T>(std::marker::PhantomData<T>);
     | ------------------------------ doesn't satisfy `_: _ValueParserViaParse`
     |
     = note: the following trait bounds were not satisfied:
             `Foo: ValueEnum`
             which is required by `&&&&&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaValueEnum`
             `Foo: ValueParserFactory`
             which is required by `&&&&&&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaFactory`
             `Foo: From<OsString>`
             which is required by `&&&&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaFromOsString`
             `Foo: From<&'s std::ffi::OsStr>`
             which is required by `&&&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaFromOsStr`
             `Foo: From<std::string::String>`
             which is required by `&&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaFromString`
             `Foo: From<&'s str>`
             which is required by `&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaFromStr`
             `(): Into<Box<(dyn std::error::Error + Send + Sync + 'static)>>`
             which is required by `_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaParse`
note: the traits `ValueEnum`, `ValueParserFactory`,  and `From` must be implemented
    --> /home/steve/.cargo/registry/src/index.crates.io-6f17d22bba15001f/clap_builder-4.5.1/src/builder/value_parser.rs:2312:1
     |
2312 | pub trait ValueParserFactory {
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
    ::: /home/steve/.cargo/registry/src/index.crates.io-6f17d22bba15001f/clap_builder-4.5.1/src/derive.rs:264:1
     |
264  | pub trait ValueEnum: Sized + Clone {
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
    ::: /home/steve/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/convert/mod.rs:579:1
     |
579  | pub trait From<T>: Sized {
     | ^^^^^^^^^^^^^^^^^^^^^^^^
     = note: this error originates in the macro `clap::value_parser` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0599`.
error: could not compile `rust` (bin "rust") due to 2 previous errors

Expected Behaviour

Build without issues

Additional Context

Related issues:

  1. 4286

    From this issue, I found you can fix it by impl From<String> for Foo.

use std::str::FromStr;
use clap::Parser;

#[derive(Clone)]
struct Foo;

impl FromStr for Foo {
    type Err = ();
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        todo!()
    }
}

impl From<String> for Foo {
    fn from(value: String) -> Self {
        todo!()
    }
}

#[derive(Parser)]
struct Cli {
    #[arg(short, long)]
    foo: Foo,
}

fn main() {
    let cli = Cli::parse();
}
$ cargo b
$ echo $?
0

  1. 4994

    From this one, I found that you can also fix it by changing the () type to other types as described in this comment:

use std::str::FromStr;
use clap::Parser;

#[derive(Clone)]
struct Foo;

impl FromStr for Foo {
    type Err = String;
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        todo!()
    }
}

#[derive(Parser)]
struct Cli {
    #[arg(short, long)]
    foo: Foo,
}

fn main() {
    let cli = Cli::parse();
}
$ cargo b 
$ echo $?
0

Debug Output

error[E0599]: the method `value_parser` exists for reference `&&&&&&_AutoValueParser<Foo>`, but its trait bounds were not satisfied
    --> src/main.rs:17:5
     |
5    | struct Foo;
     | ----------
     | |
     | doesn't satisfy `Foo: From<&'s std::ffi::OsStr>`
     | doesn't satisfy `Foo: From<&'s str>`
     | doesn't satisfy `Foo: From<OsString>`
     | doesn't satisfy `Foo: From<std::string::String>`
     | doesn't satisfy `Foo: ValueEnum`
     | doesn't satisfy `Foo: ValueParserFactory`
...
17   |     #[arg(short, long)]
     |     ^ method cannot be called on `&&&&&&_AutoValueParser<Foo>` due to unsatisfied trait bounds
     |
    ::: /home/steve/.cargo/registry/src/index.crates.io-6f17d22bba15001f/clap_builder-4.5.1/src/builder/value_parser.rs:2462:1
     |
2462 | pub struct _AutoValueParser<T>(std::marker::PhantomData<T>);
     | ------------------------------ doesn't satisfy `_: _ValueParserViaParse`
     |
     = note: the following trait bounds were not satisfied:
             `Foo: ValueEnum`
             which is required by `&&&&&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaValueEnum`
             `Foo: ValueParserFactory`
             which is required by `&&&&&&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaFactory`
             `Foo: From<OsString>`
             which is required by `&&&&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaFromOsString`
             `Foo: From<&'s std::ffi::OsStr>`
             which is required by `&&&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaFromOsStr`
             `Foo: From<std::string::String>`
             which is required by `&&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaFromString`
             `Foo: From<&'s str>`
             which is required by `&_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaFromStr`
             `(): Into<Box<(dyn std::error::Error + Send + Sync + 'static)>>`
             which is required by `_AutoValueParser<Foo>: clap::builder::via_prelude::_ValueParserViaParse`
note: the traits `ValueEnum`, `ValueParserFactory`,  and `From` must be implemented
    --> /home/steve/.cargo/registry/src/index.crates.io-6f17d22bba15001f/clap_builder-4.5.1/src/builder/value_parser.rs:2312:1
     |
2312 | pub trait ValueParserFactory {
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
    ::: /home/steve/.cargo/registry/src/index.crates.io-6f17d22bba15001f/clap_builder-4.5.1/src/derive.rs:264:1
     |
264  | pub trait ValueEnum: Sized + Clone {
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
    ::: /home/steve/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/convert/mod.rs:579:1
     |
579  | pub trait From<T>: Sized {
     | ^^^^^^^^^^^^^^^^^^^^^^^^
     = note: this error originates in the macro `clap::value_parser` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0599`.
error: could not compile `rust` (bin "rust") due to 2 previous errors
epage commented 4 months ago

If I'm understanding correctly, you are saying that Foo::from_str can fail. In that case, we need to be able to render a message but we don't know how to render a message for (). It seems like an error is the correct thing to do.

SteveLauC commented 4 months ago

In that case, we need to be able to render a message but we don't know how to render a message for (). It seems like an error is the correct thing to do.

This is understandable. I am wondering can we:

  1. Give a better error message stating that we don't know how to render the error message for the unit type.
  2. Document it

Either will make the case better

epage commented 4 months ago

We can't improve the error message.

As for documenting it, the challenge is finding the right places where someone is expected to look and won't add to a wall of text that will make it so no one will read any of it.