From 7aa1d8ac2af76696f4b0d08a5ad0fde02adfc8d3 Mon Sep 17 00:00:00 2001
From: Fernando Herrera <fernando.j.herrera@gmail.com>
Date: Mon, 13 Sep 2021 20:59:11 +0100
Subject: [PATCH] error check on def and alias

---
 crates/nu-cli/src/errors.rs    |  8 ++++
 crates/nu-parser/src/errors.rs |  1 +
 crates/nu-parser/src/parser.rs | 67 ++++++++++++++++++++++------------
 3 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/crates/nu-cli/src/errors.rs b/crates/nu-cli/src/errors.rs
index b5470ead13..a7ee992168 100644
--- a/crates/nu-cli/src/errors.rs
+++ b/crates/nu-cli/src/errors.rs
@@ -261,6 +261,14 @@ pub fn report_parsing_error(
                     .with_labels(vec![Label::primary(diag_file_id, diag_range)
                         .with_message("needs a parameter name")])
             }
+            ParseError::AssignmentMismatch(msg, label, span) => {
+                let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?;
+                Diagnostic::error()
+                    .with_message(msg)
+                    .with_labels(vec![
+                        Label::primary(diag_file_id, diag_range).with_message(label)
+                    ])
+            }
         };
 
     // println!("DIAG");
diff --git a/crates/nu-parser/src/errors.rs b/crates/nu-parser/src/errors.rs
index f4c486e0fd..2965d5f149 100644
--- a/crates/nu-parser/src/errors.rs
+++ b/crates/nu-parser/src/errors.rs
@@ -30,4 +30,5 @@ pub enum ParseError {
     RestNeedsName(Span),
     ExtraColumns(usize, Span),
     MissingColumns(usize, Span),
+    AssignmentMismatch(String, String, Span),
 }
diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs
index 1c44c6bbad..1d24bea002 100644
--- a/crates/nu-parser/src/parser.rs
+++ b/crates/nu-parser/src/parser.rs
@@ -63,16 +63,35 @@ 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),
-        ))
+fn check_name<'a>(
+    working_set: &mut StateWorkingSet,
+    spans: &'a [Span],
+) -> Option<(&'a Span, ParseError)> {
+    if spans.len() == 1 {
+        None
+    } else if spans.len() < 4 {
+        if working_set.get_span_contents(spans[1]) == b"=" {
+            let name = String::from_utf8_lossy(working_set.get_span_contents(spans[0]));
+            Some((
+                &spans[1],
+                ParseError::AssignmentMismatch(
+                    format!("{} missing name", name),
+                    "missing name".into(),
+                    spans[1],
+                ),
+            ))
+        } else {
+            None
+        }
     } else if working_set.get_span_contents(spans[2]) != b"=" {
-        Some(ParseError::UnknownState(
-            "missing equal sign in definition".into(),
-            span(spans),
+        let name = String::from_utf8_lossy(working_set.get_span_contents(spans[0]));
+        Some((
+            &spans[2],
+            ParseError::AssignmentMismatch(
+                format!("{} missing sign", name),
+                "missing equal sign".into(),
+                spans[2],
+            ),
         ))
     } else {
         None
@@ -2490,7 +2509,7 @@ pub fn parse_def(
         (
             garbage_statement(spans),
             Some(ParseError::UnknownState(
-                "definition unparseable. Expected structure: def <name> [] {}".into(),
+                "Expected structure: def <name> [] {}".into(),
                 span(spans),
             )),
         )
@@ -2504,9 +2523,9 @@ 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) {
+        if let Some((span, err)) = check_name(working_set, spans) {
             return (
-                Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])),
+                Statement::Pipeline(Pipeline::from_vec(vec![garbage(*span)])),
                 Some(err),
             );
         }
@@ -2562,9 +2581,9 @@ 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) {
+        if let Some((span, err)) = check_name(working_set, spans) {
             return (
-                Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])),
+                Statement::Pipeline(Pipeline::from_vec(vec![garbage(*span)])),
                 Some(err),
             );
         }
@@ -2606,16 +2625,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)
+        }
     }
 }