Merge pull request #78 from wycats/improved-errors

Improve errors

src/cli.rs

@@ -8,10 +8,10 @@ use crate::context::Context;
 crate use crate::errors::ShellError;
 use crate::evaluate::Scope;
 crate use crate::format::{EntriesListView, GenericView};
+use crate::git::current_branch;
 use crate::object::Value;
 use crate::parser::{ParsedCommand, Pipeline};
 use crate::stream::empty_stream;
-use crate::git::current_branch;
 
 use log::debug;
 use rustyline::error::ReadlineError;
@@ -86,7 +86,7 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
             context.env.lock().unwrap().cwd().display().to_string(),
             match current_branch() {
                 Some(s) => format!("({})", s),
-                None => "".to_string()
+                None => "".to_string(),
             }
         ));
 
@@ -110,6 +110,18 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
                     .unwrap();
                 }
 
+                ShellError::TypeError(desc) => context
+                    .host
+                    .lock()
+                    .unwrap()
+                    .stdout(&format!("TypeError: {}", desc)),
+
+                ShellError::MissingProperty { subpath, .. } => context
+                    .host
+                    .lock()
+                    .unwrap()
+                    .stdout(&format!("Missing property {}", subpath)),
+
                 ShellError::String(s) => context.host.lock().unwrap().stdout(&format!("{:?}", s)),
             },
 

src/errors.rs

@@ -9,6 +9,8 @@ use serde_derive::{Deserialize, Serialize};
 #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Serialize, Deserialize)]
 pub enum ShellError {
     String(StringError),
+    TypeError(String),
+    MissingProperty { subpath: String, expr: String },
     Diagnostic(ShellDiagnostic, String),
 }
 
@@ -108,6 +110,8 @@ impl std::fmt::Display for ShellError {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         match self {
             ShellError::String(s) => write!(f, "{}", &s.title),
+            ShellError::TypeError { .. } => write!(f, "TypeError"),
+            ShellError::MissingProperty { .. } => write!(f, "MissingProperty"),
             ShellError::Diagnostic(_, _) => write!(f, "<diagnostic>"),
         }
     }

src/evaluate/evaluator.rs

@@ -1,17 +1,21 @@
+use crate::object::Primitive;
 use crate::parser::ast;
 use crate::prelude::*;
-use crate::object::Primitive;
 use derive_new::new;
+use indexmap::IndexMap;
 
 #[derive(new)]
 crate struct Scope {
     it: Value,
+    #[new(default)]
+    vars: IndexMap<String, Value>,
 }
 
 impl Scope {
     crate fn empty() -> Scope {
         Scope {
             it: Value::nothing(),
+            vars: IndexMap::new(),
         }
     }
 }
@@ -46,10 +50,11 @@ fn evaluate_reference(r: &ast::Variable, scope: &Scope) -> Result<Value, ShellEr
 
     match r {
         It => Ok(scope.it.copy()),
-        Other(s) => Err(ShellError::string(&format!(
-            "Unimplemented variable reference: {}",
-            s
-        ))),
+        Other(s) => Ok(scope
+            .vars
+            .get(s)
+            .map(|v| v.copy())
+            .unwrap_or_else(|| Value::nothing())),
     }
 }
 
