strum
strum copied to clipboard
Provide means to specify error string when matching variant is not found
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}"
.
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"),
}
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.
@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:
-
Expose a
#[strum(error=ErrorType)]
like you suggested. YourErrorType
should implementFrom<ParseError>
. Then, if parsing fails, strum will call::strum::ParseError::VariantNotFound.into()
and return your error type. -
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!
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.
For me, something like would work:
#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub enum ParseError {
VariantNotFound(String), // variant name
}
another related issue https://github.com/Peternator7/strum/issues/91
will donate 3 usd for fix.
@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.
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.