fix: allow_missing_commas: false was ignored for array elements

src/cst/mod.rs

@@ -4062,6 +4062,22 @@ value3: true
     }
   }
 
+  #[test]
+  fn missing_comma_between_array_elements() {
+    build_cst("[1 2]");
+
+    // but is strict when strict
+    let options = crate::ParseOptions {
+      allow_missing_commas: false,
+      ..Default::default()
+    };
+    assert_eq!(
+      CstRootNode::parse("[1 2]", &options).err().unwrap().to_string(),
+      "Expected comma on line 1 column 3"
+    );
+    CstRootNode::parse("[1, 2]", &options).unwrap();
+  }
+
   #[track_caller]
   fn build_cst(text: &str) -> CstRootNode {
     CstRootNode::parse(text, &crate::ParseOptions::default()).unwrap()

src/parse_to_ast.rs

@@ -414,13 +414,24 @@ fn parse_array<'a>(context: &mut Context<'a>) -> Result<Array<'a>, ParseError> {
     }
 
     // skip the comma
-    if let Some(Token::Comma) = context.scan()? {
-      let comma_range = context.create_range_from_last_token();
-      if let Some(Token::CloseBracket) = context.scan()?
-        && !context.allow_trailing_commas
-      {
-        return Err(context.create_error_for_range(comma_range, ParseErrorKind::TrailingCommasNotAllowed));
+    let after_value_end = context.last_token_end;
+    match context.scan()? {
+      Some(Token::Comma) => {
+        let comma_range = context.create_range_from_last_token();
+        if let Some(Token::CloseBracket) = context.scan()?
+          && !context.allow_trailing_commas
+        {
+          return Err(context.create_error_for_range(comma_range, ParseErrorKind::TrailingCommasNotAllowed));
+        }
+      }
+      Some(token) if !context.allow_missing_commas && token.is_value_start() => {
+        let range = Range {
+          start: after_value_end,
+          end: after_value_end,
+        };
+        return Err(context.create_error_for_range(range, ParseErrorKind::ExpectedComma));
       }
+      _ => {}
     }
   }
 
@@ -582,25 +593,25 @@ mod tests {
 
   #[track_caller]
   fn assert_has_strict_error(text: &str, message: &str) {
-    let result = parse_to_ast(
-      text,
-      &Default::default(),
-      &ParseOptions {
-        allow_comments: false,
-        allow_loose_object_property_names: false,
-        allow_trailing_commas: false,
-        allow_missing_commas: false,
-        allow_single_quoted_strings: false,
-        allow_hexadecimal_numbers: false,
-        allow_unary_plus_numbers: false,
-      },
-    );
+    let result = parse_to_ast(text, &Default::default(), &strict_options());
     match result {
       Ok(_) => panic!("Expected error, but did not find one."),
       Err(err) => assert_eq!(err.to_string(), message),
     }
   }
 
+  fn strict_options() -> ParseOptions {
+    ParseOptions {
+      allow_comments: false,
+      allow_loose_object_property_names: false,
+      allow_trailing_commas: false,
+      allow_missing_commas: false,
+      allow_single_quoted_strings: false,
+      allow_hexadecimal_numbers: false,
+      allow_unary_plus_numbers: false,
+    }
+  }
+
   #[test]
   fn it_should_not_include_tokens_by_default() {
     let result = parse_to_ast("{}", &Default::default(), &Default::default()).unwrap();
@@ -742,6 +753,67 @@ mod tests {
     }
   }
 
+  #[test]
+  fn missing_comma_between_array_elements() {
+    let text = "[1 2]";
+    let result = parse_to_ast(text, &Default::default(), &Default::default()).unwrap();
+    let value = result.value.unwrap();
+    let elements = &value.as_array().unwrap().elements;
+    assert_eq!(elements.len(), 2);
+
+    // but is strict when strict
+    assert_has_strict_error(text, "Expected comma on line 1 column 3");
+    assert_has_strict_error("[01]", "Expected comma on line 1 column 3");
+    assert_has_strict_error(r#"["a" "b"]"#, "Expected comma on line 1 column 5");
+    assert_has_strict_error("[true false]", "Expected comma on line 1 column 6");
+    assert_has_strict_error("[null null]", "Expected comma on line 1 column 6");
+    assert_has_strict_error("[[1] [2]]", "Expected comma on line 1 column 5");
+    assert_has_strict_error(r#"[{"a":1} {"b":2}]"#, "Expected comma on line 1 column 9");
+    assert_has_strict_error("[1 2", "Expected comma on line 1 column 3");
+
+    // these are not missing commas
+    assert_has_strict_error("[1", "Unterminated array on line 1 column 1");
+    assert_has_strict_error("[1 a ]", "Unexpected word on line 1 column 4");
+
+    for text in ["[]", "[ ]", "[1,2]", "[1 , 2]", "[[1],[2]]", r#"[{"a":1},{"b":2}]"#] {
+      parse_to_ast(text, &Default::default(), &strict_options()).unwrap();
+    }
+  }
+
+  #[test]
+  fn missing_comma_not_allowed_with_trailing_commas_allowed_in_array() {
+    let options = ParseOptions {
+      allow_missing_commas: false,
+      allow_trailing_commas: true,
+      ..Default::default()
+    };
+    for text in ["[]", "[1,]", "[1, 2,]", "[[1,],[2,],]"] {
+      parse_to_ast(text, &Default::default(), &options).unwrap();
+    }
+  }
+
+  #[test]
+  fn missing_comma_with_comment_between_array_elements() {
+    // when comments are allowed but missing commas are not,
+    // should still detect the missing comma after the comment is skipped
+    let result = parse_to_ast(
+      r#"[
+  1 // comment here
+  2
+]"#,
+      &Default::default(),
+      &ParseOptions {
+        allow_comments: true,
+        allow_missing_commas: false,
+        ..Default::default()
+      },
+    );
+    match result {
+      Ok(_) => panic!("Expected error, but did not find one."),
+      Err(err) => assert_eq!(err.to_string(), "Expected comma on line 2 column 4"),
+    }
+  }
+
   #[test]
   fn it_should_error_when_arrays_are_deeply_nested() {
     // Deeply nested arrays cause a stack overflow when recursion depth is not limited

src/parse_to_value.rs

@@ -343,24 +343,25 @@ mod tests {
 
   #[track_caller]
   fn assert_has_strict_error(text: &str, message: &str) {
-    let result = parse_to_value(
-      text,
-      &ParseOptions {
-        allow_comments: false,
-        allow_loose_object_property_names: false,
-        allow_trailing_commas: false,
-        allow_missing_commas: false,
-        allow_single_quoted_strings: false,
-        allow_hexadecimal_numbers: false,
-        allow_unary_plus_numbers: false,
-      },
-    );
+    let result = parse_to_value(text, &strict_options());
     match result {
       Ok(_) => panic!("Expected error, but did not find one."),
       Err(err) => assert_eq!(err.to_string(), message),
     }
   }
 
+  fn strict_options() -> ParseOptions {
+    ParseOptions {
+      allow_comments: false,
+      allow_loose_object_property_names: false,
+      allow_trailing_commas: false,
+      allow_missing_commas: false,
+      allow_single_quoted_strings: false,
+      allow_hexadecimal_numbers: false,
+      allow_unary_plus_numbers: false,
+    }
+  }
+
   #[test]
   fn it_should_error_when_has_multiple_values() {
     assert_has_error(
@@ -500,6 +501,67 @@ mod tests {
     }
   }
 
+  #[test]
+  fn missing_comma_between_array_elements() {
+    let text = "[1 2]";
+    let value = parse_to_value(text, &Default::default()).unwrap().unwrap();
+    assert_eq!(
+      value,
+      JsonValue::Array(vec![JsonValue::Number("1"), JsonValue::Number("2")].into())
+    );
+
+    // but is strict when strict
+    assert_has_strict_error(text, "Expected comma on line 1 column 3");
+    assert_has_strict_error("[01]", "Expected comma on line 1 column 3");
+    assert_has_strict_error(r#"["a" "b"]"#, "Expected comma on line 1 column 5");
+    assert_has_strict_error("[true false]", "Expected comma on line 1 column 6");
+    assert_has_strict_error("[null null]", "Expected comma on line 1 column 6");
+    assert_has_strict_error("[[1] [2]]", "Expected comma on line 1 column 5");
+    assert_has_strict_error(r#"[{"a":1} {"b":2}]"#, "Expected comma on line 1 column 9");
+    assert_has_strict_error("[1 2", "Expected comma on line 1 column 3");
+
+    // these are not missing commas
+    assert_has_strict_error("[1", "Unterminated array on line 1 column 3");
+    assert_has_strict_error("[1 a ]", "Unexpected word on line 1 column 4");
+
+    for text in ["[]", "[ ]", "[1,2]", "[1 , 2]", "[[1],[2]]", r#"[{"a":1},{"b":2}]"#] {
+      parse_to_value(text, &strict_options()).unwrap();
+    }
+  }
+
+  #[test]
+  fn missing_comma_not_allowed_with_trailing_commas_allowed_in_array() {
+    let options = ParseOptions {
+      allow_missing_commas: false,
+      allow_trailing_commas: true,
+      ..Default::default()
+    };
+    for text in ["[]", "[1,]", "[1, 2,]", "[[1,],[2,],]"] {
+      parse_to_value(text, &options).unwrap();
+    }
+  }
+
+  #[test]
+  fn missing_comma_with_comment_between_array_elements() {
+    // when comments are allowed but missing commas are not,
+    // should still detect the missing comma after the comment is skipped
+    let result = parse_to_value(
+      r#"[
+  1 // comment here
+  2
+]"#,
+      &ParseOptions {
+        allow_comments: true,
+        allow_missing_commas: false,
+        ..Default::default()
+      },
+    );
+    match result {
+      Ok(_) => panic!("Expected error, but did not find one."),
+      Err(err) => assert_eq!(err.to_string(), "Expected comma on line 2 column 4"),
+    }
+  }
+
   #[test]
   fn it_should_parse_unquoted_keys_with_hex_and_trailing_comma() {
     let text = r#"{

src/parser.rs

@@ -192,20 +192,30 @@ impl<'a> JsoncParser<'a> {
   /// After an array element, scans for the comma/close-bracket and
   /// returns the next token.
   pub fn scan_array_comma(&mut self) -> Result<Option<Token<'a>>, ParseError> {
-    let token = self.scan()?;
-    if matches!(&token, Some(Token::Comma)) {
-      let comma_range = Range::new(self.scanner.token_start(), self.scanner.token_end());
-      let next = self.scan()?;
-      if matches!(&next, Some(Token::CloseBracket)) && !self.allow_trailing_commas {
-        return Err(
+    debug_assert!(self.pending_token.is_none(), "the previous value must be consumed");
+    let after_value_end = self.scanner.token_end();
+    match self.scan()? {
+      Some(Token::Comma) => {
+        let comma_range = Range::new(self.scanner.token_start(), self.scanner.token_end());
+        let next = self.scan()?;
+        if matches!(&next, Some(Token::CloseBracket)) && !self.allow_trailing_commas {
+          return Err(
+            self
+              .scanner
+              .create_error_for_range(comma_range, ParseErrorKind::TrailingCommasNotAllowed),
+          );
+        }
+        Ok(next)
+      }
+      Some(token) if !self.allow_missing_commas && token.is_value_start() => {
+        let range = Range::new(after_value_end, after_value_end);
+        Err(
           self
             .scanner
-            .create_error_for_range(comma_range, ParseErrorKind::TrailingCommasNotAllowed),
-        );
+            .create_error_for_range(range, ParseErrorKind::ExpectedComma),
+        )
       }
-      Ok(next)
-    } else {
-      Ok(token)
+      token => Ok(token),
     }
   }
 

src/serde.rs

@@ -722,4 +722,84 @@ mod tests {
 
     assert_eq!(result, Config { value: 42 });
   }
+
+  #[test]
+  fn missing_comma_between_array_elements() {
+    let result = parse_to_serde_value::<SerdeValue>("[1 2]", &Default::default()).unwrap();
+    assert_eq!(result, serde_json::json!([1, 2]));
+
+    // but is strict when strict
+    assert_has_strict_error("[1 2]", "Expected comma on line 1 column 3");
+    assert_has_strict_error("[01]", "Expected comma on line 1 column 3");
+    assert_has_strict_error(r#"["a" "b"]"#, "Expected comma on line 1 column 5");
+    assert_has_strict_error("[true false]", "Expected comma on line 1 column 6");
+    assert_has_strict_error("[null null]", "Expected comma on line 1 column 6");
+    assert_has_strict_error("[[1] [2]]", "Expected comma on line 1 column 5");
+    assert_has_strict_error(r#"[{"a":1} {"b":2}]"#, "Expected comma on line 1 column 9");
+
+    for text in ["[]", "[1,2]", "[1 , 2]", "[[1],[2]]", r#"[{"a":1},{"b":2}]"#] {
+      parse_to_serde_value_strict::<SerdeValue>(text).unwrap();
+    }
+  }
+
+  #[test]
+  fn missing_comma_between_array_elements_when_draining() {
+    // a tuple stops reading before the end of the array, so the
+    // remaining elements are drained and should still be strict
+    assert_has_strict_drain_error::<(u32,)>("[1 2]", "Expected comma on line 1 column 3");
+    // the drained element here ends with a `}`
+    assert_has_strict_drain_error::<(u32, SerdeValue)>(r#"[1, {"a":2} 3]"#, "Expected comma on line 1 column 12");
+  }
+
+  #[test]
+  fn missing_comma_with_comment_between_array_elements() {
+    // when comments are allowed but missing commas are not,
+    // should still detect the missing comma after the comment is skipped
+    let result = parse_to_serde_value::<SerdeValue>(
+      r#"[
+  1 // comment here
+  2
+]"#,
+      &ParseOptions {
+        allow_comments: true,
+        allow_missing_commas: false,
+        ..Default::default()
+      },
+    );
+    match result {
+      Ok(_) => panic!("Expected error, but did not find one."),
+      Err(err) => assert_eq!(err.to_string(), "Expected comma on line 2 column 4"),
+    }
+  }
+
+  #[track_caller]
+  fn assert_has_strict_error(text: &str, message: &str) {
+    match parse_to_serde_value_strict::<SerdeValue>(text) {
+      Ok(_) => panic!("Expected error, but did not find one."),
+      Err(err) => assert_eq!(err.to_string(), message),
+    }
+  }
+
+  #[track_caller]
+  fn assert_has_strict_drain_error<T: ::serde::de::DeserializeOwned>(text: &str, message: &str) {
+    match parse_to_serde_value_strict::<T>(text) {
+      Ok(_) => panic!("Expected error, but did not find one."),
+      Err(err) => assert_eq!(err.to_string(), message),
+    }
+  }
+
+  fn parse_to_serde_value_strict<T: ::serde::de::DeserializeOwned>(text: &str) -> Result<T, ParseError> {
+    parse_to_serde_value(
+      text,
+      &ParseOptions {
+        allow_comments: false,
+        allow_loose_object_property_names: false,
+        allow_trailing_commas: false,
+        allow_missing_commas: false,
+        allow_single_quoted_strings: false,
+        allow_hexadecimal_numbers: false,
+        allow_unary_plus_numbers: false,
+      },
+    )
+  }
 }