export errors and reduce box usage (#67)

* Export Error to library, at the cost that the internal Pairs is now stringified

* get rid of Json parsing for boolean and put the PestError in a Box (especially to keep the results small in Ok(_) case)

* make `json_path_instance` work without PathInstance, which is a Box<dyn> type and thus needs an additinal vtable for each search

Author: xMAC94x

src/lib.rs

@@ -103,6 +103,7 @@
 use crate::parser::model::JsonPath;
 use crate::parser::parser::parse_json_path;
 use crate::path::json_path_instance;
+use parser::errors::JsonPathParserError;
 use serde_json::Value;
 use std::convert::TryInto;
 use std::fmt::Debug;
@@ -150,7 +151,7 @@ extern crate pest;
 /// #Note:
 /// the result is going to be cloned and therefore it can be significant for the huge queries
 pub trait JsonPathQuery {
-    fn path(self, query: &str) -> Result<Value, String>;
+    fn path(self, query: &str) -> Result<Value, JsonPathParserError>;
 }
 
 #[derive(Clone, Debug)]
@@ -159,7 +160,7 @@ pub struct JsonPathInst {
 }
 
 impl FromStr for JsonPathInst {
-    type Err = String;
+    type Err = JsonPathParserError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Ok(JsonPathInst {
@@ -170,6 +171,7 @@ impl FromStr for JsonPathInst {
 
 impl JsonPathInst {
     pub fn find_slice<'a>(&'a self, value: &'a Value) -> Vec<JsonPtr<'a, Value>> {
+        use crate::path::Path;
         json_path_instance(&self.inner, value)
             .find(JsonPathValue::from_root(value))
             .into_iter()
@@ -205,7 +207,7 @@ impl<'a> Deref for JsonPtr<'a, Value> {
 }
 
 impl JsonPathQuery for Value {
-    fn path(self, query: &str) -> Result<Value, String> {
+    fn path(self, query: &str) -> Result<Value, JsonPathParserError> {
         let p = JsonPathInst::from_str(query)?;
         Ok(find(&p, &self))
     }
@@ -419,6 +421,7 @@ impl<'a, Data> JsonPathValue<'a, Data> {
 /// );
 /// ```
 pub fn find_slice<'a>(path: &'a JsonPathInst, json: &'a Value) -> Vec<JsonPathValue<'a, Value>> {
+    use crate::path::Path;
     let instance = json_path_instance(&path.inner, json);
     let res = instance.find(JsonPathValue::from_root(json));
     let has_v: Vec<JsonPathValue<'_, Value>> = res.into_iter().filter(|v| v.has_value()).collect();

src/parser/errors.rs

@@ -1,23 +1,27 @@
-use pest::iterators::Pairs;
 use thiserror::Error;
 
 use super::parser::Rule;
 
 #[derive(Error, Debug)]
-#[allow(clippy::large_enum_variant)]
-pub enum JsonPathParserError<'a> {
+pub enum JsonPathParserError {
     #[error("Failed to parse rule: {0}")]
-    PestError(#[from] pest::error::Error<Rule>),
-    #[error("Failed to parse JSON: {0}")]
-    JsonParsingError(#[from] serde_json::Error),
-    #[error("{0}")]
-    ParserError(String),
-    #[error("Unexpected rule {0:?} when trying to parse logic atom: {1:?}")]
-    UnexpectedRuleLogicError(Rule, Pairs<'a, Rule>),
-    #[error("Unexpected `none` when trying to parse logic atom: {0:?}")]
-    UnexpectedNoneLogicError(Pairs<'a, Rule>),
-}
-
-pub fn parser_err(cause: &str) -> JsonPathParserError<'_> {
-    JsonPathParserError::ParserError(format!("Failed to parse JSONPath: {cause}"))
+    PestError(#[from] Box<pest::error::Error<Rule>>),
+    #[error("Unexpected rule {0:?} when trying to parse logic atom: {1} within {2}")]
+    UnexpectedRuleLogicError(Rule, String, String),
+    #[error("Unexpected `none` when trying to parse logic atom: {0} within {1}")]
+    UnexpectedNoneLogicError(String, String),
+    #[error("Pest returned successful parsing but did not produce any output, that should be unreachable due to .pest definition file: SOI ~ chain ~ EOI")]
+    UnexpectedPestOutput,
+    #[error("expected a `Rule::path` but found nothing")]
+    NoRulePath,
+    #[error("expected a `JsonPath::Descent` but found nothing")]
+    NoJsonPathDescent,
+    #[error("expected a `JsonPath::Field` but found nothing")]
+    NoJsonPathField,
+    #[error("expected a `f64` or `i64`, but got {0}")]
+    InvalidNumber(String),
+    #[error("Invalid toplevel rule for JsonPath: {0:?}")]
+    InvalidTopLevelRule(Rule),
+    #[error("Failed to get inner pairs for {0}")]
+    EmptyInner(String),
 }

src/parser/model.rs

@@ -2,6 +2,8 @@ use crate::parse_json_path;
 use serde_json::Value;
 use std::convert::TryFrom;
 
+use super::errors::JsonPathParserError;
+
 /// The basic structures for parsing json paths.
 /// The common logic of the structures pursues to correspond the internal parsing structure.
 #[derive(Debug, Clone)]
@@ -35,10 +37,15 @@ impl JsonPath {
 }
 
 impl TryFrom<&str> for JsonPath {
-    type Error = String;
+    type Error = JsonPathParserError;
 
+    /// Parses a string into a [JsonPath].
+    ///
+    /// # Errors
+    ///
+    /// Returns a variant of [JsonPathParserError] if the parsing operation failed.
     fn try_from(value: &str) -> Result<Self, Self::Error> {
-        parse_json_path(value).map_err(|e| e.to_string())
+        parse_json_path(value)
     }
 }
 

src/parser/parser.rs

@@ -1,7 +1,6 @@
 #![allow(clippy::empty_docs)]
 
-use crate::parser::errors::JsonPathParserError::ParserError;
-use crate::parser::errors::{parser_err, JsonPathParserError};
+use crate::parser::errors::JsonPathParserError;
 use crate::parser::model::FilterExpression::{And, Not, Or};
 use crate::parser::model::{
     FilterExpression, FilterSign, Function, JsonPath, JsonPathIndex, Operand,
@@ -20,9 +19,10 @@ struct JsonPathParser;
 ///
 /// Returns a variant of [JsonPathParserError] if the parsing operation failed.
 pub fn parse_json_path(jp_str: &str) -> Result<JsonPath, JsonPathParserError> {
-    JsonPathParser::parse(Rule::path, jp_str)?
+    JsonPathParser::parse(Rule::path, jp_str)
+        .map_err(Box::new)?
         .next()
-        .ok_or(parser_err(jp_str))
+        .ok_or(JsonPathParserError::UnexpectedPestOutput)
         .and_then(parse_internal)
 }
 
@@ -36,7 +36,7 @@ fn parse_internal(rule: Pair<Rule>) -> Result<JsonPath, JsonPathParserError> {
         Rule::path => rule
             .into_inner()
             .next()
-            .ok_or(parser_err("expected a Rule::path but found nothing"))
+            .ok_or(JsonPathParserError::NoRulePath)
             .and_then(parse_internal),
         Rule::current => rule
             .into_inner()
@@ -53,14 +53,14 @@ fn parse_internal(rule: Pair<Rule>) -> Result<JsonPath, JsonPathParserError> {
         Rule::wildcard => Ok(JsonPath::Wildcard),
         Rule::descent => parse_key(down(rule)?)?
             .map(JsonPath::Descent)
-            .ok_or(parser_err("expected a JsonPath::Descent but found nothing")),
+            .ok_or(JsonPathParserError::NoJsonPathDescent),
         Rule::descent_w => Ok(JsonPath::DescentW),
         Rule::function => Ok(JsonPath::Fn(Function::Length)),
         Rule::field => parse_key(down(rule)?)?
             .map(JsonPath::Field)
-            .ok_or(parser_err("expected a JsonPath::Field but found nothing")),
+            .ok_or(JsonPathParserError::NoJsonPathField),
         Rule::index => parse_index(rule).map(JsonPath::Index),
-        _ => Err(ParserError(format!("{rule} did not match any 'Rule' "))),
+        rule => Err(JsonPathParserError::InvalidTopLevelRule(rule)),
     }
 }
 
@@ -106,12 +106,17 @@ fn number_to_value(number: &str) -> Result<Value, JsonPathParserError> {
         .or_else(|| number.parse::<f64>().ok().map(Value::from))
     {
         Some(value) => Ok(value),
-        None => Err(JsonPathParserError::ParserError(format!(
-            "Failed to parse {number} as either f64 or i64"
-        ))),
+        None => Err(JsonPathParserError::InvalidNumber(number.to_string())),
     }
 }
 
+fn bool_to_value(boolean: &str) -> Value {
+    boolean
+        .parse::<bool>()
+        .map(Value::from)
+        .expect("unreachable: according to .pest this is either `true` or `false`")
+}
+
 fn parse_unit_indexes(pairs: Pairs<Rule>) -> Result<JsonPathIndex, JsonPathParserError> {
     let mut keys = vec![];
 
@@ -152,34 +157,40 @@ fn parse_filter_index(pair: Pair<Rule>) -> Result<JsonPathIndex, JsonPathParserE
 
 fn parse_logic_or(pairs: Pairs<Rule>) -> Result<FilterExpression, JsonPathParserError> {
     let mut expr: Option<FilterExpression> = None;
-    let error_message = format!("Failed to parse logical expression: {:?}", pairs);
-    for pair in pairs {
+    // only possible for the loop not to produce any value (except Errors)
+    if pairs.len() == 0 {
+        return Err(JsonPathParserError::UnexpectedNoneLogicError(
+            pairs.get_input().to_string(),
+            pairs.as_str().to_string(),
+        ));
+    }
+    for pair in pairs.into_iter() {
         let next_expr = parse_logic_and(pair.into_inner())?;
         match expr {
             None => expr = Some(next_expr),
             Some(e) => expr = Some(Or(Box::new(e), Box::new(next_expr))),
         }
     }
-    match expr {
-        Some(expr) => Ok(expr),
-        None => Err(JsonPathParserError::ParserError(error_message)),
-    }
+    Ok(expr.expect("unreachable: above len() == 0 check should have catched this"))
 }
 
 fn parse_logic_and(pairs: Pairs<Rule>) -> Result<FilterExpression, JsonPathParserError> {
     let mut expr: Option<FilterExpression> = None;
-    let error_message = format!("Failed to parse logical `and` expression: {:?}", pairs,);
+    // only possible for the loop not to produce any value (except Errors)
+    if pairs.len() == 0 {
+        return Err(JsonPathParserError::UnexpectedNoneLogicError(
+            pairs.get_input().to_string(),
+            pairs.as_str().to_string(),
+        ));
+    }
     for pair in pairs {
         let next_expr = parse_logic_not(pair.into_inner())?;
         match expr {
             None => expr = Some(next_expr),
             Some(e) => expr = Some(And(Box::new(e), Box::new(next_expr))),
         }
     }
-    match expr {
-        Some(expr) => Ok(expr),
-        None => Err(JsonPathParserError::ParserError(error_message)),
-    }
+    Ok(expr.expect("unreachable: above len() == 0 check should have catched this"))
 }
 
 fn parse_logic_not(mut pairs: Pairs<Rule>) -> Result<FilterExpression, JsonPathParserError> {
@@ -191,10 +202,13 @@ fn parse_logic_not(mut pairs: Pairs<Rule>) -> Result<FilterExpression, JsonPathP
                     .map(|expr|Not(Box::new(expr)))
             },
             Rule::logic_atom => parse_logic_atom(pairs.next().expect("unreachable in arithmetic: should have a value as pairs.peek() was Some(_)").into_inner()),
-            x => Err(JsonPathParserError::UnexpectedRuleLogicError(x, pairs)),
+            rule => Err(JsonPathParserError::UnexpectedRuleLogicError(rule, pairs.get_input().to_string(), pairs.as_str().to_string())),
         }
     } else {
-        Err(JsonPathParserError::UnexpectedNoneLogicError(pairs))
+        Err(JsonPathParserError::UnexpectedNoneLogicError(
+            pairs.get_input().to_string(),
+            pairs.as_str().to_string(),
+        ))
     }
 }
 
@@ -213,10 +227,13 @@ fn parse_logic_atom(mut pairs: Pairs<Rule>) -> Result<FilterExpression, JsonPath
                     Ok(FilterExpression::Atom(left, sign, right))
                 }
             }
-            x => Err(JsonPathParserError::UnexpectedRuleLogicError(x, pairs)),
+            rule => Err(JsonPathParserError::UnexpectedRuleLogicError(rule, pairs.get_input().to_string(), pairs.as_str().to_string())),
         }
     } else {
-        Err(JsonPathParserError::UnexpectedNoneLogicError(pairs))
+        Err(JsonPathParserError::UnexpectedNoneLogicError(
+            pairs.get_input().to_string(),
+            pairs.as_str().to_string(),
+        ))
     }
 }
 
@@ -226,7 +243,7 @@ fn parse_atom(rule: Pair<Rule>) -> Result<Operand, JsonPathParserError> {
         Rule::number => Operand::Static(number_to_value(rule.as_str())?),
         Rule::string_qt => Operand::Static(Value::from(down(atom)?.as_str())),
         Rule::chain => parse_chain_in_operand(down(rule)?)?,
-        Rule::boolean => Operand::Static(rule.as_str().parse::<Value>()?),
+        Rule::boolean => Operand::Static(bool_to_value(rule.as_str())),
         _ => Operand::Static(Value::Null),
     };
     Ok(parsed_atom)
@@ -246,10 +263,10 @@ fn parse_index(rule: Pair<Rule>) -> Result<JsonPathIndex, JsonPathParserError> {
 }
 
 fn down(rule: Pair<Rule>) -> Result<Pair<Rule>, JsonPathParserError> {
-    let error_message = format!("Failed to get inner pairs for {:?}", rule);
+    let error_message = rule.to_string();
     match rule.into_inner().next() {
-        Some(rule) => Ok(rule.to_owned()),
-        None => Err(ParserError(error_message)),
+        Some(rule) => Ok(rule),
+        None => Err(JsonPathParserError::EmptyInner(error_message)),
     }
 }
 

src/path/index.rs

@@ -7,6 +7,8 @@ use crate::JsonPathValue::{NoValue, Slice};
 use serde_json::value::Value::Array;
 use serde_json::Value;
 
+use super::TopPaths;
+
 /// process the slice like [start:end:step]
 #[derive(Debug)]
 pub(crate) struct ArraySlice {
@@ -127,7 +129,7 @@ impl<'a> Current<'a> {
     pub(crate) fn from(jp: &'a JsonPath, root: &'a Value) -> Self {
         match jp {
             JsonPath::Empty => Current::none(),
-            tail => Current::new(json_path_instance(tail, root)),
+            tail => Current::new(Box::new(json_path_instance(tail, root))),
         }
     }
     pub(crate) fn new(tail: PathInstance<'a>) -> Self {
@@ -151,30 +153,32 @@ impl<'a> Path<'a> for Current<'a> {
 
 /// the list of indexes like [1,2,3]
 pub(crate) struct UnionIndex<'a> {
-    indexes: Vec<PathInstance<'a>>,
+    indexes: Vec<TopPaths<'a>>,
 }
 
 impl<'a> UnionIndex<'a> {
     pub fn from_indexes(elems: &'a [Value]) -> Self {
-        let mut indexes: Vec<PathInstance<'a>> = vec![];
+        let mut indexes: Vec<TopPaths<'a>> = vec![];
 
         for idx in elems.iter() {
-            indexes.push(Box::new(ArrayIndex::new(idx.as_u64().unwrap() as usize)))
+            indexes.push(TopPaths::ArrayIndex(ArrayIndex::new(
+                idx.as_u64().unwrap() as usize
+            )))
         }
 
         UnionIndex::new(indexes)
     }
     pub fn from_keys(elems: &'a [String]) -> Self {
-        let mut indexes: Vec<PathInstance<'a>> = vec![];
+        let mut indexes: Vec<TopPaths<'a>> = vec![];
 
         for key in elems.iter() {
-            indexes.push(Box::new(ObjectField::new(key)))
+            indexes.push(TopPaths::ObjectField(ObjectField::new(key)))
         }
 
         UnionIndex::new(indexes)
     }
 
-    pub fn new(indexes: Vec<PathInstance<'a>>) -> Self {
+    pub fn new(indexes: Vec<TopPaths<'a>>) -> Self {
         UnionIndex { indexes }
     }
 }
@@ -582,8 +586,8 @@ mod tests {
         let chain = chain!(path!($), path!("key"), index);
         let path_inst = json_path_instance(&chain, &json);
         let expected_res = jp_v![
-            &exp4;"$.['key'][0]", 
-            &exp3;"$.['key'][2]", 
+            &exp4;"$.['key'][0]",
+            &exp3;"$.['key'][2]",
             &exp4;"$.['key'][4]"];
         assert_eq!(
             path_inst.find(JsonPathValue::from_root(&json)),