JelteF / derive_more

Some more derive(Trait) options
MIT License
1.68k stars 117 forks source link

Can’t define traits on TryIntoError #396

Open mina86 opened 1 month ago

mina86 commented 1 month ago

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 https://github.com/JelteF/derive_more/issues/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 commented 2 weeks ago

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 commented 1 week ago

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 commented 1 week ago

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).