assert_de_tokens_error panics if deserialization fails in an early field

[tspiteri]

For the code below, in both the Fine and Bad cases, an attempt is made to deserialize the nonzero field with a value of 0, which returns an error. In the Fine case, the nonzero field is the last field and everything works as expected. In the Bad case, the nonzero field is the first field, and assert_de_tokens_error panics with '2 remaining tokens'.

#[macro_use]
extern crate serde_derive;
extern crate serde;
#[cfg(test)]
extern crate serde_test;

use serde::de::{self, Deserialize, Deserializer, Visitor};
use std::fmt::{Formatter, Result as FmtResult};

#[derive(Serialize)]
pub struct Nonzero(i32);

struct NonzeroVisitor;

impl<'de> Visitor<'de> for NonzeroVisitor {
    type Value = Nonzero;

    fn expecting(&self, formatter: &mut Formatter) -> FmtResult {
        formatter.write_str("an integer not equal to 0")
    }

    fn visit_i32<E>(self, value: i32) -> Result<Self::Value, E>
    where
        E: de::Error,
    {
        if value != 0 {
            Ok(Nonzero(value))
        } else {
            Err(E::custom(String::from("value is zero")))
        }
    }
}
impl<'de> Deserialize<'de> for Nonzero {
    fn deserialize<D>(deserializer: D) -> Result<Nonzero, D::Error>
    where
        D: Deserializer<'de>,
    {
        deserializer.deserialize_i32(NonzeroVisitor)
    }
}

#[derive(Serialize, Deserialize)]
pub struct Fine {
    other: i32,
    nonzero: Nonzero,
}

#[derive(Serialize, Deserialize)]
pub struct Bad {
    nonzero: Nonzero,
    other: i32,
}

#[cfg(test)]
mod tests {
    use serde_test::{self, Token};

    #[test]
    fn fine() {
        use Fine;
        let tokens_fine = [
            Token::Struct {
                name: "Fine",
                len: 2,
            },
            Token::Str("other"),
            Token::I32(5),
            Token::Str("nonzero"),
            Token::I32(0),
            Token::StructEnd,
        ];
        serde_test::assert_de_tokens_error::<Fine>(&tokens_fine, "value is zero");
    }

    #[test]
    fn bad() {
        use Bad;
        let tokens_bad = [
            Token::Struct {
                name: "Bad",
                len: 2,
            },
            Token::Str("nonzero"),
            Token::I32(0),
            Token::Str("other"),
            Token::I32(5),
            Token::StructEnd,
        ];
        serde_test::assert_de_tokens_error::<Bad>(&tokens_bad, "value is zero");
    }
}

[dtolnay]

Thanks! I would welcome a pull request to fix this.

[tspiteri]

I would submit a PR that removes the check that is panicking, but I don't know if I'm missing something.

It seems that commit 093201abfb736b7b32828696d7d190603ebd6aa6 introduced an assertion that there are no tokens after an error, which is causing this issue. I do not know the reason behind that check, so I'm not comfortable simply deleting it in case it serves some other purpose. And I don't know if the similar check in assert_ser_tokens_error might cause a similar issue to this.

[dtolnay]

Ah, good call. Since the error is detected on the Token::I32(0) token, how about removing the rest of the tokens that are not needed for the test?

let tokens_bad = [
    Token::Struct {
        name: "Bad",
        len: 2,
    },
    Token::Str("nonzero"),
    Token::I32(0),
];
serde_test::assert_de_tokens_error::<Bad>(&tokens_bad, "value is zero");

[tspiteri]

Yes, that solves my problem. Closing. Thanks!