error parsing for def, alias and let

crates/nu-cli/src/errors.rs

@@ -208,8 +208,9 @@ pub fn report_parsing_error(
                 let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?;
                 Diagnostic::error()
                     .with_message("Unknown state")
-                    .with_labels(vec![Label::primary(diag_file_id, diag_range)
-                        .with_message(format!("unknown state {}", name))])
+                    .with_labels(vec![
+                        Label::primary(diag_file_id, diag_range).with_message(format!("{}", name))
+                    ])
             }
             ParseError::NonUtf8(span) => {
                 let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?;

crates/nu-parser/src/parser.rs

@@ -26,6 +26,10 @@ fn garbage(span: Span) -> Expression {
     Expression::garbage(span)
 }
 
+fn garbage_statement(spans: &[Span]) -> Statement {
+    Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))]))
+}
+
 fn is_identifier_byte(b: u8) -> bool {
     b != b'.' && b != b'[' && b != b'(' && b != b'{'
 }
@@ -59,6 +63,22 @@ fn check_call(command: Span, sig: &Signature, call: &Call) -> Option<ParseError>
     }
 }
 
+fn check_name(working_set: &mut StateWorkingSet, spans: &[Span]) -> Option<ParseError> {
+    if spans[1..].len() < 2 {
+        Some(ParseError::UnknownState(
+            "missing definition name".into(),
+            span(spans),
+        ))
+    } else if working_set.get_span_contents(spans[2]) != b"=" {
+        Some(ParseError::UnknownState(
+            "missing equal sign in definition".into(),
+            spans[2],
+        ))
+    } else {
+        None
+    }
+}
+
 pub fn parse_external_call(
     working_set: &mut StateWorkingSet,
     spans: &[Span],
@@ -2325,6 +2345,13 @@ pub fn parse_def(
         error = error.or(err);
         working_set.exit_scope();
 
+        if error.is_some() {
+            return (
+                Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])),
+                error,
+            );
+        }
+
         let name = name_expr.as_string();
 
         let signature = sig.as_signature();
