Tighten up support for trailing commas in array literals.
The initial support for trailing commas in array literals had a bug
when `deserialize_struct()` was used in conjunction with
`#[serde(deny_unknown_fields)]`. I noticed this because there was a
test case in `test_parse_enum_errors()` that I expected to fail
(because it assumed trailing commas were disallowed), yet it was
still passing (i.e., the trailing comma error was still being thrown).
It appeared that when the number of fields to be deserialized was
known, instead invoking `next_element_seed()` until it returned
`Ok(None)`, serde would stop invoking it after it saw the last
required field and move on to the `end_seq()` call.
Before this change, `end_seq()` would fail because the `SeqAccess`
would not have consumed the final trailing comma. This changes the
implementation of `SeqAccess` so that it is responsible for consuming
the comma associated with a value before returning the value.
Overall, I think this implementation of
`SeqAccess::next_element_seed()` is cleaner than the original.
diff --git a/src/de.rs b/src/de.rs
index f3951b2..851b490 100644
--- a/src/de.rs
+++ b/src/de.rs
@@ -1664,15 +1664,11 @@
struct SeqAccess<'a, R: 'a> {
de: &'a mut Deserializer<R>,
- saw_trailing_comma_from_previous_entry: bool,
}
impl<'a, R: 'a> SeqAccess<'a, R> {
fn new(de: &'a mut Deserializer<R>) -> Self {
- SeqAccess {
- de: de,
- saw_trailing_comma_from_previous_entry: true,
- }
+ SeqAccess { de }
}
}
@@ -1683,36 +1679,25 @@
where
T: de::DeserializeSeed<'de>,
{
- let peek = match try!(self.de.parse_whitespace()) {
- Some(b']') => {
- return Ok(None);
- }
- Some(b',') if !self.saw_trailing_comma_from_previous_entry => {
- self.de.eat_char();
- self.saw_trailing_comma_from_previous_entry = true;
- try!(self.de.parse_whitespace())
- }
- Some(b',') if self.saw_trailing_comma_from_previous_entry => {
- return Err(self.de.peek_error(ErrorCode::TrailingComma));
- }
- Some(b) => Some(b),
- None => {
- return Err(self.de.peek_error(ErrorCode::EofWhileParsingList));
- }
- };
-
- match peek {
- Some(b']') => return Ok(None),
+ match try!(self.de.parse_whitespace()) {
+ Some(b']') => Ok(None),
Some(_) => {
- if self.saw_trailing_comma_from_previous_entry {
- let result = Ok(Some(try!(seed.deserialize(&mut *self.de))));
- self.saw_trailing_comma_from_previous_entry = false;
- result
- } else {
- Err(self.de.peek_error(ErrorCode::ExpectedListCommaOrEnd))
+ let result = Ok(Some(try!(seed.deserialize(&mut *self.de))));
+ match try!(self.de.parse_whitespace()) {
+ Some(b',') => self.de.eat_char(),
+ Some(b']') => {
+ // Ignore.
+ }
+ Some(_) => {
+ return Err(self.de.peek_error(ErrorCode::ExpectedListCommaOrEnd));
+ }
+ None => {
+ return Err(self.de.peek_error(ErrorCode::EofWhileParsingList));
+ }
}
+ result
}
- None => Err(self.de.peek_error(ErrorCode::EofWhileParsingValue)),
+ None => Err(self.de.peek_error(ErrorCode::EofWhileParsingList)),
}
}
}
diff --git a/tests/test.rs b/tests/test.rs
index e864127..20c066f 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -1053,7 +1053,7 @@
("[", "EOF while parsing a list at line 1 column 1"),
("[ ", "EOF while parsing a list at line 1 column 2"),
("[1", "EOF while parsing a list at line 1 column 2"),
- ("[1,", "EOF while parsing a value at line 1 column 3"),
+ ("[1,", "EOF while parsing a list at line 1 column 3"),
// ("[1,]", "trailing comma at line 1 column 4"),
("[1 2]", "expected `,` or `]` at line 1 column 4"),
("[]a", "trailing characters at line 1 column 3"),
@@ -1245,7 +1245,7 @@
"unknown field `foo`, expected `age` or `name` at line 1 column 39"),
// JSON does not allow trailing commas in data structures
- ("{\"Cat\":[0, \"Kate\",]}", "trailing comma at line 1 column 19"),
+ // ("{\"Cat\":[0, \"Kate\",]}", "trailing comma at line 1 column 19"),
// ("{\"Cat\":{\"age\": 2, \"name\": \"Kate\",}}",
// "trailing comma at line 1 column 34"),
],
diff --git a/tests/trailing_comma.rs b/tests/trailing_comma.rs
index 83a2deb..04b26d6 100644
--- a/tests/trailing_comma.rs
+++ b/tests/trailing_comma.rs
@@ -1,4 +1,9 @@
#[macro_use]
+extern crate serde_derive;
+
+extern crate serde;
+
+#[macro_use]
extern crate serde_jsonrc;
use serde_jsonrc::{from_str, Value};
@@ -26,3 +31,24 @@
let value: Value = from_str(s).unwrap();
assert_eq!(value, json!({"key": ["one", "two", "three"]}));
}
+
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
+enum Animal {
+ Dog,
+ Frog(String, Vec<isize>),
+ Cat { age: usize, name: String },
+ AntHive(Vec<String>),
+}
+
+#[test]
+fn test_parse_enum_animal_and_no_other_test() {
+ let animal: Animal = from_str("{\"Cat\":[0, \"Kate\",]}").unwrap();
+ assert_eq!(
+ animal,
+ Animal::Cat {
+ age: 0,
+ name: "Kate".to_string()
+ }
+ );
+}
