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

Can’t define traits on TryIntoError #396

Open
mina86 opened this issue Aug 12, 2024 · 3 comments
Open

Can’t define traits on TryIntoError #396

mina86 opened this issue Aug 12, 2024 · 3 comments

Comments

@mina86
Copy link

mina86 commented Aug 12, 2024

I’m working with a third-party crate which defines a ClientError type and a From<&'static str> trait for that type. Further, the crate has a generic method with the following two bounds: T: TryInto<Foo> and T::Error: Into<ClientError>. Lastly, in my code I have an enum whose one of the variant carries Foo value and uses derive_more::TryInto to define the required TryInto<Foo> implementation.

With derive_more 0.99 this worked fine. derive_more created the implementation with &'static str error which satisfies Into<ClientError> bound.

Sadly, with derive_more 1.0 the conversion error is now TryIntoError which cannot be converted into ClientError and because both of those types are third-party I cannot add the implementation.

I wonder if it would be possible to add a customisable error. For example I could then do:

#[derive(derive_more::TryInto)]
#[try_into(error = MyError)]
pub enum Blah {
    Foo(third_party::Foo),
    Bar(third_party::Bar),
    Baz(third_party::Baz),
}

#[derive(derive_more::From)]
struct MyError(derive_more::TryIntoError<Blah>);

and then define all necessary conversions on MyError. Even more flexible if I could define error type and constructor separately.

This actually is something I’ve suggested a while back, see #315, and back then I concluded that TryIntoError would work for my case. Alas, turns out not quite.

PS. variant_names and output_type be pub?

@mkatychev
Copy link

mkatychev commented Aug 30, 2024

clap's derive builder provides a pretty flexible pattern for this approach:

#[derive(Parser, Debug, Clone)]
#[command(name = "pretty-print-file")]
pub struct MyPrinter {
    #[arg(short, long, value_parser = |p: &str| PrettyFile::from_path(p))]
    pub file_to_print: Option<PrettyFile>,
}

In that same way, a closure could be passed in so that the message (or possibly Self) can be changed:

#[derive(derive_more::TryInto)]
#[try_into(error = |msg| MyError(format!("try_into: {msg}")))]
pub enum Blah {
    Foo(third_party::Foo),
    Bar(third_party::Bar),
    Baz(third_party::Baz),
}

@JelteF
Copy link
Owner

JelteF commented Sep 5, 2024

I think this is very reasonable. I think the best solution would contain the following three things:

  1. Allow providing an error type. This error type should have a From implementation for TryIntoError.
  2. Allow providing a custom constructor for an error type by using a closure that takes a TryIntoError.
  3. Make output_type and variant_names public members of TryIntoError. I think we should probably make variant_names an &'static [] in that case in that case.

@JelteF
Copy link
Owner

JelteF commented Sep 5, 2024

PRs for this feature are welcome (to be clear, any of the three features I described above features could be contributed separately in a PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants