fix error when exporting consts with type signatures in modules (#14118)

Fixes #14023

# Description

- Prevents "failed to find added variable" when modules export constants
  with type signatures:

```nushell
> module foo { export const bar: int = 2 }
Error: nu::parser::unknown_state

  × Internal error.
   ╭─[entry #1:1:21]
 1 │ module foo { export const bar: int = 2 }
   ·                     ─────────┬────────
   ·                              ╰── failed to find added variable
```

- Returns `name_is_builtin_var` errors for names with type signatures:

```nushell
> let env: string = "";
Error: nu::parser::name_is_builtin_var

  × `env` used as variable name.
   ╭─[entry #1:1:5]
 1 │ let env: string = "";
   ·     ─┬─
   ·      ╰── already a builtin variable
```

Author: sgvictorino

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

@@ -1,10 +1,11 @@
 use nu_test_support::nu;
+use rstest::rstest;
 
-#[test]
-fn let_name_builtin_var() {
-    let actual = nu!("let in = 3");
-
-    assert!(actual
+#[rstest]
+#[case("let in = 3")]
+#[case("let in: int = 3")]
+fn let_name_builtin_var(#[case] assignment: &str) {
+    assert!(nu!(assignment)
         .err
         .contains("'in' is the name of a builtin Nushell variable"));
 }

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

@@ -1,4 +1,5 @@
 use nu_test_support::nu;
+use rstest::rstest;
 
 #[test]
 fn mut_variable() {
@@ -7,11 +8,11 @@ fn mut_variable() {
     assert_eq!(actual.out, "4");
 }
 
-#[test]
-fn mut_name_builtin_var() {
-    let actual = nu!("mut in = 3");
-
-    assert!(actual
+#[rstest]
+#[case("mut in = 3")]
+#[case("mut in: int = 3")]
+fn mut_name_builtin_var(#[case] assignment: &str) {
+    assert!(nu!(assignment)
         .err
         .contains("'in' is the name of a builtin Nushell variable"));
 }

crates/nu-parser/src/parse_keywords.rs

@@ -1204,7 +1204,7 @@ pub fn parse_export_in_block(
     match full_name {
         "export alias" => parse_alias(working_set, lite_command, None),
         "export def" => parse_def(working_set, lite_command, None).0,
-        "export const" => parse_const(working_set, &lite_command.parts[1..]),
+        "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,
         "export extern" => parse_extern(working_set, lite_command, None),
@@ -1514,7 +1514,7 @@ pub fn parse_export_in_module(
                 result
             }
             b"const" => {
-                let pipeline = parse_const(working_set, &spans[1..]);
+                let (pipeline, var_name_span) = parse_const(working_set, &spans[1..]);
                 let export_const_decl_id = if let Some(id) = working_set.find_decl(b"export const")
                 {
                     id
@@ -1542,8 +1542,8 @@ pub fn parse_export_in_module(
 
                 let mut result = vec![];
 
-                if let Some(var_name_span) = spans.get(2) {
-                    let var_name = working_set.get_span_contents(*var_name_span);
+                if let Some(var_name_span) = var_name_span {
+                    let var_name = working_set.get_span_contents(var_name_span);
                     let var_name = trim_quotes(var_name);
 
                     if let Some(var_id) = working_set.find_variable(var_name) {
@@ -1762,7 +1762,7 @@ pub fn parse_module_block(
                 }
                 b"const" => block
                     .pipelines
-                    .push(parse_const(working_set, &command.parts)),
+                    .push(parse_const(working_set, &command.parts).0),
                 b"extern" => block
                     .pipelines
                     .push(parse_extern(working_set, command, None)),
@@ -3154,7 +3154,8 @@ pub fn parse_let(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline
     garbage_pipeline(working_set, spans)
 }
 
-pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline {
+/// Additionally returns a span encompassing the variable name, if successful.
+pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> (Pipeline, Option<Span>) {
     trace!("parsing: const");
 
     // JT: Disabling check_name because it doesn't work with optional types in the declaration
@@ -3271,28 +3272,37 @@ pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipelin
                     let call = Box::new(Call {
                         decl_id,
                         head: spans[0],
-                        arguments: vec![Argument::Positional(lvalue), Argument::Positional(rvalue)],
+                        arguments: vec![
+                            Argument::Positional(lvalue.clone()),
+                            Argument::Positional(rvalue),
+                        ],
                         parser_info: HashMap::new(),
                     });
 
-                    return Pipeline::from_vec(vec![Expression::new(
-                        working_set,
-                        Expr::Call(call),
-                        Span::concat(spans),
-                        Type::Any,
-                    )]);
+                    return (
+                        Pipeline::from_vec(vec![Expression::new(
+                            working_set,
+                            Expr::Call(call),
+                            Span::concat(spans),
+                            Type::Any,
+                        )]),
+                        Some(lvalue.span),
+                    );
                 }
             }
         }
         let ParsedInternalCall { call, output } =
             parse_internal_call(working_set, spans[0], &spans[1..], decl_id);
 
-        return Pipeline::from_vec(vec![Expression::new(
-            working_set,
-            Expr::Call(call),
-            Span::concat(spans),
-            output,
-        )]);
+        return (
+            Pipeline::from_vec(vec![Expression::new(
+                working_set,
+                Expr::Call(call),
+                Span::concat(spans),
+                output,
+            )]),
+            None,
+        );
     } else {
         working_set.error(ParseError::UnknownState(
             "internal error: let or const statements not found in core language".into(),
@@ -3305,7 +3315,7 @@ pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipelin
         Span::concat(spans),
     ));
 
-    garbage_pipeline(working_set, spans)
+    (garbage_pipeline(working_set, spans), None)
 }
 
 pub fn parse_mut(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline {

crates/nu-parser/src/parser.rs

@@ -3113,7 +3113,8 @@ pub fn parse_var_with_opt_type(
     spans_idx: &mut usize,
     mutable: bool,
 ) -> (Expression, Option<Type>) {
-    let bytes = working_set.get_span_contents(spans[*spans_idx]).to_vec();
+    let name_span = spans[*spans_idx];
+    let bytes = working_set.get_span_contents(name_span).to_vec();
 
     if bytes.contains(&b' ')
         || bytes.contains(&b'"')
@@ -3125,9 +3126,11 @@ pub fn parse_var_with_opt_type(
     }
 
     if bytes.ends_with(b":") {
+        let name_span = Span::new(name_span.start, name_span.end - 1);
+        let var_name = bytes[0..(bytes.len() - 1)].to_vec();
+
         // We end with colon, so the next span should be the type
         if *spans_idx + 1 < spans.len() {
-            let span_beginning = *spans_idx;
             *spans_idx += 1;
             // signature like record<a: int b: int> is broken into multiple spans due to
             // whitespaces. Collect the rest into one span and work on it
@@ -3144,8 +3147,6 @@ pub fn parse_var_with_opt_type(
             let ty = parse_type(working_set, &type_bytes, tokens[0].span);
             *spans_idx = spans.len() - 1;
 
-            let var_name = bytes[0..(bytes.len() - 1)].to_vec();
-
             if !is_variable(&var_name) {
                 working_set.error(ParseError::Expected(
                     "valid variable name",
@@ -3157,17 +3158,10 @@ pub fn parse_var_with_opt_type(
             let id = working_set.add_variable(var_name, spans[*spans_idx - 1], ty.clone(), mutable);
 
             (
-                Expression::new(
-                    working_set,
-                    Expr::VarDecl(id),
-                    Span::concat(&spans[span_beginning..*spans_idx + 1]),
-                    ty.clone(),
-                ),
+                Expression::new(working_set, Expr::VarDecl(id), name_span, ty.clone()),
                 Some(ty),
             )
         } else {
-            let var_name = bytes[0..(bytes.len() - 1)].to_vec();
-
             if !is_variable(&var_name) {
                 working_set.error(ParseError::Expected(
                     "valid variable name",
@@ -5615,7 +5609,7 @@ pub fn parse_builtin_commands(
                 .parts_including_redirection()
                 .collect::<Vec<Span>>(),
         ),
-        b"const" => parse_const(working_set, &lite_command.parts),
+        b"const" => parse_const(working_set, &lite_command.parts).0,
         b"mut" => parse_mut(
             working_set,
             &lite_command

tests/repl/test_modules.rs

@@ -117,6 +117,10 @@ fn export_consts() -> TestResult {
     run_test(
         r#"module spam { export const b = 3; }; use spam b; $b"#,
         "3",
+    )?;
+    run_test(
+        r#"module spam { export const b: int = 3; }; use spam b; $b"#,
+        "3",
     )
 }