From ebf9c7e7ef26becd250e58f38a3b6592f3aaf09a Mon Sep 17 00:00:00 2001
From: elkowar <5300871+elkowar@users.noreply.github.com>
Date: Sat, 31 Jul 2021 16:01:20 +0200
Subject: [PATCH] Unify error messages and remove unused code

---
 crates/simplexpr/src/eval.rs         | 20 ++------------------
 crates/simplexpr/src/lib.rs          |  1 +
 crates/yuck/src/format_diagnostic.rs |  5 ++---
 3 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/crates/simplexpr/src/eval.rs b/crates/simplexpr/src/eval.rs
index e703797..159d35b 100644
--- a/crates/simplexpr/src/eval.rs
+++ b/crates/simplexpr/src/eval.rs
@@ -15,10 +15,6 @@ pub enum EvalError {
     #[error("Invalid regex: {0}")]
     InvalidRegex(#[from] regex::Error),
 
-    // TODO unresolved and unknown are the same for the user,....
-    #[error("got unresolved variable `{0}`")]
-    UnresolvedVariable(VarName),
-
     #[error("Unknown variable {0}")]
     UnknownVariable(VarName),
 
@@ -62,18 +58,6 @@ impl Spanned for EvalError {
 }
 
 impl SimplExpr {
-    pub fn map_terminals_into(self, f: impl Fn(Self) -> Self) -> Self {
-        use SimplExpr::*;
-        match self {
-            BinOp(span, box a, op, box b) => BinOp(span, box f(a), op, box f(b)),
-            UnaryOp(span, op, box a) => UnaryOp(span, op, box f(a)),
-            IfElse(span, box a, box b, box c) => IfElse(span, box f(a), box f(b), box f(c)),
-            JsonAccess(span, box a, box b) => JsonAccess(span, box f(a), box f(b)),
-            FunctionCall(span, name, args) => FunctionCall(span, name, args.into_iter().map(f).collect()),
-            other => f(other),
-        }
-    }
-
     /// map over all of the variable references, replacing them with whatever expression the provided function returns.
     /// Returns [Err] when the provided function fails with an [Err]
     pub fn try_map_var_refs<E, F: Fn(Span, VarName) -> Result<SimplExpr, E> + Copy>(self, f: F) -> Result<Self, E> {
@@ -154,7 +138,7 @@ impl SimplExpr {
         match self.eval(&HashMap::new()) {
             Ok(x) => Ok(x),
             Err(x) => Err(x.map_in_span(|err| match err {
-                EvalError::UnknownVariable(name) | EvalError::UnresolvedVariable(name) => EvalError::NoVariablesAllowed(name),
+                EvalError::UnknownVariable(name) => EvalError::NoVariablesAllowed(name),
                 other => other,
             })),
         }
@@ -165,7 +149,7 @@ impl SimplExpr {
         let value = match self {
             SimplExpr::Literal(_, x) => Ok(x.clone()),
             SimplExpr::VarRef(span, ref name) => {
-                Ok(values.get(name).cloned().ok_or_else(|| EvalError::UnresolvedVariable(name.clone()).at(*span))?.at(*span))
+                Ok(values.get(name).cloned().ok_or_else(|| EvalError::UnknownVariable(name.clone()).at(*span))?.at(*span))
             }
             SimplExpr::BinOp(_, a, op, b) => {
                 let a = a.eval(values)?;
diff --git a/crates/simplexpr/src/lib.rs b/crates/simplexpr/src/lib.rs
index 23de86b..6aee794 100644
--- a/crates/simplexpr/src/lib.rs
+++ b/crates/simplexpr/src/lib.rs
@@ -1,4 +1,5 @@
 #![feature(box_patterns)]
+#![feature(format_args_capture)]
 #![feature(pattern)]
 #![feature(box_syntax)]
 #![feature(try_blocks)]
diff --git a/crates/yuck/src/format_diagnostic.rs b/crates/yuck/src/format_diagnostic.rs
index 3653082..e7dbee0 100644
--- a/crates/yuck/src/format_diagnostic.rs
+++ b/crates/yuck/src/format_diagnostic.rs
@@ -186,7 +186,6 @@ fn lalrpop_error_to_diagnostic<T: std::fmt::Display, E: Spanned + ToDiagnostic>(
 impl ToDiagnostic for simplexpr::error::Error {
     fn to_diagnostic(&self) -> Diagnostic<usize> {
         use simplexpr::error::Error::*;
-        dbg!(&self);
         match self {
             ParseError { source, file_id } => lalrpop_error_to_diagnostic(source, *file_id),
             ConversionError(error) => error.to_diagnostic(),
@@ -210,7 +209,7 @@ impl ToDiagnostic for simplexpr::eval::EvalError {
             NoVariablesAllowed(name) => gen_diagnostic!(self),
             // TODO the note here is confusing when it's an unknown variable being used _within_ a string literal / simplexpr
             // it only really makes sense on top-level symbols
-            UnresolvedVariable(name) | UnknownVariable(name) => gen_diagnostic! {
+            UnknownVariable(name) => gen_diagnostic! {
                 msg = self,
                 note = format!("If you meant to use the literal value \"{}\", surround the value in quotes", name)
             },
@@ -226,7 +225,7 @@ impl ToDiagnostic for dynval::ConversionError {
             msg = self,
             label = self.value.span() => format!("`{}` is not of type `{}`", self.value, self.target_type),
         };
-        diag.with_notes(self.source.as_ref().map(|x| vec![format!("{}", x)]).unwrap_or_default())
+        diag.with_notes(self.source.as_ref().map(|x| vec![x.to_string()]).unwrap_or_default())
     }
 }