Enhancement: Better ParseError

[JRAndreassen]

Hi...

Love the crate...
I'd love it even more if the from_str() gave better error messages.

When I parse a file with serde I often get:

Failed: "Custom { field: \"Matching variant not found\" }"

It would be ooohhh so much better if it said:

Failed: Custom { field: \"Matching variant not found: [EnumName:'Value']\" }"

Perhaps change:

pub enum ParseError {
    VariantNotFound(String),
}

...
    fn from_str( s: &str,    ) ...
{
...
        _ => ::core::result::Result::Err(::strum::ParseError::VariantNotFound(format!("StructName[{}]", s))),
}

I know it's extra overhead, but ohhh so useful.
Could make it optional and/or only in Test/Debug
Since serde does not always give a good error, even with serde_path_to_error...

Thanks
JR

[Peternator7]

Hello @JRAndreassen, glad to hear you like the crate! What does your Deserialize impl look like? I wasn't aware that serde would use a type's FromStr impl when deriving Deserialize

[JRAndreassen]

Hi @Peternator7 ,

Thanks for getting back to me...

It does not use it directly...
But the handy crate "serde_with" makes it possible.
It eliminates the redundant serde attributes and code, by telling serde to use "from_str"[DeserializeFromStr] and "Display"[SerializeDisplay]

serde = { version = "^1.0", features = ["derive","rc"] }
serde_derive = "^1.0"
serde_json = "^1.0"
serde_path_to_error = "*"
serde_with = "*"

strum = "*"
strum_macros = "*"

#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, DisplayMore)]
#[derive(AsRefStr, IntoStaticStr, EnumString, EnumMessage, EnumIter, FromRepr)]
#[strum(ascii_case_insensitive)]
#[serde_as]
#[derive(SerializeDisplay, DeserializeFromStr)]
#[repr(u8)]
pub enum Units {
   #[strum(to_string = "BytesBandwidth", serialize = "Bandwidth", serialize = "OctetsBandwidth"")]
   BytesBandwidth = 0,
   #[strum(to_string = "BytesMemory", serialize = "Memory", serialize = "Mem")]
   BytesMemory = 1,
   #[strum(to_string = "BytesDisk", serialize = "Bytes", serialize = "Disk")]
   BytesDisk = 2,
   #[strum(to_string = "Temperature", serialize = "Temprature", serialize = "Temp")]
   Temperature = 3,
   #[strum(to_string = "Percent", serialize = "Pct", serialize = "%",)]
   Percent = 4,
   #[strum(to_string = "TimeResponse", serialize = "ResponseTime", serialize = "ms")]
   TimeResponse = 5,
   #[strum(to_string = "TimeSeconds", serialize = "Seconds", serialize = "sec")]
   TimeSeconds = 6,
   #[strum(to_string = "Custom", serialize = "Other")]
   Custom = 7,
...
   #[strum(to_string = "TimeHours", serialize = "Hours", serialize = "hrs")]
   TimeHours = 13,
   Invalid = 99,
 }

Which makes quite a difference, since I generate code from API(OpenApi) definitions, which
has some rather large structures.

Also... is there a particular reason why the "to_string" value gets evaluated last ?
Just asking for efficiency...
It would make sense to order the most likely choices first in the list...

    fn from_str(s: &str,  ) -
> ::core::result::Result<ChannelUnits, <Self as ::core::str::FromStr>::Err> {
...
            s if s.eq_ignore_ascii_case("Hours") => {
                ::core::result::Result::Ok(ChannelUnits::TimeHours)
            }
            s if s.eq_ignore_ascii_case("hrs") => {
                ::core::result::Result::Ok(ChannelUnits::TimeHours)
            }
            s if s.eq_ignore_ascii_case("TimeHours") => {
                ::core::result::Result::Ok(ChannelUnits::TimeHours)
            }
...
}
...
    fn get_serializations(&self) -> &'static [&'static str] {
...
     static ARR: [&'static str; 3usize] = ["Hours", "hrs", "TimeHours"];
...
   }

Thanks
JR

[FaveroFerreira]

I support this, it would be nice to know what value I am trying to parse into an Enum in the error message. @Peternator7 , if you think this is valid maybe I can takkle this issue in a PR.

[ju6ge]

I support this as well 👍

Just as a note when I deserialize an enum with serde I get the following useful error message:

unknown variant `bla`, expected one of `Red`, `Green`, `Yellow`, `Blue`

Would be awesome if strum had a similar message.