Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

serde_json ignores a renamed Struct name when throwing an error. #2402

Closed
GeeWee opened this issue Mar 14, 2023 · 2 comments
Closed

serde_json ignores a renamed Struct name when throwing an error. #2402

GeeWee opened this issue Mar 14, 2023 · 2 comments

Comments

@GeeWee
Copy link

GeeWee commented Mar 14, 2023

When renaming a Struct via Serde, I would expect that this name would also be used in error messages.

I am unsure if this is serde or serde_json behaviour, as I was not able to find the cursory code in either repository, but please feel free to move or tell me to go somewhere else.

See this reproduction case for what happens vs what I expect to happen:

    use serde::{Deserialize, Serialize};
    use serde_json::json;

    #[test]
    fn repro_case() {
        #[derive(Deserialize, Debug)]
        struct Foo {
            bar: Bar,
        }

        #[derive(Deserialize, Debug)]
        #[serde(rename = "AnotherName!")]
        struct Bar {
            qux: String,
        }

        let res = serde_json::from_value::<Foo>(json!({
            "bar": "not_a_struct"
        }))
        .unwrap_err();

        // Passes
        assert_eq!(
            res.to_string(),
            "invalid type: string \"not_a_struct\", expected struct Bar"
        );
        // Does not pass, but should
        assert_eq!(
            res.to_string(),
            "invalid type: string \"not_a_struct\", expected struct AnotherName!"
        );
    }
@dtolnay dtolnay transferred this issue from serde-rs/json Mar 14, 2023
@Mingun
Copy link
Contributor

Mingun commented Aug 11, 2023

This could be expected behavior. At least in one place there is a clear indication, that this was intended -- the rust_name is explicitly distinguished from the type_name:

serde/serde_derive/src/de.rs

Lines 1441 to 1445 in 05a5b7e

let rust_name = params.type_name();
let expecting = format!("adjacently tagged enum {}", rust_name);
let expecting = cattrs.expecting().unwrap_or(&expecting);
let type_name = cattrs.name().deserialize_name();
let deny_unknown_fields = cattrs.deny_unknown_fields();

type_name is used in strings that passed to Deserializer::deserialize_struct, etc., rust_name is used in expecting messages. Should we unify that?

@dtolnay
Copy link
Member

dtolnay commented Aug 11, 2023

I think let's keep this behavior. In cases where just a type name isn't meaningful enough, one can use something like serde(expecting = "a map of proxy configs") to fill in the user-facing message.

invalid type: string "not_a_struct", expected a map of proxy configs

@dtolnay dtolnay closed this as completed Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants