Properly handle commands defined inside of other commands (#2837)

* Fix function inner scopes

* tweak error

crates/nu-cli/src/commands/classified/block.rs

@@ -19,6 +19,11 @@ pub async fn run_block(
     mut input: InputStream,
 ) -> Result<InputStream, ShellError> {
     let mut output: Result<InputStream, ShellError> = Ok(InputStream::empty());
+    ctx.scope.enter_scope();
+    for (_, definition) in block.definitions.iter() {
+        ctx.scope.add_definition(definition.clone());
+    }
+
     for group in &block.block {
         match output {
             Ok(inp) if inp.is_empty() => {}
@@ -26,7 +31,7 @@ pub async fn run_block(
                 // Run autoview on the values we've seen so far
                 // We may want to make this configurable for other kinds of hosting
                 if let Some(autoview) = ctx.get_command("autoview") {
-                    let mut output_stream = ctx
+                    let mut output_stream = match ctx
                         .run_command(
                             autoview,
                             Tag::unknown(),
@@ -39,32 +44,49 @@ pub async fn run_block(
                             ),
                             inp,
                         )
-                        .await?;
+                        .await
+                    {
+                        Ok(x) => x,
+                        Err(e) => {
+                            ctx.scope.exit_scope();
+                            return Err(e);
+                        }
+                    };
                     match output_stream.try_next().await {
                         Ok(Some(ReturnSuccess::Value(Value {
                             value: UntaggedValue::Error(e),
                             ..
-                        }))) => return Err(e),
+                        }))) => {
+                            ctx.scope.exit_scope();
+                            return Err(e);
+                        }
                         Ok(Some(_item)) => {
                             if let Some(err) = ctx.get_errors().get(0) {
                                 ctx.clear_errors();
+                                ctx.scope.exit_scope();
                                 return Err(err.clone());
                             }
                             if ctx.ctrl_c.load(Ordering::SeqCst) {
+                                ctx.scope.exit_scope();
                                 return Ok(InputStream::empty());
                             }
                         }
                         Ok(None) => {
                             if let Some(err) = ctx.get_errors().get(0) {
                                 ctx.clear_errors();
+                                ctx.scope.exit_scope();
                                 return Err(err.clone());
                             }
                         }
-                        Err(e) => return Err(e),
+                        Err(e) => {
+                            ctx.scope.exit_scope();
+                            return Err(e);
+                        }
                     }
                 }
             }
             Err(e) => {
+                ctx.scope.exit_scope();
                 return Err(e);
             }
         }
@@ -79,10 +101,14 @@ pub async fn run_block(
                         Ok(Some(ReturnSuccess::Value(Value {
                             value: UntaggedValue::Error(e),
                             ..
-                        }))) => return Err(e),
+                        }))) => {
+                            ctx.scope.exit_scope();
+                            return Err(e);
+                        }
                         Ok(Some(_item)) => {
                             if let Some(err) = ctx.get_errors().get(0) {
                                 ctx.clear_errors();
+                                ctx.scope.exit_scope();
                                 return Err(err.clone());
                             }
                             if ctx.ctrl_c.load(Ordering::SeqCst) {
@@ -91,19 +117,25 @@ pub async fn run_block(
                                 // causes lifetime issues. A future contribution
                                 // could attempt to return the current output.
                                 // https://github.com/nushell/nushell/pull/2830#discussion_r550319687
+                                ctx.scope.exit_scope();
                                 return Ok(InputStream::empty());
                             }
                         }
                         Ok(None) => {
                             if let Some(err) = ctx.get_errors().get(0) {
                                 ctx.clear_errors();
+                                ctx.scope.exit_scope();
                                 return Err(err.clone());
                             }
                         }
-                        Err(e) => return Err(e),
+                        Err(e) => {
+                            ctx.scope.exit_scope();
+                            return Err(e);
+                        }
                     }
                 }
                 Err(e) => {
+                    ctx.scope.exit_scope();
                     return Err(e);
                 }
             }
@@ -112,6 +144,7 @@ pub async fn run_block(
             input = InputStream::empty();
         }
     }
+    ctx.scope.exit_scope();
 
     output
 }

crates/nu-parser/src/parse.rs

@@ -2142,6 +2142,8 @@ fn parse_definition(call: &LiteCommand, scope: &dyn ParserScope) -> Option<Parse
                 // We have a literal block
                 let string: String = chars.collect();
 
+                scope.enter_scope();
+
                 let (tokens, err) = lex(&string, call.parts[3].span.start() + 1);
                 if err.is_some() {
                     return err;
@@ -2152,6 +2154,7 @@ fn parse_definition(call: &LiteCommand, scope: &dyn ParserScope) -> Option<Parse
                 };
 
                 let (mut block, err) = classify_block(&lite_block, scope);
+                scope.exit_scope();
 
                 block.params = signature;
                 block.params.name = name;
@@ -2285,6 +2288,14 @@ pub fn classify_block(
         }
     }
 
+    let definitions = scope.get_definitions();
+    for definition in definitions.into_iter() {
+        let name = definition.params.name.clone();
+        if !output.definitions.contains_key(&name) {
+            output.definitions.insert(name, definition.clone());
+        }
+    }
+
     (output, error)
 }
 

tests/shell/pipeline/commands/internal.rs

@@ -380,6 +380,30 @@ fn run_custom_subcommand() {
     assert_eq!(actual.out, "bobbob");
 }
 
+#[test]
+fn run_inner_custom_command() {
+    let actual = nu!(
+        cwd: ".",
+        r#"
+          def outer [x] { def inner [y] { echo $y }; inner $x }; outer 10
+        "#
+    );
+
+    assert_eq!(actual.out, "10");
+}
+
+#[test]
+fn run_broken_inner_custom_command() {
+    let actual = nu!(
+        cwd: ".",
+        r#"
+        def outer [x] { def inner [y] { echo $y }; inner $x }; inner 10
+        "#
+    );
+
+    assert!(actual.err.contains("not found"));
+}
+
 #[test]
 fn set_variable() {
     let actual = nu!(