feat(Parser): Handled trailing comma

Author: JulesGuesnon

README.md

@@ -77,7 +77,3 @@ Parser ./benches/data/canada.json
 time:   [42.026 ms 42.188 ms 42.341 ms]
 thrpt:  [50.702 MiB/s 50.886 MiB/s 51.082 MiB/s]
 ```
-
-# Todo
-
-- [ ] Handle trailing comma

src/error.rs

@@ -21,6 +21,7 @@ pub enum Kind {
     NotAnArray,
     NotANumber,
     InvalidValue(String),
+    TrailingComma,
     NomError(nom::error::ErrorKind),
     // Used when an error will be remaped
     ToBeDefined,

src/input.rs

@@ -210,7 +210,7 @@ impl<'a, R: FromStr> ParseTo<R> for Input<'a> {
 impl<'a> Slice<Range<usize>> for Input<'a> {
     fn slice(&self, range: Range<usize>) -> Self {
         let next_data = self.data.slice(range);
-        println!("Range: Next data: {:?}", next_data);
+
         Self {
             data: next_data,
             line: 0,
@@ -219,10 +219,8 @@ impl<'a> Slice<Range<usize>> for Input<'a> {
     }
 }
 
-impl<'a> Slice<RangeTo<usize>> for Input<'a> {
-    fn slice(&self, range: RangeTo<usize>) -> Self {
-        let next_data = self.data.slice(range);
-
+impl<'a> Input<'a> {
+    fn slice_common(&self, next_data: &'a str) -> Self {
         let offset = self.data.offset(next_data);
 
         let old_data = self.data.slice(..offset);
@@ -259,44 +257,18 @@ impl<'a> Slice<RangeTo<usize>> for Input<'a> {
         }
     }
 }
+
+impl<'a> Slice<RangeTo<usize>> for Input<'a> {
+    fn slice(&self, range: RangeTo<usize>) -> Self {
+        let next_data = self.data.slice(range);
+
+        self.slice_common(next_data)
+    }
+}
 impl<'a> Slice<RangeFrom<usize>> for Input<'a> {
     fn slice(&self, range: RangeFrom<usize>) -> Self {
         let next_data = self.data.slice(range);
 
-        let offset = self.data.offset(next_data);
-
-        let old_data = self.data.slice(..offset);
-
-        if offset == 0 {
-            return Self {
-                data: next_data,
-                line: self.line,
-                col: self.col,
-            };
-        }
-
-        let new_line_iter = Memchr::new(b'\n', old_data.as_bytes());
-
-        let mut lines_to_add = 0;
-        let mut last_index = None;
-
-        for i in new_line_iter {
-            lines_to_add += 1;
-            last_index = Some(i);
-        }
-        let last_index = last_index.map(|v| v + 1).unwrap_or(0);
-
-        let col = num_chars(old_data.as_bytes().slice(last_index..));
-
-        Self {
-            data: next_data,
-            line: self.line + lines_to_add,
-            col: if lines_to_add == 0 {
-                self.col + col
-            } else {
-                // When going to a new line, char starts at 1
-                col + 1
-            },
-        }
+        self.slice_common(next_data)
     }
 }

src/parser.rs

@@ -3,7 +3,7 @@ use crate::input::Input;
 use crate::value::{Number, Position, SpannedValue, Value};
 use nom::bytes::complete::take_till;
 use nom::character::complete::digit0;
-use nom::combinator::eof;
+use nom::combinator::{eof, opt};
 use nom::error::ParseError;
 use nom::multi::many_till;
 use nom::{
@@ -39,7 +39,6 @@ where
     I: Clone,
 {
     move |i: I| {
-        let i = i.clone();
         let result = parser.parse(i.clone());
 
         match result {
@@ -49,6 +48,17 @@ where
     }
 }
 
+pub fn map_parser<I, O1, O2, E: ParseError<I>, F, G>(
+    mut parser: F,
+    mut applied_parser: G,
+) -> impl FnMut(I) -> IResult<I, O2, E>
+where
+    F: Parser<I, O1, E>,
+    G: FnMut((I, O1)) -> IResult<I, O2, E>,
+{
+    move |input: I| applied_parser(parser.parse(input)?)
+}
+
 fn parse_true(i: Span) -> Result<bool> {
     value(true, tag("rue"))(i).or_else(|_: Err<Error>| {
         let start = Position::from_ahead(i);
@@ -278,7 +288,16 @@ fn array(i: Span) -> Result<Vec<SpannedValue>> {
                         }
                     }),
                 ),
-                json_value,
+                or_else(json_value, |e: Err<Error>, i| {
+                    // If it succeeds, it means that it's a trailing comma
+                    let _ = preceded(multispace0, char(']'))(i).map_err(|_: Err<Error>| e)?;
+
+                    Err(Err::Failure(Error::new(
+                        Position::from_ahead(i),
+                        Position::from_ahead(i),
+                        Kind::TrailingComma,
+                    )))
+                }),
             ),
             preceded(
                 multispace0,
@@ -298,23 +317,27 @@ fn array(i: Span) -> Result<Vec<SpannedValue>> {
 }
 
 fn key_value(i: Span<'_>) -> Result<(String, SpannedValue)> {
+    let (i, comma) = opt(char(','))(i)?;
+
     let pos_before_space = Position::from(i);
 
     let (i, _) = multispace0(i)?;
 
-    if i.starts_with('}') || i.is_empty() {
+    if (i.starts_with('}') || i.is_empty()) && comma.is_none() {
         // Key value is called in a loop, and only an error can stop it
         return Err(Err::Error(Error::default()));
     }
 
     let (i, key) = preceded(char('"'), string)(i).or_else(|e| match e {
         Err::Error(mut e) => {
             let (i, key) = take_until_delimiter(i, true)?;
+
+            let end = Position::from_ahead(i);
+
             if key.is_empty() {
-                e.start = pos_before_space;
+                e.start = pos_before_space.clone();
             }
             e.kind = Kind::InvalidKey(key);
-            let end = Position::from_ahead(i);
             e.end = end;
 
             Err(Err::Failure(e))
@@ -341,25 +364,41 @@ fn key_value(i: Span<'_>) -> Result<(String, SpannedValue)> {
 fn hash(i: Span<'_>) -> Result<HashMap<String, SpannedValue>> {
     let start = Position::from_ahead(i);
 
-    let result = terminated(
+    let result: Result<HashMap<String, SpannedValue>> = terminated(
         map(
             separated_list0(
                 preceded(
                     multispace0,
-                    or_else(char(','), |e: Err<Error>, i| {
-                        let (i, _) = multispace0(i)?;
-
-                        match e {
-                            Err::Error(mut e) if !i.is_empty() && !i.starts_with('}') => {
-                                e.kind = Kind::MissingComma;
-                                e.start = start.clone();
-                                e.end.col -= 1;
-
-                                Err(Err::Failure(e))
+                    or_else(
+                        map_parser(char(','), |(i, _): (Span, char)| {
+                            let (j, _) = multispace0(i)?;
+
+                            if j.starts_with('}') {
+                                let position = Position::from_ahead(i);
+                                Err(Err::Failure(Error::new(
+                                    position.clone(),
+                                    position,
+                                    Kind::TrailingComma,
+                                )))
+                            } else {
+                                Ok((i, ','))
                             }
-                            e => Err(e),
-                        }
-                    }),
+                        }),
+                        |e: Err<Error>, i| {
+                            let (i, _) = multispace0(i)?;
+
+                            match e {
+                                Err::Error(mut e) if !i.is_empty() && !i.starts_with('}') => {
+                                    e.kind = Kind::MissingComma;
+                                    e.start = start.clone();
+                                    e.end.col -= 1;
+
+                                    Err(Err::Failure(e))
+                                }
+                                e => Err(e),
+                            }
+                        },
+                    ),
                 ),
                 key_value,
             ),

tests/parser.rs

@@ -312,6 +312,26 @@ mod error {
             Ok(_) => panic!("Not supposed to happen"),
         }
     }
+
+    #[test]
+    fn trailing_comma() {
+        let json = r#"{"hello": "world", }"#;
+
+        let parsed = parse(json);
+
+        assert!(parsed.is_err());
+
+        match parsed {
+            Err(e) => {
+                assert_eq!(e.start.line, 1);
+                assert_eq!(e.start.col, 18);
+                assert_eq!(e.end.line, 1);
+                assert_eq!(e.end.col, 18);
+                assert_eq!(e.kind, Kind::TrailingComma)
+            }
+            Ok(_) => panic!("Not supposed to happen"),
+        }
+    }
 }
 
 mod string {
@@ -497,6 +517,26 @@ mod array {
             Ok(_) => panic!("Not supposed to happen"),
         }
     }
+
+    #[test]
+    fn trailing_comma() {
+        let json = r#"["hello", "world", ]"#;
+
+        let parsed = parse(json);
+
+        assert!(parsed.is_err());
+
+        match parsed {
+            Err(e) => {
+                assert_eq!(e.start.line, 1);
+                assert_eq!(e.start.col, 18);
+                assert_eq!(e.end.line, 1);
+                assert_eq!(e.end.col, 18);
+                assert_eq!(e.kind, Kind::TrailingComma)
+            }
+            Ok(_) => panic!("Not supposed to happen"),
+        }
+    }
 }
 
 mod object {