From 2ba718ba72b8178aefa2c7fc0d5bb8d0195c9bdd Mon Sep 17 00:00:00 2001 From: eric-therond Date: Wed, 27 Sep 2023 18:41:30 +0200 Subject: [PATCH] improve errors location (#23) Signed-off-by: eric-therond --- src/interpreter.rs | 31 ++++++++++++++++++++----------- src/parser.rs | 6 +++--- src/scheduler.rs | 12 +++++++++--- src/value.rs | 2 +- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/interpreter.rs b/src/interpreter.rs index c043ee93..3f6a0f2f 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -346,7 +346,7 @@ impl<'source> Interpreter<'source> { match (&lhs_var, &rhs_var) { (Value::Undefined, Value::Undefined) => { - bail!("both operators are unsafe") + bail!(lhs.span().error("both operators are unsafe")) } (Value::Undefined, _) => (lhs_name, rhs_var), (_, Value::Undefined) => (rhs_name, lhs_var), @@ -395,7 +395,9 @@ impl<'source> Interpreter<'source> { // Allow variable overwritten inside a loop if self.lookup_local_var(name).is_some() && self.loop_var_values.get(rhs).is_none() { - bail!("redefinition for variable {}", name); + bail!(rhs + .span() + .error(&format!("redefinition for variable {}", name))); } (name, self.eval_expr(rhs)?) @@ -1027,7 +1029,9 @@ impl<'source> Interpreter<'source> { match schedule.order.get(query) { Some(ord) => ord.iter().map(|i| &query.stmts[*i as usize]).collect(), // TODO - _ => bail!("statements not scheduled in query {query:?}"), + _ => bail!(query + .span + .error("statements not scheduled in query {query:?}")), } } else { query.stmts.iter().collect() @@ -1194,7 +1198,7 @@ impl<'source> Interpreter<'source> { match self.functions.get(&path) { Some(r) => Ok(r), _ => { - bail!("function not found") + bail!(fcn.span().error("function not found")) } } } @@ -1644,7 +1648,7 @@ impl<'source> Interpreter<'source> { Self::make_or_get_value_mut(obj, paths) } Value::Undefined => Ok(obj), - _ => bail!("make: not an object {obj:?}"), + _ => bail!("internal error: make: not an object {obj:?}"), } } @@ -1674,7 +1678,7 @@ impl<'source> Interpreter<'source> { }; } } - _ => bail!("could not merge value"), + _ => bail!("internal error: could not merge value"), }; Ok(()) } @@ -1700,7 +1704,7 @@ impl<'source> Interpreter<'source> { comps.push(v.text()); expr = None; } - _ => bail!("not a simple ref"), + _ => bail!("internal error: not a simple ref"), } } if let Some(d) = document { @@ -1843,7 +1847,10 @@ impl<'source> Interpreter<'source> { let (refr, index) = match refr { Expr::RefBrack { refr, index, .. } => (refr.as_ref(), Some(index.as_ref())), Expr::Var(_) => (refr, None), - _ => bail!("invalid token {:?} with the default keyword", refr), + _ => bail!(refr.span().error(&format!( + "invalid token {:?} with the default keyword", + refr + ))), }; Parser::get_path_ref_components_into(refr, &mut path)?; @@ -2086,7 +2093,7 @@ impl<'source> Interpreter<'source> { Expr::True(_) | Expr::False(_) | Expr::Number(_) | Expr::String(_) ) { // OPA's behavior is ignoring the non-scalar index - bail!("index is not a scalar value"); + bail!(index.span().error("index is not a scalar value")); } let index = self.eval_expr(index)?; @@ -2105,10 +2112,12 @@ impl<'source> Interpreter<'source> { let old = i.as_ref().unwrap(); let new = index.as_ref().unwrap(); if old == new { - bail!("multiple default rules for the variable with the same index"); + bail!(refr.span().error("multiple default rules for the variable with the same index")); } } else { - bail!("conflict type with the default rules"); + bail!(refr + .span() + .error("conflict type with the default rules")); } } o.into_mut().push((rule, index)); diff --git a/src/parser.rs b/src/parser.rs index 8ebb1f86..8e127c37 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -99,7 +99,7 @@ impl<'source> Parser<'source> { } Expr::Var(v) => comps.push(v.clone()), Expr::String(s) => comps.push(s.clone()), - _ => bail!("not a simple ref"), + _ => bail!("internal error: not a simple ref"), } Ok(()) } @@ -252,7 +252,7 @@ impl<'source> Parser<'source> { _ => { // Not a comprehension. Restore state. *self = state; - bail!("internal - not a compr"); + bail!("internal error: not a compr"); } }; @@ -268,7 +268,7 @@ impl<'source> Parser<'source> { // No progress was made in parsing the query. // Restore state and try parsing as set, array or object. *self = state; - bail!("internal - not a compr"); + bail!("internal error: not a compr"); } Err(err) => Err(err), } diff --git a/src/scheduler.rs b/src/scheduler.rs index 31580741..0db11019 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -984,10 +984,16 @@ impl<'a> Analyzer<'a> { infos.push(StmtInfo { definitions }); } - if let SortResult::Order(ord) = schedule(&mut infos[..])? { - self.order.insert(query, ord); + let res = schedule(&mut infos[..]); + match res { + Ok(SortResult::Order(ord)) => { + self.order.insert(query, ord); + } + Err(err) => { + bail!(query.span.error(&err.to_string())) + } + _ => (), } - self.locals.insert(query, scope); Ok(()) diff --git a/src/value.rs b/src/value.rs index 3b2252d4..bfb21456 100644 --- a/src/value.rs +++ b/src/value.rs @@ -304,7 +304,7 @@ impl Value { Self::make_or_get_value_mut(self, paths) } Value::Undefined => Ok(self), - _ => bail!("make: not an selfect {self:?}"), + _ => bail!("internal error: make: not an selfect {self:?}"), } } }