Feature: Better error reporting.

* Common Error Type with line and column reporting.
* Removed some todos.
This commit is contained in:
Jeremy Wall 2017-12-09 10:02:45 -06:00
parent 3acf8e4679
commit 7a0a194fb9
6 changed files with 271 additions and 134 deletions

View File

@ -11,7 +11,6 @@ license = "Apache-2.0"
[dependencies]
nom = "^3.2"
nom_locate = "^0.1.1"
quick-error = "^1.2.0"
clap = "~2.26.0"
[lib]

View File

@ -28,6 +28,7 @@ use tokenizer::Span;
use ast::*;
use format;
use parse::parse;
use error;
impl MacroDef {
pub fn eval(&self,
@ -36,8 +37,9 @@ impl MacroDef {
// Error conditions. If the args don't match the length and types of the argdefs then this is
// macro call error.
if args.len() > self.argdefs.len() {
return Err(Box::new(BuildError::BadArgLen("Macro called with too many args"
.to_string())));
return Err(Box::new(error::Error::new("Macro called with too many args",
error::ErrorType::BadArgLen,
self.pos.clone())));
}
// If the args don't match the types required by the expressions then that is a TypeFail.
// If the expressions reference Symbols not defined in the MacroDef that is also an error.
@ -59,44 +61,6 @@ impl MacroDef {
}
}
quick_error! {
#[derive(Debug,PartialEq)]
pub enum BuildError {
TypeFail(msg: String) {
description("Type Error")
display("Type Error {}", msg)
}
DuplicateBinding(msg: String) {
description("Atttempt to add duplicate binding in file")
display("Atttempt to add duplicate binding in file {}", msg)
}
IncompleteParse(msg: String) {
description("Incomplete Parse of file")
display("Incomplete Parse of file {}", msg)
}
Unsupported(msg: String) {
description("Unsupported Operation")
display("Unsupported Operation {}", msg)
}
NoSuchSymbol(msg: String) {
description("Eval Error")
display("No Such Variable {}", msg)
}
BadArgLen(msg: String) {
description("Eval Error")
display("Bad Argument Length {}", msg)
}
FormatError(msg: String) {
description("String Format Error")
display("String format Error {}", msg)
}
TODO(msg: String) {
description("TODO Error")
display("TODO Error {}", msg)
}
}
}
/// BuildResult is the result of a build.
type BuildResult = Result<(), Box<Error>>;
@ -213,15 +177,15 @@ pub struct Builder {
}
macro_rules! eval_binary_expr {
($case:pat, $rside:ident, $result:expr, $msg:expr) => {
($case:pat, $pos:ident, $rside:ident, $result:expr, $msg:expr) => {
match $rside.as_ref() {
$case => {
return Ok(Rc::new($result))
return Ok(Rc::new($result));
},
val => {
return Err(Box::new(
BuildError::TypeFail(
format!("Expected {} but got {}", $msg, val))))
error::Error::new(
format!("Expected {} but got {}", $msg, val), error::ErrorType::TypeFail, $pos.clone())));
}
}
}
@ -253,7 +217,9 @@ impl Builder {
&Value::String(ref s) => Ok(Rc::new(Val::String(s.val.to_string()))),
&Value::Symbol(ref s) => {
self.lookup_sym(&(s.into()))
.ok_or(Box::new(BuildError::NoSuchSymbol(format!("Unable to find {}", s.val))))
.ok_or(Box::new(error::Error::new(format!("Unable to find {}", s.val),
error::ErrorType::NoSuchSymbol,
v.pos().clone())))
}
&Value::List(ref def) => self.list_to_val(def),
&Value::Tuple(ref tuple) => self.tuple_to_val(&tuple.val),
@ -306,9 +272,11 @@ impl Builder {
}
nom::IResult::Error(err) => Err(Box::new(err)),
nom::IResult::Incomplete(_) => {
Err(Box::new(BuildError::IncompleteParse(format!("Could not parse input from \
file: {}",
name))))
Err(Box::new(error::Error::new(
format!("Could not parse input from file: {}", name),
error::ErrorType::IncompleteParsing,
Position{line: 0, column: 0}
)))
}
}
}
@ -345,10 +313,12 @@ impl Builder {
let name = &def.name;
match self.out.entry(name.into()) {
Entry::Occupied(e) => {
return Err(Box::new(BuildError::DuplicateBinding(format!("Let binding \
return Err(Box::new(error::Error::new(format!("Let binding \
for {:?} already \
exists",
e.key()))));
e.key()),
error::ErrorType::DuplicateBinding,
def.name.pos.clone())));
}
Entry::Vacant(e) => {
e.insert(val);
@ -403,10 +373,12 @@ impl Builder {
stack.push_back(vv.clone());
} else {
// TODO(jwall): A better error for this would be nice.
return Err(Box::new(BuildError::NoSuchSymbol(format!("Unable to \
return Err(Box::new(error::Error::new(format!("Unable to \
match selector \
path {:?}",
sl))));
sl),
error::ErrorType::NoSuchSymbol,
next.pos.clone())));
}
Ok(())
}
@ -423,10 +395,12 @@ impl Builder {
stack.push_back(elems[idx].clone());
} else {
// TODO(jwall): A better error for this would be nice.
return Err(Box::new(BuildError::NoSuchSymbol(format!("Unable to \
return Err(Box::new(error::Error::new(format!("Unable to \
match selector \
path {:?}",
sl))));
sl),
error::ErrorType::NoSuchSymbol,
next.pos.clone())));
}
Ok(())
}
@ -457,29 +431,42 @@ impl Builder {
continue;
}
_ => {
return Err(Box::new(BuildError::TypeFail(format!("{} is not a Tuple or List",
sl[0].fragment))));
return Err(Box::new(error::Error::new(format!("{} is not a Tuple or List",
sl[0].fragment),
error::ErrorType::TypeFail, next.pos.clone())));
}
}
}
}
return Err(Box::new(BuildError::NoSuchSymbol(format!("Unable to find Symbol {}",
sl[0].fragment))));
return Err(Box::new(error::Error::new(format!("Unable to find Symbol {}",
sl[0].fragment),
error::ErrorType::NoSuchSymbol,
pos_sl.pos.clone())));
}
return Err(Box::new(BuildError::NoSuchSymbol("Attempted to lookup an empty selector"
.to_string())));
return Err(Box::new(error::Error::new("Attempted to lookup an empty selector",
error::ErrorType::NoSuchSymbol,
Position {
line: 0,
column: 0,
})));
}
fn add_vals(&self, left: Rc<Val>, right: Rc<Val>) -> Result<Rc<Val>, Box<Error>> {
fn add_vals(&self,
pos: &Position,
left: Rc<Val>,
right: Rc<Val>)
-> Result<Rc<Val>, Box<Error>> {
match *left {
Val::Int(i) => {
eval_binary_expr!(&Val::Int(ii),
pos,
right,
Val::Int(i + ii),
"Integer")
}
Val::Float(f) => {
eval_binary_expr!(&Val::Float(ff),
pos,
right,
Val::Float(f + ff),
"Float")
@ -490,11 +477,13 @@ impl Builder {
return Ok(Rc::new(Val::String([s.to_string(), ss.clone()].concat())))
}
val => {
return Err(Box::new(BuildError::TypeFail(format!("Expected \
return Err(Box::new(error::Error::new(format!("Expected \
String \
but got \
{:?}",
val))))
val),
error::ErrorType::TypeFail,
pos.clone())))
}
}
}
@ -507,84 +496,112 @@ impl Builder {
return Ok(Rc::new(Val::List(new_vec)));
}
val => {
return Err(Box::new(BuildError::TypeFail(format!("Expected \
return Err(Box::new(error::Error::new(format!("Expected \
List \
but got \
{:?}",
val))))
val),
error::ErrorType::TypeFail,
pos.clone())))
}
}
}
ref expr => {
return Err(Box::new(
BuildError::Unsupported(
format!("{} does not support the '+' operation", expr.type_name()))))
error::Error::new(
format!("{} does not support the '+' operation", expr.type_name()),
error::ErrorType::Unsupported,
pos.clone())))
}
}
}
fn subtract_vals(&self, left: Rc<Val>, right: Rc<Val>) -> Result<Rc<Val>, Box<Error>> {
fn subtract_vals(&self,
pos: &Position,
left: Rc<Val>,
right: Rc<Val>)
-> Result<Rc<Val>, Box<Error>> {
match *left {
Val::Int(i) => {
eval_binary_expr!(&Val::Int(ii),
pos,
right,
Val::Int(i - ii),
"Integer")
}
Val::Float(f) => {
eval_binary_expr!(&Val::Float(ff),
pos,
right,
Val::Float(f - ff),
"Float")
}
ref expr => {
return Err(Box::new(
BuildError::Unsupported(
format!("{} does not support the '-' operation", expr.type_name()))))
error::Error::new(
format!("{} does not support the '-' operation", expr.type_name()),
error::ErrorType::Unsupported,
pos.clone())))
}
}
}
fn multiply_vals(&self, left: Rc<Val>, right: Rc<Val>) -> Result<Rc<Val>, Box<Error>> {
fn multiply_vals(&self,
pos: &Position,
left: Rc<Val>,
right: Rc<Val>)
-> Result<Rc<Val>, Box<Error>> {
match *left {
Val::Int(i) => {
eval_binary_expr!(&Val::Int(ii),
pos,
right,
Val::Int(i * ii),
"Integer")
}
Val::Float(f) => {
eval_binary_expr!(&Val::Float(ff),
pos,
right,
Val::Float(f * ff),
"Float")
}
ref expr => {
return Err(Box::new(
BuildError::Unsupported(
format!("{} does not support the '*' operation", expr.type_name()))))
error::Error::new(
format!("{} does not support the '*' operation", expr.type_name()),
error::ErrorType::Unsupported,
pos.clone())))
}
}
}
fn divide_vals(&self, left: Rc<Val>, right: Rc<Val>) -> Result<Rc<Val>, Box<Error>> {
fn divide_vals(&self,
pos: &Position,
left: Rc<Val>,
right: Rc<Val>)
-> Result<Rc<Val>, Box<Error>> {
match *left {
Val::Int(i) => {
eval_binary_expr!(&Val::Int(ii),
pos,
right,
Val::Int(i / ii),
"Integer")
}
Val::Float(f) => {
eval_binary_expr!(&Val::Float(ff),
pos,
right,
Val::Float(f / ff),
"Float")
}
ref expr => {
return Err(Box::new(
BuildError::Unsupported(
format!("{} does not support the '*' operation", expr.type_name()))))
error::Error::new(
format!("{} does not support the '*' operation", expr.type_name()),
error::ErrorType::Unsupported,
pos.clone())))
}
}
}
@ -596,10 +613,10 @@ impl Builder {
let right = try!(self.eval_expr(expr));
let left = try!(self.value_to_val(v));
match kind {
&BinaryExprType::Add => self.add_vals(left, right),
&BinaryExprType::Sub => self.subtract_vals(left, right),
&BinaryExprType::Mul => self.multiply_vals(left, right),
&BinaryExprType::Div => self.divide_vals(left, right),
&BinaryExprType::Add => self.add_vals(&def.pos, left, right),
&BinaryExprType::Sub => self.subtract_vals(&def.pos, left, right),
&BinaryExprType::Mul => self.multiply_vals(&def.pos, left, right),
&BinaryExprType::Div => self.divide_vals(&def.pos, left, right),
}
}
@ -612,10 +629,12 @@ impl Builder {
if let Entry::Vacant(v) = m.entry(key.clone()) {
v.insert(val.clone());
} else {
return Err(Box::new(BuildError::TypeFail(format!("Duplicate \
return Err(Box::new(error::Error::new(format!("Duplicate \
field: {} in \
tuple",
key.val))));
key.val),
error::ErrorType::TypeFail,
def.pos.clone())));
}
}
for &(ref key, ref val) in def.fields.iter() {
@ -631,9 +650,11 @@ impl Builder {
v.insert(expr_result);
} else {
return Err(Box::new(
BuildError::TypeFail(
error::Error::new(
format!("Expected type {} for field {} but got {}",
src_val.type_name(), key.fragment, expr_result.type_name()))));
src_val.type_name(), key.fragment, expr_result.type_name()),
error::ErrorType::TypeFail,
key.pos.clone())));
}
}
};
@ -644,7 +665,9 @@ impl Builder {
new_fields.sort_by(|a, b| a.0.cmp(&b.0));
return Ok(Rc::new(Val::Tuple(new_fields)));
}
Err(Box::new(BuildError::TypeFail(format!("Expected Tuple got {}", v))))
Err(Box::new(error::Error::new(format!("Expected Tuple got {}", v),
error::ErrorType::TypeFail,
def.selector.pos.clone())))
}
fn eval_format(&self, def: &FormatDef) -> Result<Rc<Val>, Box<Error>> {
@ -656,7 +679,7 @@ impl Builder {
vals.push(rcv.deref().clone());
}
let formatter = format::Formatter::new(tmpl.clone(), vals);
Ok(Rc::new(Val::String(try!(formatter.render()))))
Ok(Rc::new(Val::String(try!(formatter.render(&def.pos)))))
}
fn eval_call(&self, def: &CallDef) -> Result<Rc<Val>, Box<Error>> {
@ -672,17 +695,21 @@ impl Builder {
let fields = try!(m.eval(argvals));
return Ok(Rc::new(Val::Tuple(fields)));
}
Err(Box::new(BuildError::TypeFail(// We should pretty print the selectors here.
format!("{} is not a Macro", v))))
Err(Box::new(error::Error::new(// We should pretty print the selectors here.
format!("{} is not a Macro", v),
error::ErrorType::TypeFail,
def.pos.clone())))
}
fn eval_macro_def(&self, def: &MacroDef) -> Result<Rc<Val>, Box<Error>> {
match def.validate_symbols() {
Ok(()) => Ok(Rc::new(Val::Macro(def.clone()))),
Err(set) => {
Err(Box::new(BuildError::NoSuchSymbol(format!("Macro has the following \
Err(Box::new(error::Error::new(format!("Macro has the following \
undefined symbols: {:?}",
set))))
set),
error::ErrorType::NoSuchSymbol,
def.pos.clone())))
}
}
}
@ -705,9 +732,11 @@ impl Builder {
// Otherwise return the default
return self.eval_expr(def_expr);
} else {
return Err(Box::new(BuildError::TypeFail(format!("Expected String but got \
return Err(Box::new(error::Error::new(format!("Expected String but got \
{} in Select expression",
v.type_name()))));
v.type_name()),
error::ErrorType::TypeFail,
def.pos.clone())));
}
}

