better array key errors

examples/serde.rs

@@ -1,5 +1,5 @@
 use miette::DiagnosticResult;
-use php_literal_parser::{from_str, ParseError};
+use php_literal_parser::from_str;
 use serde_derive::Deserialize;
 
 #[derive(Debug, Deserialize, PartialEq)]
@@ -9,7 +9,7 @@ struct Target {
 }
 
 fn main() -> DiagnosticResult<()> {
-    let target = from_str(r#"["foo" => true, "bars" => [1, 2, 3, 4,]]"#)?;
+    let target = from_str(r#"["foo" => true, "bars" => [1, 2, 3, 4]]"#)?;
 
     assert_eq!(
         Target {

src/error.rs

@@ -9,16 +9,20 @@ use std::num::ParseFloatError;
 use std::str::ParseBoolError;
 use thiserror::Error;
 
+/// Any error that occurred while trying to parse the php literal
 #[derive(Error, Debug, Clone, Diagnostic)]
 pub enum ParseError {
     #[error(transparent)]
     #[diagnostic(transparent)]
+    /// A token that wasn't expected was found while parsing
     UnexpectedToken(#[from] UnexpectedTokenError),
     #[error(transparent)]
     #[diagnostic(transparent)]
+    /// A malformed integer, float, boolean or string literal was found
     InvalidPrimitive(#[from] PrimitiveError),
     #[error("Array key not valid for this position")]
-    #[diagnostic(code(php_object_parser::invalid_array_key))]
+    #[diagnostic(transparent)]
+    /// An array key was found that is invalid for this position
     UnexpectedArrayKey(ArrayKeyError),
     #[error("Trailing characters after parsing")]
     #[diagnostic(code(php_object_parser::trailing))]
@@ -37,6 +41,7 @@ impl serde::de::Error for ParseError {
     }
 }
 
+/// A token that wasn't expected was found while parsing
 #[derive(Debug, Clone, Diagnostic)]
 #[diagnostic(code(php_object_parser::unexpected_token))]
 pub struct UnexpectedTokenError {
@@ -67,6 +72,7 @@ impl UnexpectedTokenError {
     }
 }
 
+/// List of expected tokens
 #[derive(Clone)]
 pub struct TokenList(Vec<Token>);
 
@@ -120,6 +126,7 @@ impl Display for UnexpectedTokenError {
 
 impl Error for UnexpectedTokenError {}
 
+/// A malformed integer, float, boolean or string literal was found
 #[derive(Debug, Clone, Error, Diagnostic)]
 #[diagnostic(code(php_object_parser::invalid_primitive))]
 #[error("{kind}")]
@@ -168,16 +175,37 @@ pub struct ArrayKeyError {
     src: String,
     #[snippet(src)]
     snip: SourceSpan,
-    #[highlight(snip, label("This is the highlight"))]
+    #[highlight(snip, label("{}", self.kind))]
     err_span: SourceSpan,
+    kind: ArrayKeyErrorKind,
+}
+
+#[derive(Debug, Clone)]
+pub enum ArrayKeyErrorKind {
+    IntegerExpected,
+    NonConsecutive,
+}
+
+impl Display for ArrayKeyErrorKind {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "{}",
+            match self {
+                ArrayKeyErrorKind::IntegerExpected => "Expected integer key",
+                ArrayKeyErrorKind::NonConsecutive => "Expected consecutive integer key",
+            }
+        )
+    }
 }
 
 impl ArrayKeyError {
-    pub fn new(source: &str, snip: Span, err_span: Span) -> Self {
+    pub fn new(kind: ArrayKeyErrorKind, source: &str, err_span: Span) -> Self {
         ArrayKeyError {
             src: source.into(),
-            snip: map_span(&snip),
+            snip: map_span(&(0..source.len())),
             err_span: map_span(&err_span),
+            kind,
         }
     }
 }

src/lib.rs

@@ -67,17 +67,8 @@ use std::fmt::{Display, Formatter};
 use std::hash::{Hash, Hasher};
 use std::ops::Index;
 
-#[derive(Debug, PartialEq, Clone)]
-pub enum Value {
-    Bool(bool),
-    Int(i64),
-    Float(f64),
-    String(String),
-    Array(HashMap<Key, Value>),
-    Null,
-}
-
-/// A php value, can be either a bool, int, float, string or array
+/// A php value, can be either a bool, int, float, string, an array or null
+///
 /// note that in php all arrays are associative and thus represented by a map in rust.
 ///
 /// You can convert a `Value` into a regular rust type by pattern matching or using the `into_` functions.
@@ -101,6 +92,16 @@ pub enum Value {
 /// assert!(value["not"]["found"].is_null());
 /// # }
 /// ```
+#[derive(Debug, PartialEq, Clone)]
+pub enum Value {
+    Bool(bool),
+    Int(i64),
+    Float(f64),
+    String(String),
+    Array(HashMap<Key, Value>),
+    Null,
+}
+
 impl Value {
     /// Check if the value is a bool
     pub fn is_bool(&self) -> bool {
@@ -310,6 +311,7 @@ impl Display for Value {
     }
 }
 
+/// A php array key, can be either an int or string
 #[derive(Debug, Eq, Clone)]
 pub enum Key {
     Int(i64),

src/serde_impl.rs

@@ -4,7 +4,7 @@ use serde::de::{
 };
 use serde::Deserialize;
 
-use crate::error::{ArrayKeyError, ExpectToken, ResultExt};
+use crate::error::{ArrayKeyError, ArrayKeyErrorKind, ExpectToken, ResultExt};
 use crate::lexer::{SpannedToken, Token};
 use crate::num::ParseIntError;
 use crate::parser::{ArraySyntax, Parser};
@@ -535,9 +535,14 @@ impl<'de, 'a> SeqAccess<'de> for ArrayWalker<'de, 'a> {
                 let key = self.de.parser.parse_array_key(token)?;
                 match key {
                     Key::Int(key) if key == self.next_int_key => Ok(()),
+                    Key::Int(_) => Err(ParseError::UnexpectedArrayKey(ArrayKeyError::new(
+                        ArrayKeyErrorKind::NonConsecutive,
+                        self.source(),
+                        span,
+                    ))),
                     _ => Err(ParseError::UnexpectedArrayKey(ArrayKeyError::new(
+                        ArrayKeyErrorKind::IntegerExpected,
                         self.source(),
-                        span.clone(),
                         span,
                     ))),
                 }?;