Require let to be a statement (#594)

crates/nu-command/src/system/run_external.rs

@@ -1,7 +1,6 @@
 use std::collections::HashMap;
 use std::env;
 use std::io::{BufRead, BufReader, Write};
-use std::path::Path;
 use std::process::{Command as CommandSys, Stdio};
 use std::sync::atomic::Ordering;
 use std::sync::mpsc;
@@ -48,7 +47,7 @@ impl Command for External {
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        let mut name: Spanned<String> = call.req(engine_state, stack, 0)?;
+        let name: Spanned<String> = call.req(engine_state, stack, 0)?;
         let args: Vec<Value> = call.rest(engine_state, stack, 1)?;
         let last_expression = call.has_flag("last_expression");
 
@@ -56,34 +55,6 @@ impl Command for External {
         let config = stack.get_config().unwrap_or_default();
         let env_vars_str = env_to_strings(engine_state, stack, &config)?;
 
-        // Check if this is a single call to a directory, if so auto-cd
-        let path = nu_path::expand_path(&name.item);
-        let orig = name.item.clone();
-        name.item = path.to_string_lossy().to_string();
-
-        let path = Path::new(&name.item);
-        if (orig.starts_with('.')
-            || orig.starts_with('~')
-            || orig.starts_with('/')
-            || orig.starts_with('\\'))
-            && path.is_dir()
-            && args.is_empty()
-        {
-            // We have an auto-cd
-            let _ = std::env::set_current_dir(&path);
-
-            //FIXME: this only changes the current scope, but instead this environment variable
-            //should probably be a block that loads the information from the state in the overlay
-            stack.add_env_var(
-                "PWD".into(),
-                Value::String {
-                    val: name.item.clone(),
-                    span: call.head,
-                },
-            );
-            return Ok(PipelineData::new(call.head));
-        }
-
         let mut args_strs = vec![];
 
         for arg in args {

crates/nu-parser/src/errors.rs

@@ -195,6 +195,14 @@ 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

@@ -973,6 +973,10 @@ pub fn parse_let(
                         );
                         error = error.or(err);
 
+                        if idx < (spans.len() - 1) {
+                            error = error.or(Some(ParseError::ExtraPositional(spans[idx + 1])));
+                        }
+
                         let mut idx = 0;
                         let (lvalue, err) =
                             parse_var_with_opt_type(working_set, &spans[1..(span.0)], &mut idx);

crates/nu-parser/src/parser.rs

@@ -3352,6 +3352,16 @@ 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/main.rs

@@ -21,6 +21,7 @@ use reedline::{
 };
 use std::{
     io::Write,
+    path::Path,
     sync::{
         atomic::{AtomicBool, Ordering},
         Arc,
@@ -402,7 +403,35 @@ fn main() -> Result<()> {
 
             let input = line_editor.read_line(prompt);
             match input {
-                Ok(Signal::Success(s)) => {
+                Ok(Signal::Success(mut s)) => {
+                    // Check if this is a single call to a directory, if so auto-cd
+                    let path = nu_path::expand_path(&s);
+                    let orig = s.clone();
+                    s = path.to_string_lossy().to_string();
+
+                    let path = Path::new(&s);
+                    if (orig.starts_with('.')
+                        || orig.starts_with('~')
+                        || orig.starts_with('/')
+                        || orig.starts_with('\\'))
+                        && path.is_dir()
+                    {
+                        // We have an auto-cd
+                        let _ = std::env::set_current_dir(&path);
+
+                        //FIXME: this only changes the current scope, but instead this environment variable
+                        //should probably be a block that loads the information from the state in the overlay
+                        stack.add_env_var(
+                            "PWD".into(),
+                            Value::String {
+                                val: s.clone(),
+                                span: Span { start: 0, end: 0 },
+                            },
+                        );
+
+                        continue;
+                    }
+
                     eval_source(
                         &mut engine_state,
                         &mut stack,

src/tests/test_parser.rs

@@ -113,3 +113,8 @@ fn long_flag() -> TestResult {
         "100",
     )
 }
+
+#[test]
+fn let_not_statement() -> TestResult {
+    fail_test(r#"let x = "hello" | str length"#, "can't")
+}