From 3b6180ad7d9541a2e15fb25040e9aac85df77774 Mon Sep 17 00:00:00 2001 From: elkowar <5300871+elkowar@users.noreply.github.com> Date: Fri, 16 Jul 2021 18:57:22 +0200 Subject: [PATCH] Better error handling --- examples/errors.rs | 12 ++- src/ast.rs | 23 ++++++ src/dynval.rs | 20 ++++- src/error.rs | 10 +-- src/eval.rs | 13 ++-- src/parser/simplexpr_parser.lalrpop | 114 ---------------------------- src/simplexpr_parser.lalrpop | 9 +-- 7 files changed, 66 insertions(+), 135 deletions(-) delete mode 100644 src/parser/simplexpr_parser.lalrpop diff --git a/examples/errors.rs b/examples/errors.rs index 9bd7053..0ecc8e3 100644 --- a/examples/errors.rs +++ b/examples/errors.rs @@ -1,11 +1,19 @@ +use std::collections::HashMap; + +use simplexpr::dynval::DynVal; + fn main() { let mut files = codespan_reporting::files::SimpleFiles::new(); - let input = "12 + \"hi\" * foo ) ? bar == baz : false"; + let input = "12 + foo * 2 < 2 ? bar == true : false"; let _ = files.add("foo.eww", input); let ast = simplexpr::parser::parse_string(input); - match ast { + + let mut vars = HashMap::new(); + vars.insert("foo".to_string(), "2".into()); + + match ast.and_then(|x| x.eval(&vars).map_err(|e| e.into())) { Ok(ast) => { println!("{:?}", ast); } diff --git a/src/ast.rs b/src/ast.rs index 96ab4bb..ab81e03 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -5,6 +5,20 @@ use serde::{Deserialize, Serialize}; #[derive(Eq, PartialEq, Clone, Copy, Serialize, Deserialize)] pub struct Span(pub usize, pub usize); +pub trait MaybeSpanned { + fn try_span(&self) -> Option; +} + +impl MaybeSpanned for Span { + fn try_span(&self) -> Option { + Some(*self) + } +} + +pub fn span_between(a: impl MaybeSpanned, b: impl MaybeSpanned) -> Option { + Some(Span(a.try_span()?.0, b.try_span()?.1)) +} + impl std::fmt::Display for Span { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}..{}", self.0, self.1) @@ -51,6 +65,11 @@ pub enum SimplExpr { JsonAccess(Span, Box, Box), FunctionCall(Span, String, Vec), } +impl MaybeSpanned for SimplExpr { + fn try_span(&self) -> Option { + Some(self.span()) + } +} impl std::fmt::Display for SimplExpr { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -68,6 +87,10 @@ impl std::fmt::Display for SimplExpr { } } impl SimplExpr { + pub fn literal(span: Span, s: String) -> Self { + Self::Literal(span, DynVal(s, Some(span))) + } + pub fn span(&self) -> Span { match self { SimplExpr::Literal(span, _) => *span, diff --git a/src/dynval.rs b/src/dynval.rs index f9352ec..5a38635 100644 --- a/src/dynval.rs +++ b/src/dynval.rs @@ -1,4 +1,4 @@ -use crate::ast::Span; +use crate::ast::{MaybeSpanned, Span}; use itertools::Itertools; use serde::{Deserialize, Serialize}; use std::{convert::TryFrom, fmt, iter::FromIterator}; @@ -6,7 +6,7 @@ use std::{convert::TryFrom, fmt, iter::FromIterator}; pub type Result = std::result::Result; #[derive(Debug, thiserror::Error)] -#[error("Type error: Failed to turn {value} into a {target_type}")] +#[error("Failed to turn {value} into a {target_type}")] pub struct ConversionError { value: DynVal, target_type: &'static str, @@ -17,11 +17,21 @@ impl ConversionError { fn new(value: DynVal, target_type: &'static str, source: Box) -> Self { ConversionError { value, target_type, source: Some(source) } } + + pub fn span(&self) -> Option { + self.value.1 + } } #[derive(Clone, Deserialize, Serialize, Default)] pub struct DynVal(pub String, pub Option); +impl MaybeSpanned for DynVal { + fn try_span(&self) -> Option { + self.1 + } +} + impl From for DynVal { fn from(s: String) -> Self { DynVal(s, None) @@ -30,7 +40,7 @@ impl From for DynVal { impl fmt::Display for DynVal { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.0) + write!(f, "\"{}\"", self.0) } } impl fmt::Debug for DynVal { @@ -106,6 +116,10 @@ impl From<&serde_json::Value> for DynVal { } impl DynVal { + pub fn at(self, span: Span) -> Self { + DynVal(self.0, Some(span)) + } + pub fn from_string(s: String) -> Self { DynVal(s, None) } diff --git a/src/error.rs b/src/error.rs index b794344..b6a50c1 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,16 +6,13 @@ pub type Result = std::result::Result; pub enum Error { #[error("Parse error: {source}")] ParseError { source: lalrpop_util::ParseError }, - #[error("Conversion error: {source}")] - ConversionError { - #[from] - source: dynval::ConversionError, - }, + #[error("Conversion error: {0}")] + ConversionError(#[from] dynval::ConversionError), #[error("At: {0}: {1}")] Spanned(Span, Box), #[error(transparent)] - Eval(crate::eval::EvalError), + Eval(#[from] crate::eval::EvalError), #[error(transparent)] Other(#[from] Box), @@ -31,6 +28,7 @@ impl Error { Self::ParseError { source } => get_parse_error_span(source), Self::Spanned(span, _) => Some(*span), Self::Eval(err) => err.span(), + Self::ConversionError(err) => err.span(), _ => None, } } diff --git a/src/eval.rs b/src/eval.rs index 66034c2..689818c 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -11,10 +11,10 @@ pub enum EvalError { #[error("Invalid regex: {0}")] InvalidRegex(#[from] regex::Error), - #[error("got unresolved variable {0}")] + #[error("got unresolved variable `{0}`")] UnresolvedVariable(VarName), - #[error("Conversion error: {0}")] + #[error("Type error: {0}")] ConversionError(#[from] ConversionError), #[error("Incorrect number of arguments given to function: {0}")] @@ -34,6 +34,7 @@ impl EvalError { pub fn span(&self) -> Option { match self { EvalError::Spanned(span, _) => Some(*span), + EvalError::ConversionError(err) => err.span(), _ => None, } } @@ -103,10 +104,11 @@ impl SimplExpr { } pub fn eval(self, values: &HashMap) -> Result { - match self { + let span = self.span(); + let value = match self { SimplExpr::Literal(_, x) => Ok(x), SimplExpr::VarRef(span, ref name) => { - values.get(name).cloned().ok_or_else(|| EvalError::UnresolvedVariable(name.to_string()).at(span)) + Ok(values.get(name).cloned().ok_or_else(|| EvalError::UnresolvedVariable(name.to_string()).at(span))?.at(span)) } SimplExpr::BinOp(_, a, op, b) => { let a = a.eval(values)?; @@ -167,7 +169,8 @@ impl SimplExpr { let args = args.into_iter().map(|a| a.eval(values)).collect::>()?; call_expr_function(&function_name, args).map_err(|e| e.at(span)) } - } + }; + Ok(value?.at(span)) } } diff --git a/src/parser/simplexpr_parser.lalrpop b/src/parser/simplexpr_parser.lalrpop deleted file mode 100644 index 46dc13a..0000000 --- a/src/parser/simplexpr_parser.lalrpop +++ /dev/null @@ -1,114 +0,0 @@ - -use crate::ast::{SimplExpr::{self, *}, Span, BinOp::*, UnaryOp::*}; -use crate::parser::lexer::{Token, LexicalError}; -use crate::parser::lalrpop_helpers::*; - - -grammar; - -extern { - type Location = usize; - type Error = LexicalError; - - enum Token { - "+" => Token::Plus, - "-" => Token::Minus, - "*" => Token::Times, - "/" => Token::Div, - "%" => Token::Mod, - "==" => Token::Equals, - "!=" => Token::NotEquals, - "&&" => Token::And, - "||" => Token::Or, - ">" => Token::GT, - "<" => Token::LT, - "?:" => Token::Elvis, - "=~" => Token::RegexMatch, - - "!" => Token::Not, - - "," => Token::Comma, - "?" => Token::Question, - ":" => Token::Colon, - "(" => Token::LPren, - ")" => Token::RPren, - "[" => Token::LBrack, - "]" => Token::RBrack, - "." => Token::Dot, - - "true" => Token::True, - "false" => Token::False, - - "identifier" => Token::Ident(), - "number" => Token::NumLit(), - "string" => Token::StrLit(), - - } -} - -Comma: Vec = { - ",")*> => match e { - None => v, - Some(e) => { - v.push(e); - v - } - } -}; - -pub Expr: SimplExpr = { - #[precedence(level="0")] - , - => VarRef(Span(l, r), ident.to_string()), - "(" ")", - - #[precedence(level="1")] #[assoc(side="right")] - "(" > ")" => FunctionCall(Span(l, r), ident, args), - "[" "]" => JsonAccess(Span(l, r), b(value), b(index)), - - "." => { - JsonAccess(Span(l, r), b(value), b(Literal(Span(lit_l, r), index.into()))) - }, - - #[precedence(level="2")] #[assoc(side="right")] - "!" => UnaryOp(Span(l, r), Not, b(e)), - - #[precedence(level="3")] #[assoc(side="left")] - "*" => BinOp(Span(l, r), b(le), Times, b(re)), - "/" => BinOp(Span(l, r), b(le), Div, b(re)), - "%" => BinOp(Span(l, r), b(le), Mod, b(re)), - - #[precedence(level="4")] #[assoc(side="left")] - "+" => BinOp(Span(l, r), b(le), Plus, b(re)), - "-" => BinOp(Span(l, r), b(le), Minus, b(re)), - - #[precedence(level="5")] #[assoc(side="left")] - "==" => BinOp(Span(l, r), b(le), Equals, b(re)), - "!=" => BinOp(Span(l, r), b(le), NotEquals, b(re)), - "<" => BinOp(Span(l, r), b(le), GT, b(re)), - ">" => BinOp(Span(l, r), b(le), LT, b(re)), - "=~" => BinOp(Span(l, r), b(le), RegexMatch, b(re)), - - #[precedence(level="6")] #[assoc(side="left")] - "&&" => BinOp(Span(l, r), b(le), And, b(re)), - "||" => BinOp(Span(l, r), b(le), Or, b(re)), - "?:" => BinOp(Span(l, r), b(le), Elvis, b(re)), - - #[precedence(level="7")] #[assoc(side="right")] - "?" ":" => { - IfElse(Span(l, r), b(cond), b(then), b(els)) - }, -}; - -ExprReset = ; - -Literal: SimplExpr = { - => Literal(Span(l, r), x.into()), - => Literal(Span(l, r), x.into()), - "true" => Literal(Span(l, r), "true".into()), - "false" => Literal(Span(l, r), "false".into()), -} - -StrLit: String = { - => x[1..x.len() - 1].to_owned(), -}; diff --git a/src/simplexpr_parser.lalrpop b/src/simplexpr_parser.lalrpop index 46dc13a..ea1ba6f 100644 --- a/src/simplexpr_parser.lalrpop +++ b/src/simplexpr_parser.lalrpop @@ -1,4 +1,3 @@ - use crate::ast::{SimplExpr::{self, *}, Span, BinOp::*, UnaryOp::*}; use crate::parser::lexer::{Token, LexicalError}; use crate::parser::lalrpop_helpers::*; @@ -103,10 +102,10 @@ pub Expr: SimplExpr = { ExprReset = ; Literal: SimplExpr = { - => Literal(Span(l, r), x.into()), - => Literal(Span(l, r), x.into()), - "true" => Literal(Span(l, r), "true".into()), - "false" => Literal(Span(l, r), "false".into()), + => SimplExpr::literal(Span(l, r), x), + => SimplExpr::literal(Span(l, r), x), + "true" => SimplExpr::literal(Span(l, r), "true".into()), + "false" => SimplExpr::literal(Span(l, r), "false".into()), } StrLit: String = {