@@ -2364,23 +2391,15 @@ pub fn parse_def(
                 )
             }
             _ => (
-                Statement::Pipeline(Pipeline::from_vec(vec![Expression {
-                    expr: Expr::Garbage,
-                    span: span(spans),
-                    ty: Type::Unknown,
-                }])),
+                Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])),
                 error,
             ),
         }
     } else {
         (
-            Statement::Pipeline(Pipeline::from_vec(vec![Expression {
-                expr: Expr::Garbage,
-                span: span(spans),
-                ty: Type::Unknown,
-            }])),
+            garbage_statement(spans),
             Some(ParseError::UnknownState(
-                "internal error: definition unparseable".into(),
+                "definition unparseable. Expected structure: def <name> [] {}".into(),
                 span(spans),
             )),
         )
@@ -2394,6 +2413,13 @@ pub fn parse_alias(
     let name = working_set.get_span_contents(spans[0]);
 
     if name == b"alias" {
+        if let Some(err) = check_name(working_set, spans) {
+            return (
+                Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])),
+                Some(err),
+            );
+        }
+
         if let Some(decl_id) = working_set.find_decl(b"alias") {
             let (call, call_span, _) =
                 parse_internal_call(working_set, spans[0], &spans[1..], decl_id);
@@ -2430,11 +2456,7 @@ pub fn parse_alias(
     }
 
     (
-        Statement::Pipeline(Pipeline::from_vec(vec![Expression {
-            expr: Expr::Garbage,
-            span: span(spans),
-            ty: Type::Unknown,
-        }])),
+        garbage_statement(spans),
         Some(ParseError::UnknownState(
             "internal error: let statement unparseable".into(),
             span(spans),
@@ -2449,6 +2471,13 @@ pub fn parse_let(
     let name = working_set.get_span_contents(spans[0]);
 
     if name == b"let" {
+        if let Some(err) = check_name(working_set, spans) {
+            return (
+                Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])),
+                Some(err),
+            );
+        }
+
         if let Some(decl_id) = working_set.find_decl(b"let") {
             let (call, call_span, err) =
                 parse_internal_call(working_set, spans[0], &spans[1..], decl_id);
@@ -2474,11 +2503,7 @@ pub fn parse_let(
         }
     }
     (
-        Statement::Pipeline(Pipeline::from_vec(vec![Expression {
-            expr: Expr::Garbage,
-            span: span(spans),
-            ty: Type::Unknown,
-        }])),
+        garbage_statement(spans),
         Some(ParseError::UnknownState(
             "internal error: let statement unparseable".into(),
             span(spans),
@@ -2490,16 +2515,16 @@ pub fn parse_statement(
     working_set: &mut StateWorkingSet,
     spans: &[Span],
 ) -> (Statement, Option<ParseError>) {
-    // FIXME: improve errors by checking keyword first
-    if let (decl, None) = parse_def(working_set, spans) {
-        (decl, None)
-    } else if let (stmt, None) = parse_let(working_set, spans) {
-        (stmt, None)
-    } else if let (stmt, None) = parse_alias(working_set, spans) {
-        (stmt, None)
-    } else {
-        let (expr, err) = parse_expression(working_set, spans);
-        (Statement::Pipeline(Pipeline::from_vec(vec![expr])), err)
+    let name = working_set.get_span_contents(spans[0]);
+
+    match name {
+        b"def" => parse_def(working_set, spans),
+        b"let" => parse_let(working_set, spans),
+        b"alias" => parse_alias(working_set, spans),
+        _ => {
+            let (expr, err) = parse_expression(working_set, spans);
+            (Statement::Pipeline(Pipeline::from_vec(vec![expr])), err)
+        }
     }
 }
 
@@ -2508,13 +2533,10 @@ pub fn parse_block(
     lite_block: &LiteBlock,
     scoped: bool,
 ) -> (Block, Option<ParseError>) {
-    let mut error = None;
     if scoped {
         working_set.enter_scope();
     }
 
-    let mut block = Block::new();
-
     // Pre-declare any definition so that definitions
     // that share the same block can see each other
     for pipeline in &lite_block.block {
@@ -2523,25 +2545,35 @@ pub fn parse_block(
         }
     }
 
-    for pipeline in &lite_block.block {
-        if pipeline.commands.len() > 1 {
-            let mut output = vec![];
-            for command in &pipeline.commands {
-                let (expr, err) = parse_expression(working_set, &command.parts);
-                error = error.or(err);
+    let mut error = None;
 
-                output.push(expr);
+    let block: Block = lite_block
+        .block
+        .iter()
+        .map(|pipeline| {
+            if pipeline.commands.len() > 1 {
+                let output = pipeline
+                    .commands
+                    .iter()
+                    .map(|command| {
+                        let (expr, err) = parse_expression(working_set, &command.parts);
+                        error = err.map(|err| err);
+
+                        expr
+                    })
+                    .collect::<Vec<Expression>>();
+
+                Statement::Pipeline(Pipeline {
+                    expressions: output,
+                })
+            } else {
+                let (stmt, err) = parse_statement(working_set, &pipeline.commands[0].parts);
+                error = err.map(|err| err);
+
+                stmt
             }
-            block.stmts.push(Statement::Pipeline(Pipeline {
-                expressions: output,
-            }));
-        } else {
-            let (stmt, err) = parse_statement(working_set, &pipeline.commands[0].parts);
-            error = error.or(err);
-
-            block.stmts.push(stmt);
-        }
-    }
+        })
+        .into();
 
     if scoped {
         working_set.exit_scope();

crates/nu-protocol/src/ast/block.rs

@@ -48,3 +48,15 @@ impl Block {
         }
     }
 }
+
+impl<T> From<T> for Block
+where
+    T: Iterator<Item = Statement>,
+{
+    fn from(stmts: T) -> Self {
+        Self {
+            signature: Box::new(Signature::new("")),
+            stmts: stmts.collect(),
+        }
+    }
+}