Enhancement: better error messages for missing struct fields?

[maurges]

Currently missing fields in struct gives no line info, not even the typename info, which makes it hard to find where in the row of same-y declarations you missed a field. For example:

#[derive(serde::Deserialize, Debug)]
struct Test {
    f1: i64,
    f2: String,
}

fn main() {
    let s = r#"
Test (
    f1: 55,
)
"#;
    let x: Result<Test, _> = ron::from_str(s);
    println!("{:?}", x);
}

gives: Err(Error { code: Message("missing field f2"), position: Position { line: 0, col: 0 } })

It would be great if I could get line info, or at least the name "Test" of the record.

Also there is a similar problem with enum variants. Which seems doubly strange, as when I write an incorrect constructor for a struct, like replacing "Test" with "NotTest" in the example above, I get precise error location and type info.

[juntyr]

On master, #356 is already included (this was excluded from v0.7.1 in #382), which does give the correct line info (also for enum variants):

Err(SpannedError { code: Message("missing field `f2`"), position: Position { line: 4, col: 1 } })

I'll check if we could also give the name of the struct or enum variant where the field is missing

[torkleyy]

I think we should make necessary API breaking changes now (if any) and then release master as 0.8, so the new features, especially better errors, become available.

[juntyr]

Ok, so the error message here comes from serde's default implementation of Error::missing_field, which just generates this string. If we wanted, ron could overwrite all or some of the Error methods to capture the results in nicer error variants - this might be a good idea on its own, just to make error handling easier.

If we then have such variants, we could populate them with extra optional information that would be None by default, e.g. the name of the surrounding struct, the name of the field etc. While this might make for fantastic error messages in some cases, it could also bloat the error type. For now, I don't think adding this functionality to ron is necessary.

@d86leader What I use for such nice contextual error messages is the serde_path_to_error crate:

let mut de_ron = ron::Deserializer::from_str_with_options(ron_str, ron_options)?;

let mut track = serde_path_to_error::Track::new();
let de = serde_path_to_error::Deserializer::new(&mut de_ron, &mut track);

let result = match Test::deserialize(de) {
    Ok(args) => Ok(args),
    Err(err) => {
        let path = track.path();
        let err = de_ron.span_error(err);

        Err(format!(
            "{}{} @ ({}):\n{}",
            path,
            if path.iter().count() >= 1 { "" } else { "*" },
            err.position,
            err.code,
        ))
    },
}

[maurges]

Thanks! I agree, if there is line info, having constructor or type name is not necessarily necessary. I will close this issue then.

Can't wait for 0.8, with this and supplying extensions via decoder options!

[juntyr]

@torkleyy Do you think we should convert serde's errors into nice error variants? For the invalid_[type|value|length] error methods we wouldn't be able to do better than stringifying the expected vs provided elements anyways, but we could very nicely represent the unknown_variant, unknown_field, missing_field, and duplicate_field errors.

[torkleyy]

If we can provide better error information without overly cluttering the codebase, absolutely, yes!