Improve keyword parsing, including for (#747)

* Improve keyword parsing, including for

* touchup

crates/nu-parser/src/errors.rs

@@ -61,10 +61,20 @@ pub enum ParseError {
     #[diagnostic(
         code(nu::parser::unexpected_keyword),
         url(docsrs),
-        help("'export' keyword is allowed only in a module.")
+        help("'{0}' keyword is allowed only in a module.")
     )]
     UnexpectedKeyword(String, #[label("unexpected {0}")] Span),
 
+    #[error("Statement used in pipeline.")]
+    #[diagnostic(
+        code(nu::parser::unexpected_keyword),
+        url(docsrs),
+        help(
+            "'{0}' keyword is not allowed in pipeline. Use '{0}' by itself, outside of a pipeline."
+        )
+    )]
+    StatementInPipeline(String, #[label("not allowed in pipeline")] Span),
+
     #[error("Incorrect value")]
     #[diagnostic(code(nu::parser::incorrect_value), url(docsrs), help("{2}"))]
     IncorrectValue(String, #[label("unexpected {0}")] Span, String),
@@ -203,14 +213,6 @@ pub enum ParseError {
     #[diagnostic(code(nu::parser::file_not_found), url(docsrs))]
     FileNotFound(String, #[label("File not found: {0}")] Span),
 
-    #[error("'let' statements can't be part of a pipeline")]
-    #[diagnostic(
-        code(nu::parser::let_not_statement),
-        url(docsrs),
-        help("use parens to assign to a variable\neg) let x = ('hello' | str length)")
-    )]
-    LetNotStatement(#[label = "let statement part of a pipeline"] Span),
-
     #[error("{0}")]
     #[diagnostic()]
     LabeledError(String, String, #[label("{1}")] Span),

crates/nu-parser/src/parse_keywords.rs

@@ -60,12 +60,12 @@ pub fn parse_def_predecl(working_set: &mut StateWorkingSet, spans: &[Span]) -> O
 pub fn parse_for(
     working_set: &mut StateWorkingSet,
     spans: &[Span],
-) -> (Statement, Option<ParseError>) {
+) -> (Expression, Option<ParseError>) {
     // Checking that the function is used with the correct name
     // Maybe this is not necessary but it is a sanity check
     if working_set.get_span_contents(spans[0]) != b"for" {
         return (
-            garbage_statement(spans),
+            garbage(spans[0]),
             Some(ParseError::UnknownState(
                 "internal error: Wrong call name for 'for' function".into(),
                 span(spans),
@@ -79,7 +79,7 @@ pub fn parse_for(
     let (call, call_span) = match working_set.find_decl(b"for") {
         None => {
             return (
-                garbage_statement(spans),
+                garbage(spans[0]),
                 Some(ParseError::UnknownState(
                     "internal error: def declaration not found".into(),
                     span(spans),
@@ -117,12 +117,12 @@ pub fn parse_for(
             err = check_call(call_span, &sig, &call).or(err);
             if err.is_some() || call.has_flag("help") {
                 return (
-                    Statement::Pipeline(Pipeline::from_vec(vec![Expression {
+                    Expression {
                         expr: Expr::Call(call),
                         span: call_span,
                         ty: Type::Unknown,
                         custom_completion: None,
-                    }])),
+                    },
                     err,
                 );
             }
@@ -162,12 +162,12 @@ pub fn parse_for(
     }
 
     (
-        Statement::Pipeline(Pipeline::from_vec(vec![Expression {
+        Expression {
             expr: Expr::Call(call),
             span: call_span,
             ty: Type::Unknown,
             custom_completion: None,
-        }])),
+        },
         error,
     )
 }

crates/nu-parser/src/parser.rs

@@ -488,6 +488,15 @@ pub fn parse_multispan_value(
 
             (arg, error)
         }
+        SyntaxShape::MathExpression => {
+            trace!("parsing: math expression");
+
+            let (arg, err) = parse_math_expression(working_set, &spans[*spans_idx..], None);
+            error = error.or(err);
+            *spans_idx = spans.len() - 1;
+
+            (arg, error)
+        }
         SyntaxShape::Expression => {
             trace!("parsing: expression");
 
@@ -3337,7 +3346,49 @@ pub fn parse_expression(
     let (output, err) = if is_math_expression_byte(bytes[0]) {
         parse_math_expression(working_set, &spans[pos..], None)
     } else {
-        parse_call(working_set, &spans[pos..], expand_aliases, spans[0])
+        // For now, check for special parses of certain keywords
+        match bytes {
+            b"def" => (
+                parse_call(working_set, &spans[pos..], expand_aliases, spans[0]).0,
+                Some(ParseError::StatementInPipeline("def".into(), spans[0])),
+            ),
+            b"let" => (
+                parse_call(working_set, &spans[pos..], expand_aliases, spans[0]).0,
+                Some(ParseError::StatementInPipeline("let".into(), spans[0])),
+            ),
+            b"alias" => (
+                parse_call(working_set, &spans[pos..], expand_aliases, spans[0]).0,
+                Some(ParseError::StatementInPipeline("alias".into(), spans[0])),
+            ),
+            b"module" => (
+                parse_call(working_set, &spans[pos..], expand_aliases, spans[0]).0,
+                Some(ParseError::StatementInPipeline("module".into(), spans[0])),
+            ),
+            b"use" => (
+                parse_call(working_set, &spans[pos..], expand_aliases, spans[0]).0,
+                Some(ParseError::StatementInPipeline("use".into(), spans[0])),
+            ),
+            b"source" => (
+                parse_call(working_set, &spans[pos..], expand_aliases, spans[0]).0,
+                Some(ParseError::StatementInPipeline("source".into(), spans[0])),
+            ),
+            b"export" => (
+                parse_call(working_set, &spans[pos..], expand_aliases, spans[0]).0,
+                Some(ParseError::UnexpectedKeyword("export".into(), spans[0])),
+            ),
+            b"hide" => (
+                parse_call(working_set, &spans[pos..], expand_aliases, spans[0]).0,
+                Some(ParseError::StatementInPipeline("hide".into(), spans[0])),
+            ),
+            #[cfg(feature = "plugin")]
+            b"register" => (
+                parse_call(working_set, &spans[pos..], expand_aliases, spans[0]).0,
+                Some(ParseError::StatementInPipeline("plugin".into(), spans[0])),
+            ),
+
+            b"for" => parse_for(working_set, spans),
+            _ => parse_call(working_set, &spans[pos..], expand_aliases, spans[0]),
+        }
     };
 
     let with_env = working_set.find_decl(b"with-env");
@@ -3425,7 +3476,10 @@ pub fn parse_statement(
     match name {
         b"def" => parse_def(working_set, spans),
         b"let" => parse_let(working_set, spans),
-        b"for" => parse_for(working_set, spans),
+        b"for" => {
+            let (expr, err) = parse_for(working_set, spans);
+            (Statement::Pipeline(Pipeline::from_vec(vec![expr])), err)
+        }
         b"alias" => parse_alias(working_set, spans),
         b"module" => parse_module(working_set, spans),
         b"use" => parse_use(working_set, spans),
@@ -3563,16 +3617,6 @@ pub fn parse_block(
                     })
                     .collect::<Vec<Expression>>();
 
-                if let Some(let_call_id) = working_set.find_decl(b"let") {
-                    for expr in output.iter() {
-                        if let Expr::Call(x) = &expr.expr {
-                            if let_call_id == x.decl_id && output.len() != 1 && error.is_none() {
-                                error = Some(ParseError::LetNotStatement(expr.span));
-                            }
-                        }
-                    }
-                }
-
                 for expr in output.iter_mut().skip(1) {
                     if expr.has_in_variable(working_set) {
                         *expr = wrap_expr_with_collect(working_set, expr);

src/tests/test_engine.rs

@@ -143,3 +143,8 @@ fn proper_variable_captures_with_nesting() -> TestResult {
         "102",
     )
 }
+
+#[test]
+fn proper_variable_for() -> TestResult {
+    run_test(r#"for x in 1..3 { if $x == 2 { "bob" } } | get 1"#, "bob")
+}

src/tests/test_parser.rs

@@ -116,7 +116,7 @@ fn long_flag() -> TestResult {
 
 #[test]
 fn let_not_statement() -> TestResult {
-    fail_test(r#"let x = "hello" | str length"#, "can't")
+    fail_test(r#"let x = "hello" | str length"#, "used in pipeline")
 }
 
 #[test]