fix(parser): export def exposes arguments to current scope (#16262)

- Skip the extra call of `parse_internal_call` unless the `help` flag is set.
- Wrap `parse_internal_call` in a new scope to avoid leaking variables.

Closes #16211

crates/nu-command/tests/commands/export.rs

@@ -0,0 +1,26 @@
+use nu_test_support::nu;
+
+#[test]
+fn export_command_help() {
+    let actual = nu!("export -h");
+
+    assert!(
+        actual
+            .out
+            .contains("Export definitions or environment variables from a module")
+    );
+}
+
+#[test]
+fn export_command_unexpected() {
+    let actual = nu!("export foo");
+
+    assert!(actual.err.contains("unexpected export"));
+}
+
+#[test]
+fn export_alias_should_not_panic() {
+    let actual = nu!("export alias");
+
+    assert!(actual.err.contains("Missing"));
+}

crates/nu-command/tests/commands/export_def.rs

@@ -10,3 +10,14 @@ fn export_subcommands_help() {
             .contains("Define a custom command and export it from a module")
     );
 }
+
+#[test]
+fn export_should_not_expose_arguments() {
+    // issue #16211
+    let actual = nu!(r#"
+            export def foo [bar: int] {}
+            scope variables
+        "#);
+
+    assert!(!actual.out.contains("bar"));
+}

crates/nu-command/tests/commands/mod.rs

@@ -32,6 +32,7 @@ mod empty;
 mod error_make;
 mod every;
 mod exec;
+mod export;
 mod export_def;
 mod fill;
 mod filter;

crates/nu-parser/src/parse_keywords.rs

@@ -1026,7 +1026,7 @@ fn check_alias_name<'a>(working_set: &mut StateWorkingSet, spans: &'a [Span]) ->
         return None;
     };
 
-    if spans.len() == 1 {
+    if spans.len() == command_len {
         None
     } else if spans.len() < command_len + 3 {
         if working_set.get_span_contents(spans[command_len]) == b"=" {
@@ -1324,8 +1324,6 @@ pub fn parse_export_in_block(
     working_set: &mut StateWorkingSet,
     lite_command: &LiteCommand,
 ) -> Pipeline {
-    let call_span = Span::concat(&lite_command.parts);
-
     let parts = lite_command.command_parts();
     let full_name = if parts.len() > 1 {
         let sub = working_set.get_span_contents(parts[1]);
@@ -1347,11 +1345,45 @@ pub fn parse_export_in_block(
         return garbage_pipeline(working_set, &lite_command.parts);
     }
 
-    // No need to care for this when attributes are present, parse_attribute_block will throw the
-    // necessary error
-    if !lite_command.has_attributes() {
+    let pipeline = match full_name {
+        // `parse_def` and `parse_extern` work both with and without attributes
+        "export def" => parse_def(working_set, lite_command, None).0,
+        "export extern" => parse_extern(working_set, lite_command, None),
+        // Other definitions can't have attributes, so we handle attributes here with parse_attribute_block
+        _ if lite_command.has_attributes() => parse_attribute_block(working_set, lite_command),
+        "export alias" => parse_alias(working_set, lite_command, None),
+        "export const" => parse_const(working_set, &lite_command.parts[1..]).0,
+        "export use" => parse_use(working_set, lite_command, None).0,
+        "export module" => parse_module(working_set, lite_command, None).0,
+        _ => garbage_pipeline(working_set, &lite_command.parts),
+    };
+
+    let is_help_call = pipeline
+        .elements
+        .first()
+        .is_some_and(|e| match &e.expr.expr {
+            Expr::Call(call) => has_flag_const(working_set, call, "help").unwrap_or(false),
+            Expr::AttributeBlock(AttributeBlock {
+                attributes: _,
+                item: sub_expr,
+            }) => {
+                if let Expr::Call(call) = &sub_expr.expr {
+                    has_flag_const(working_set, call, "help").unwrap_or(false)
+                } else {
+                    false
+                }
+            }
+            _ => false,
+        });
+
+    // HACK: This is for different messages of e.g. `export def --help` and `def --help`,
+    if is_help_call || full_name == "export" {
+        let call_span = Span::concat(&lite_command.parts);
+
         if let Some(decl_id) = working_set.find_decl(full_name.as_bytes()) {
             let starting_error_count = working_set.parse_errors.len();
+            // Wrapped in a new scope to avoid unexpected side effects
+            working_set.enter_scope();
             let ParsedInternalCall { call, output, .. } = parse_internal_call(
                 working_set,
                 if full_name == "export" {
@@ -1366,14 +1398,19 @@ pub fn parse_export_in_block(
                 },
                 decl_id,
             );
+            working_set.exit_scope();
             // don't need errors generated by parse_internal_call
             // further error will be generated by detail `parse_xxx` function.
             working_set.parse_errors.truncate(starting_error_count);
 
-            let decl = working_set.get_decl(decl_id);
-            check_call(working_set, call_span, &decl.signature(), &call);
-            let Ok(is_help) = has_flag_const(working_set, &call, "help") else {
-                return garbage_pipeline(working_set, &lite_command.parts);
+            let is_help = is_help_call || {
+                let decl = working_set.get_decl(decl_id);
+                check_call(working_set, call_span, &decl.signature(), &call);
+                if let Ok(is_help) = has_flag_const(working_set, &call, "help") {
+                    is_help
+                } else {
+                    return garbage_pipeline(working_set, &lite_command.parts);
+                }
             };
 
             if starting_error_count != working_set.parse_errors.len() || is_help {
@@ -1384,6 +1421,10 @@ pub fn parse_export_in_block(
                     output,
                 )]);
             }
+            working_set.error(ParseError::UnexpectedKeyword(
+                full_name.into(),
+                lite_command.parts[0],
+            ));
         } else {
             working_set.error(ParseError::UnknownState(
                 format!("internal error: '{full_name}' declaration not found",),
@@ -1393,25 +1434,7 @@ pub fn parse_export_in_block(
         };
     }
 
-    match full_name {
-        // `parse_def` and `parse_extern` work both with and without attributes
-        "export def" => parse_def(working_set, lite_command, None).0,
-        "export extern" => parse_extern(working_set, lite_command, None),
-        // Other definitions can't have attributes, so we handle attributes here with parse_attribute_block
-        _ if lite_command.has_attributes() => parse_attribute_block(working_set, lite_command),
-        "export alias" => parse_alias(working_set, lite_command, None),
-        "export const" => parse_const(working_set, &lite_command.parts[1..]).0,
-        "export use" => parse_use(working_set, lite_command, None).0,
-        "export module" => parse_module(working_set, lite_command, None).0,
-        _ => {
-            working_set.error(ParseError::UnexpectedKeyword(
-                full_name.into(),
-                lite_command.parts[0],
-            ));
-
-            garbage_pipeline(working_set, &lite_command.parts)
-        }
-    }
+    pipeline
 }
 
 // This one will trigger only in a module