Provide means to specify error string when matching variant is not found

[rusterize]

Right now --when no matching variant is found-- (parsing a string into an enum using the derived from_str) a constant string error is returned saying "Matching variant not found". There are two improvements that can be done on this:

• Better default: A better default would probably be something like "expected 'string1', 'string2', 'string3' or 'string4'". Even though it costs to iterate over the options, it doesn't matter, because it would only happen when no valid string is found.

• Custom error: The user should be able to specify a custom error message, something like:

#[derive(EnumString)]
#[strum(onerror="We like you, but can't understand what you wrote, please use 'string1' or 'string2'")
enum Foo {
   #[strum(serialize="string1")]
   Option1,
   #[strum(serialize="string2")]
   Option2,
}

** special token: You could even provide a special token for the onerror keyword so that the user doesn't have to repeat the options all the time. Something like onerror="We like you, but can't understand what you wrote, please use {opts}".

[Peternator7]

I appreciate the feedback, I'm hesitant to make this change. At the moment the error message comes from the impl of Error for strum::ParseError. In order to make this change, the VariantNotFound variant of parse error would need to take a &'static str as an argument, which is a breaking change.

You can still change the error message by matching on the result and printing out a custom message.

match Color::from_str("red") {
    Ok(col) => /* do something with color */,
    Err(_) => println!("We couldn't understand that. Try entering red, blue, or green"),
}

[alberdingk-thijm]

What about allowing different custom error types to be used, rather than fiddling with strum::ParseError?

For instance, I'm using failure for a project and wanted to parse to a Vec<Type>, where Type is an EnumString derived enum. It would be useful to use a stronger error type for the problem.

I can, as you point out, convert the strum::ParseError type to something else using or_else, but if it were simply a matter of adding a #[strum(error=ErrorType)] procedural macro, that would make the code more clear and succinct.

[Peternator7]

@alberdingk-thijm

That's potentially doable. Right now there aren't any attributes on the enum itself, only variants, but that could change. :)

#[strum(error=ErrorType)] isn't quite enough information to instantiate any arbitrary object from. There's the matter of picking the enum variant, or it's possible the error type isn't an enum at all. I see a couple options on how to add a feature like this:

1. Expose a #[strum(error=ErrorType)] like you suggested. Your ErrorType should implement From<ParseError>. Then, if parsing fails, strum will call ::strum::ParseError::VariantNotFound.into() and return your error type.

2. Accept both an ErrorType and a "new" function for creating your error type. While this means you don't have to implement any traits, it adds quite a bit of noise to the attributes.

   #[strum(error=::crate::mod::error::Error, error_fn=::crate::mod::error::Error::new)
   pub enum Error {}
   

Let me know if you have any thoughts or other implementation ideas. Otherwise I'll probably see how well idea 1 works!

[alberdingk-thijm]

I had also imagined it like 1, as that keeps the code fairly clean. Using From<ParseError> also fits the user's intuition nicely in that the error function is associated with a ParseError, rather than some other error type.

[dzmitry-lahoda]

For me, something like would work:

#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub enum ParseError {
    VariantNotFound(String), // variant name
}

another related issue #91

will donate 3 usd for fix.

[TedDriggs]

@Peternator7 I'm the maintainer of derive-builder, and we just went through this issue with our 0.10.0 release. What we ended up with that worked well was:

// exported by derive_builder
pub struct UninitializedFieldError(&'static str);

We then allow the user to drop in any custom error type that impl From<UninitializedFieldError>. Ours had the advantage of knowing the str would be static, but in this case you could do something similar:

pub struct VariantNotFoundError<'a>(&'a str);

impl<'a> From<VariantNotFoundError<'a>> for ParseError {
    fn from(_: VariantNotFoundError<'a>) -> Self {
        ParseError::VariantNotFound
    }
}

Then the enum-level attribute has a default value of strum::ParseError, making it backwards-compatible.

[orowith2os]

How goes this? I'd love something along the lines of:

#[strum(error= CustomError(String))]
pub enum Foo {}

Where String gets replaced with the invalid variant.

[qrilka]

@Peternator7 any updates on this one?
I do quite like strum but I miss a solution for this one: as a workaround I remap strum::ParseError to an error with a bit more context. Having it as something closer to strum derive macros would be an improvement.

[qrilka]

@Peternator7 would you mind commenting the current state of the issue? Was it closed as WONTFIX as I couldn't find any related updates in the changelog.

[Peternator7]

Closed this one accidentally sorry while cleaning up some older issues. Re-opened, but tbh, I don't have any plans around this particular issue.