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

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

Open
2 tasks done
SteveLauC opened this issue Feb 17, 2024 · 3 comments
Open
2 tasks done
Labels
C-bug Category: Updating dependencies

Comments

@SteveLauC
Copy link

SteveLauC commented Feb 17, 2024

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. Derive requires field types to impl Clone #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. Derive: specifying invalid struct field type fails with Epic error message #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
Copy link
Member

epage commented Feb 17, 2024

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
Copy link
Author

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
Copy link
Member

epage commented Feb 18, 2024

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.

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