Struct length is ignored by `assert_de_tokens`
peterjoel opened this issue · 3 comments
I just noticed this anomaly in the code I provided for a different issue.
#[derive(Deserialize)]
#[serde(rename = "Message")]
struct Flattened {
a: String,
b: String,
}
impl Flattened {
fn new(a: &str, b: &str) -> Flattened {
Flattened {
a: a.to_string(),
b: b.to_string(),
}
}
}
#[test]
fn both_deserialize_as_flat_struct_tokens() {
let expected_tokens = &[
Token::Struct { name: "Message", len: 5 },
Token::Str("a"),
Token::Str("v1"),
Token::Str("b"),
Token::Str("99"),
Token::StructEnd,
];
let flattened = Flattened::new("v1", "99");
assert_de_tokens(&flattened, expected_tokens);
}
The struct only has two fields, but the test is checking for len: 5
, and still passes.
@dtolnay, it seems that this issue should be moved to https://github.com/serde-rs/test.
Actually, I think, it should be just closed, because len
is just a hint from a deserializer to a type being deserialized, and not a strict requirement
Only assert_ser_tokens
pays attention to this len, comparing it against the len passed to serialize_struct
, while assert_de_tokens
just ignores it, as you found.
One might expect assert_de_tokens
to compare this len against the number of field names passed by the Deserialize impl to deserialize_struct
.
The reason that is not a good option is because sometimes these 2 disagree.
For example, this struct always has 3 possible field names for deserialization, but always serializes 2 fields.
#[derive(Serialize, Deserialize)]
pub struct Demo {
a: i32,
#[serde(alias = "c")]
b: i32,
}
As one counterproposal perhaps assert_de_tokens
should pay attention and assert_tokens
should not. Currently assert_tokens
is equivalent to assert_ser_tokens
+ assert_de_tokens
and I think we are better off keeping it that way.
Another possibility is make assert_de_tokens
compare the len not against the field names of the deserialize_struct
call, but against the number of fields actually present in the test tokens list. I think that's not so useful; the point is to test the Deserialize impl, not test the test code. This only makes the test code more tedious.