Merge pull request #353 from fmoor/feature/trailing_comma

change error type for trailing commas, fixes #352

src/de.rs

@@ -105,6 +105,7 @@ impl<'de, R: Read<'de>> Deserializer<R> {
     /// only has trailing whitespace.
     pub fn end(&mut self) -> Result<()> {
         match try!(self.parse_whitespace()) {
+            Some(b',') => Err(self.peek_error(ErrorCode::TrailingComma)),
             Some(_) => Err(self.peek_error(ErrorCode::TrailingCharacters)),
             None => Ok(()),
         }
@@ -517,6 +518,13 @@ impl<'de, R: Read<'de>> Deserializer<R> {
                 self.eat_char();
                 Ok(())
             }
+            Some(b',') => {
+                self.eat_char();
+                match self.parse_whitespace() {
+                    Ok(Some(b']')) => Err(self.peek_error(ErrorCode::TrailingComma)),
+                    _ => Err(self.peek_error(ErrorCode::TrailingCharacters)),
+                }
+            }
             Some(_) => Err(self.peek_error(ErrorCode::TrailingCharacters)),
             None => Err(self.peek_error(ErrorCode::EofWhileParsingList)),
         }
@@ -528,6 +536,7 @@ impl<'de, R: Read<'de>> Deserializer<R> {
                 self.eat_char();
                 Ok(())
             }
+            Some(b',') => Err(self.peek_error(ErrorCode::TrailingComma)),
             Some(_) => Err(self.peek_error(ErrorCode::TrailingCharacters)),
             None => Err(self.peek_error(ErrorCode::EofWhileParsingObject)),
         }
@@ -994,16 +1003,18 @@ impl<'de, 'a, R: Read<'de> + 'a> de::SeqAccess<'de> for SeqAccess<'a, R> {
     where
         T: de::DeserializeSeed<'de>,
     {
-        match try!(self.de.parse_whitespace()) {
+        let peek = match try!(self.de.parse_whitespace()) {
             Some(b']') => {
                 return Ok(None);
             }
             Some(b',') if !self.first => {
                 self.de.eat_char();
+                try!(self.de.parse_whitespace())
             }
-            Some(_) => {
+            Some(b) => {
                 if self.first {
                     self.first = false;
+                    Some(b)
                 } else {
                     return Err(self.de.peek_error(ErrorCode::ExpectedListCommaOrEnd));
                 }
@@ -1011,10 +1022,13 @@ impl<'de, 'a, R: Read<'de> + 'a> de::SeqAccess<'de> for SeqAccess<'a, R> {
             None => {
                 return Err(self.de.peek_error(ErrorCode::EofWhileParsingList));
             }
-        }
+        };
 
-        let value = try!(seed.deserialize(&mut *self.de));
-        Ok(Some(value))
+        match peek {
+            Some(b']') => Err(self.de.peek_error(ErrorCode::TrailingComma)),
+            Some(_) => Ok(Some(try!(seed.deserialize(&mut *self.de)))),
+            None => Err(self.de.peek_error(ErrorCode::EofWhileParsingValue)),
+        }
     }
 }
 
@@ -1062,6 +1076,7 @@ impl<'de, 'a, R: Read<'de> + 'a> de::MapAccess<'de> for MapAccess<'a, R> {
 
         match peek {
             Some(b'"') => seed.deserialize(MapKey { de: &mut *self.de }).map(Some),
+            Some(b'}') => Err(self.de.peek_error(ErrorCode::TrailingComma)),
             Some(_) => Err(self.de.peek_error(ErrorCode::KeyMustBeAString)),
             None => Err(self.de.peek_error(ErrorCode::EofWhileParsingValue)),
         }

src/error.rs

@@ -75,6 +75,7 @@ impl Error {
             ErrorCode::InvalidUnicodeCodePoint |
             ErrorCode::KeyMustBeAString |
             ErrorCode::LoneLeadingSurrogateInHexEscape |
+            ErrorCode::TrailingComma |
             ErrorCode::TrailingCharacters |
             ErrorCode::UnexpectedEndOfHexEscape |
             ErrorCode::RecursionLimitExceeded => Category::Syntax,
@@ -244,6 +245,9 @@ pub enum ErrorCode {
     /// Lone leading surrogate in hex escape.
     LoneLeadingSurrogateInHexEscape,
 
+    /// JSON has a comma after the last value in an array or map.
+    TrailingComma,
+
     /// JSON has non-whitespace trailing characters after the value.
     TrailingCharacters,
 
@@ -321,6 +325,7 @@ impl Display for ErrorCode {
             ErrorCode::LoneLeadingSurrogateInHexEscape => {
                 f.write_str("lone leading surrogate in hex escape")
             }
+            ErrorCode::TrailingComma => f.write_str("trailing comma"),
             ErrorCode::TrailingCharacters => f.write_str("trailing characters"),
             ErrorCode::UnexpectedEndOfHexEscape => f.write_str("unexpected end of hex escape"),
             ErrorCode::RecursionLimitExceeded => f.write_str("recursion limit exceeded"),

tests/test.rs

@@ -894,7 +894,7 @@ fn test_parse_list() {
             ("[ ", "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,]", "expected value at line 1 column 4"),
+            ("[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"),
         ],
@@ -1111,9 +1111,14 @@ fn test_parse_enum_errors() {
              "invalid type: map, expected tuple variant Animal::Frog at line 1 column 10"),
             ("{\"Cat\":[]}", "invalid length 0, expected tuple of 2 elements at line 1 column 9"),
             ("{\"Cat\":[0]}", "invalid length 1, expected tuple of 2 elements at line 1 column 10"),
-            ("{\"Cat\":[0, \"\", 2]}", "trailing characters at line 1 column 14"),
+            ("{\"Cat\":[0, \"\", 2]}", "trailing characters at line 1 column 16"),
             ("{\"Cat\":{\"age\": 5, \"name\": \"Kate\", \"foo\":\"bar\"}",
              "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\":{\"age\": 2, \"name\": \"Kate\",}}",
+             "trailing comma at line 1 column 34"),
         ],
     );
 }