#[clap(parse(try_from_str))] doesn't use try_from(&str)

[epage]

Issue by TilCreator
Friday Apr 09, 2021 at 11:02 GMT
Originally opened as clap-rs/clap#2437

---

Please complete the following tasks

I have searched the discussions
I have searched the existing issues

Rust Version

rustc 1.50.0 (cb75ad5db 2021-02-10)

Clap Version

3.0.0-beta.2

Minimal reproducible code

use reqwest::Url;
use scraper::Selector;

/*
Available functions:
reqwest::Url::try_from(s: &'a str) -> Result<Self, Self::Error>
reqwest::Url::from_str(input: &str) -> Result<Url, crate::ParseError>
scraper::Selector::try_from(s: &'i str) -> Result<Self, Self::Error>
*/

#[derive(Clap, Debug)]
#[clap(name = env!("CARGO_PKG_NAME"))]
struct Args {
    // There is no problem with parsing url. it's just a working example
    #[clap(
        takes_value = true,
        parse(try_from_str)
    )]
    url: Url,
    #[clap(
        takes_value = true,
        parse(try_from_str),
    )]
    selector: Selector,
}

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

Steps to reproduce the bug with the above code

cargo run -- https://exameple.com body

Actual Behaviour

It does not compile because #[clap(parse(try_from_str))] doesn't find Selector::try_from(&str).

Expected Behaviour

It compiles and finds the try_from(&str) of Selector.

Additional Context

From the name try_from_str I would expect Clap to use try_from(&str) -> Result<_> rather than (/both) from(&str) -> Result<_>.
#1661

Debug Output

[dependencies]
reqwest = "0.11.2"
scraper = "0.12.0"
clap = "3.0.0-beta.2"

[epage]

Comment by epage
Saturday Oct 16, 2021 at 19:42 GMT

---

It looks like this also exists in structopt.

The challenge is naming. For our From<T> versions, we need to differentiate T, which leads to from_str for From<&str> and from_os_str for From<&OsStr>. That then conflicts with using from_str for FromStr.

Granted, if we did auto-detection like #2298, this problem goes away except when the user wants to override for a specific case.

[epage]

Comment by epage
Saturday Oct 16, 2021 at 19:47 GMT

---

Forgot to add, from observation, it looks like the naming scheme has less to do with what trait is the default but describes what the function signature is when providing a version of it.

From that perspective, I'd be tempted to Close this as Won't Fix.

[epage]

Comment by epage
Saturday Oct 16, 2021 at 19:49 GMT

---

I've not confirmed it but I believe the user workaround would be parse(try_from_str = Url::try_from)

[epage]

Comment by TilCreator
Saturday Oct 16, 2021 at 20:29 GMT

---

The workaround works for most cases, but some use weird types where a wrapper is still necessary. Here are examples for both:
My original example with the workaround:

use clap::Clap;
use reqwest::Url;
use scraper::Selector;
use std::convert::TryFrom;

/*
Available functions:
reqwest::Url::try_from(s: &'a str) -> Result<Self, Self::Error>
reqwest::Url::from_str(input: &str) -> Result<Url, crate::ParseError>
scraper::Selector::try_from(s: &'i str) -> Result<Self, Self::Error>
*/

#[derive(Clap, Debug)]
#[clap(name = env!("CARGO_PKG_NAME"))]
struct Args {
    // There is no problem with parsing url. it's just a working example (also from `try_from`)
    #[clap(
        takes_value = true,
        parse(try_from_str = Url::try_from),
    )]
    url: Url,
    #[clap( // Fails with many compile errors
        takes_value = true,
        parse(try_from_str = Selector::try_from),
    )]
    selector: Selector,
}

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

    println!("{:#?}", args);
}

That sadly doesn't fix it completely, because the Selector type doesn't play nice.
For such cases a wrapper case can be used, here an example from one of my projects for that:
Wrapper definition
Wrapper usage

If you don't have to add anything this can be closed, thx

[epage]

Comment by pksunkara
Saturday Oct 16, 2021 at 20:38 GMT

---

Let's keep this open since I have the impression of tackling this as part of #2683.

[epage]

Comment by epage
Saturday Oct 16, 2021 at 20:39 GMT

---

    url: Url,
    #[clap( // Fails with many compile errors
        takes_value = true,
        parse(try_from_str = Selector::try_from),
    )]
    selector: Selector,*/

The errors have less to do with TryFrom vs FromStr and more to do with the fact that scrapers error type does not impl Error, making it incompatible with most of the Rust ecosystem.

Technically, you do not need a full wrapper type to resolve this. You can do it just by defining a function and passing that in instead.