@@ -59,9 +64,10 @@ fn evaluate_binary(binary: &ast::Binary, scope: &Scope) -> Result<Value, ShellEr
 
     match left.compare(binary.operator, &right) {
         Some(v) => Ok(Value::boolean(v)),
-        None => Err(ShellError::string(&format!(
-            "Unimplemented evaluate_binary:\n{:#?}",
-            binary
+        None => Err(ShellError::TypeError(format!(
+            "Can't compare {} and {}",
+            left.type_name(),
+            right.type_name()
         ))),
     }
 }
@@ -73,17 +79,18 @@ fn evaluate_block(block: &ast::Block, _scope: &Scope) -> Result<Value, ShellErro
 fn evaluate_path(path: &ast::Path, scope: &Scope) -> Result<Value, ShellError> {
     let head = path.head();
     let mut value = &evaluate_expr(head, scope)?;
+    let mut seen = vec![];
 
     for name in path.tail() {
         let next = value.get_data_by_key(&name);
+        seen.push(name);
 
         match next {
             None => {
-                return Err(ShellError::string(&format!(
-                    "No key {} found in {}",
-                    name,
-                    path.print(),
-                )))
+                return Err(ShellError::MissingProperty {
+                    expr: path.print(),
+                    subpath: itertools::join(seen, "."),
+                });
             }
             Some(v) => value = v,
         }

src/main.rs

@@ -63,7 +63,6 @@ fn main() -> Result<(), Box<dyn Error>> {
         None => {}
         Some(values) => {
             for item in values {
-                println!("filtering {:?}", item);
                 builder.filter_module(&format!("nu::{}", item), LevelFilter::Trace);
             }
         }

src/object/base.rs

@@ -60,6 +60,21 @@ impl Serialize for Primitive {
 }
 
 impl Primitive {
+    crate fn type_name(&self) -> String {
+        use Primitive::*;
+
+        match self {
+            Nothing => "nothing",
+            Int(_) => "int",
+            Float(_) => "float",
+            Bytes(_) => "bytes",
+            String(_) => "string",
+            Boolean(_) => "boolean",
+            Date(_) => "date",
+        }
+        .to_string()
+    }
+
     crate fn format(&self, field_name: Option<&str>) -> String {
         match self {
             Primitive::Nothing => format!("{}", Color::Black.bold().paint("-")),
@@ -142,6 +157,16 @@ pub enum Value {
 }
 
 impl Value {
+    crate fn type_name(&self) -> String {
+        match self {
+            Value::Primitive(p) => p.type_name(),
+            Value::Object(_) => format!("object"),
+            Value::List(_) => format!("list"),
+            Value::Block(_) => format!("block"),
+            Value::Error(_) => format!("error"),
+        }
+    }
+
     crate fn data_descriptors(&self) -> Vec<DataDescriptor> {
         match self {
             Value::Primitive(_) => vec![DataDescriptor::value_of()],

src/parser/ast.rs

@@ -206,6 +206,7 @@ pub enum Variable {
     Other(String),
 }
 
+#[cfg(test)]
 crate fn var(name: &str) -> Expression {
     match name {
         "it" => Expression::VariableReference(Variable::It),
@@ -235,6 +236,7 @@ impl Variable {
     }
 }
 
+#[cfg(test)]
 pub fn bare(s: &str) -> BarePath {
     BarePath {
         head: s.into(),
@@ -296,6 +298,7 @@ impl Unit {
     }
 }
 
+#[cfg(test)]
 pub fn unit(num: i64, unit: impl Into<Unit>) -> Expression {
     Expression::Leaf(Leaf::Unit(num, unit.into()))
 }
@@ -360,6 +363,7 @@ impl Binary {
     }
 }
 
+#[cfg(test)]
 crate fn binary(
     left: impl Into<Expression>,
     operator: impl Into<Operator>,
@@ -398,10 +402,12 @@ pub enum Flag {
     Longhand(String),
 }
 
+#[cfg(test)]
 crate fn flag(s: &str) -> Flag {
     Flag::Longhand(s.into())
 }
 
+#[cfg(test)]
 crate fn short(s: &str) -> Flag {
     Flag::Shorthand(s.into())
 }
@@ -428,6 +434,7 @@ pub struct ParsedCommand {
 }
 
 impl ParsedCommand {
+    #[allow(unused)]
     fn print(&self) -> String {
         let mut out = vec![];
 
@@ -472,6 +479,7 @@ impl Pipeline {
         Pipeline { commands }
     }
 
+    #[allow(unused)]
     crate fn print(&self) -> String {
         itertools::join(self.commands.iter().map(|i| i.print()), " | ")
     }