86
src/error.rs Normal file
View File

@ -0,0 +1,86 @@
use std::error;
use std::fmt;
use ast::*;
pub enum ErrorType {
// Build Errors
TypeFail,
DuplicateBinding,
IncompleteParsing,
Unsupported,
NoSuchSymbol,
BadArgLen,
FormatError,
// Parsing Errors
UnexpectedToken,
EmptyExpression,
}
impl fmt::Display for ErrorType {
fn fmt(&self, w: &mut fmt::Formatter) -> fmt::Result {
let name = match self {
&ErrorType::TypeFail => "TypeFail",
&ErrorType::DuplicateBinding => "DuplicateBinding",
&ErrorType::IncompleteParsing => "IncompleteParsing",
&ErrorType::Unsupported => "Unsupported",
&ErrorType::NoSuchSymbol => "NoSuchSymbol",
&ErrorType::BadArgLen => "BadArgLen",
&ErrorType::FormatError => "FormatError",
&ErrorType::UnexpectedToken => "UnexpectedToken",
&ErrorType::EmptyExpression => "EmptyExpression",
};
w.write_str(name)
}
}
pub struct Error {
pub err_type: ErrorType,
pub pos: Position,
pub msg: String,
_pkgonly: (),
}
impl Error {
pub fn new<S: Into<String>>(msg: S, t: ErrorType, pos: Position) -> Self {
Error {
err_type: t,
pos: pos,
msg: msg.into(),
_pkgonly: (),
}
}
}
impl fmt::Debug for Error {
fn fmt(&self, w: &mut fmt::Formatter) -> fmt::Result {
write!(
w,
"{}: \"{}\" {}:{}",
self.err_type,
self.msg,
self.pos.line,
self.pos.column
)
}
}
impl fmt::Display for Error {
fn fmt(&self, w: &mut fmt::Formatter) -> fmt::Result {
write!(
w,
"{}: \"{}\" {}:{}",
self.err_type,
self.msg,
self.pos.line,
self.pos.column
)
}
}
impl error::Error for Error {
fn description(&self) -> &str {
&self.msg
}
}

View File

@ -15,7 +15,8 @@
use std::clone::Clone;
use std::error::Error;
use build::BuildError;
use ast::*;
use error;
pub struct Formatter<V: Into<String> + Clone> {
tmpl: String,
@ -30,16 +31,17 @@ impl<V: Into<String> + Clone> Formatter<V> {
}
}
pub fn render(&self) -> Result<String, Box<Error>> {
pub fn render(&self, pos: &Position) -> Result<String, Box<Error>> {
let mut buf = String::new();
let mut should_escape = false;
let mut count = 0;
for c in self.tmpl.chars() {
if c == '@' && !should_escape {
if count == self.args.len() {
return Err(Box::new(BuildError::FormatError("Too few arguments to string \
formatter."
.to_string())));
return Err(Box::new(error::Error::new("Too few arguments to string \
formatter.",
error::ErrorType::FormatError,
pos.clone())));
}
let arg = self.args[count].clone();
let strval = arg.into();
@ -52,9 +54,10 @@ impl<V: Into<String> + Clone> Formatter<V> {
}
}
if self.args.len() != count {
return Err(Box::new(BuildError::FormatError("Too many arguments to string \
formatter."
.to_string())));
return Err(Box::new(error::Error::new("Too many arguments to string \
formatter.",
error::ErrorType::FormatError,
pos.clone())));
}
return Ok(buf);
}
@ -63,22 +66,35 @@ impl<V: Into<String> + Clone> Formatter<V> {
#[cfg(test)]
mod test {
use super::Formatter;
use ast::Position;
#[test]
fn test_format_happy_path() {
let formatter = Formatter::new("foo @ @ \\@", vec!["bar", "quux"]);
assert_eq!(formatter.render().unwrap(), "foo bar quux @");
let pos = Position {
line: 0,
column: 0,
};
assert_eq!(formatter.render(&pos).unwrap(), "foo bar quux @");
}
#[test]
fn test_format_happy_wrong_too_few_args() {
let formatter = Formatter::new("foo @ @ \\@", vec!["bar"]);
assert!(formatter.render().is_err());
let pos = Position {
line: 0,
column: 0,
};
assert!(formatter.render(&pos).is_err());
}
#[test]
fn test_format_happy_wrong_too_many_args() {
let formatter = Formatter::new("foo @ @ \\@", vec!["bar", "quux", "baz"]);
assert!(formatter.render().is_err());
let pos = Position {
line: 0,
column: 0,
};
assert!(formatter.render(&pos).is_err());
}
}

View File

@ -16,15 +16,14 @@ extern crate nom;
#[macro_use]
extern crate nom_locate;
#[macro_use]
extern crate quick_error;
#[macro_use]
pub mod ast;
pub mod tokenizer;
pub mod parse;
pub mod build;
pub mod convert;
pub mod error;
mod format;
pub use ast::Value;

View File

@ -20,23 +20,7 @@ use nom::InputLength;
use ast::*;
use tokenizer::*;
quick_error! {
#[derive(Debug,PartialEq)]
pub enum ParseError {
UnexpectedToken(expected: String, actual: String) {
description("Unexpected Token")
display("Unexpected Token Expected {} Got {}", expected, actual)
}
EmptyExpression(msg: String ) {
description("EmptyExpression")
display("Unexpected EmptyExpression {}", msg)
}
}
}
// TODO(jwall): Error Reporting with Line and Column information.
use error as E;
type ParseResult<O> = Result<O, Box<Error>>;
@ -171,7 +155,6 @@ pub fn selector_or_symbol(input: Span) -> IResult<Span, Value> {
return IResult::Incomplete(i);
}
IResult::Error(_) => {
// TODO(jwall): Maybe be smarter about the error reporting here?
return ws!(input, selector_value);
}
IResult::Done(rest, val) => {
@ -275,16 +258,27 @@ named!(grouped_expression( Span ) -> Expression,
)
);
fn assert_nonempty_list<T>(v: Vec<T>) -> ParseResult<Vec<T>> {
if v.is_empty() {
return Err(Box::new(ParseError::EmptyExpression("Selectors can't be empty.".to_string())));
// TODO(jwall): This can go away once the non_empty_separated_list in nom is fixed to work
// with nom_locate.
fn assert_nonempty_list<T>(t: (Span, Vec<T>)) -> ParseResult<Vec<T>> {
if t.1.is_empty() {
return Err(Box::new(E::Error::new("Selectors can't be empty.",
E::ErrorType::EmptyExpression,
Position {
line: t.0.line as usize,
column: t.0.offset as usize,
})));
}
return Ok(v);
return Ok(t.1);
}
named!(selector_list( Span ) -> SelectorList,
map_res!(
separated_list!(dottok, barewordtok),
do_parse!(
pos: position!() >>
list: separated_list!(dottok, barewordtok) >>
(pos, list)
),
assert_nonempty_list
)
);
@ -330,7 +324,12 @@ fn tuple_to_macro(mut t: (Span, Vec<Value>, Value)) -> ParseResult<Expression> {
}
// TODO(jwall): Show a better version of the unexpected parsed value.
val => {
Err(Box::new(ParseError::UnexpectedToken("{ .. }".to_string(), format!("{:?}", val))))
Err(Box::new(E::Error::new(format!("Expected Tuple Got {:?}", val),
E::ErrorType::UnexpectedToken,
Position {
line: t.0.line as usize,
column: t.0.offset as usize,
})))
}
}
}
@ -363,9 +362,13 @@ fn tuple_to_select(t: (Span, Expression, Expression, Value)) -> ParseResult<Expr
pos: Position::new(t.0.line as usize, t.0.offset as usize),
}))
}
// TODO(jwall): Show a better version of the unexpected parsed value.
val => {
Err(Box::new(ParseError::UnexpectedToken("{ .. }".to_string(), format!("{:?}", val))))
Err(Box::new(E::Error::new(format!("Expected Tuple Got {:?}", val),
E::ErrorType::UnexpectedToken,
Position {
line: t.0.line as usize,
column: t.0.offset as usize,
})))
}
}
}
@ -414,7 +417,12 @@ fn tuple_to_call(t: (Span, Value, Vec<Expression>)) -> ParseResult<Expression> {
pos: Position::new(t.0.line as usize, t.0.offset as usize),
}))
} else {
Err(Box::new(ParseError::UnexpectedToken("Selector".to_string(), format!("{:?}", t.0))))
Err(Box::new(E::Error::new(format!("Expected Selector Got {:?}", t.0),
E::ErrorType::UnexpectedToken,
Position {
line: t.0.line as usize,
column: t.0.offset as usize,
})))
}
}