normalize special characters in module names to allow variable access (…

…nushell#14353)

Fixes nushell#14252

# User-Facing Changes

- Special characters in module names are replaced with underscores when
  importing constants, preventing "expected valid variable name":

```nushell
> module foo-bar { export const baz = 1 }
> use foo-bar
> $foo_bar.baz
```

- "expected valid variable name" errors now include a suggestion list:

```nushell
> module foo-bar { export const baz = 1 }
> use foo-bar
> $foo-bar
Error: nu::parser::parse_mismatch_with_did_you_mean

  × Parse mismatch during operation.
   ╭─[entry #1:1:1]
 1 │ $foo-bar;
   · ────┬───
   ·     ╰── expected valid variable name. Did you mean '$foo_bar'?
   ╰────
```

crates/nu-parser/src/parser.rs

@@ -2122,7 +2122,21 @@ pub fn parse_variable_expr(working_set: &mut StateWorkingSet, span: Span) -> Exp
         String::from_utf8_lossy(contents).to_string()
     };
 
-    if let Some(id) = parse_variable(working_set, span) {
+    let bytes = working_set.get_span_contents(span);
+    let suggestion = || {
+        DidYouMean::new(
+            &working_set.list_variables(),
+            working_set.get_span_contents(span),
+        )
+    };
+    if !is_variable(bytes) {
+        working_set.error(ParseError::ExpectedWithDidYouMean(
+            "valid variable name",
+            suggestion(),
+            span,
+        ));
+        garbage(working_set, span)
+    } else if let Some(id) = working_set.find_variable(bytes) {
         Expression::new(
             working_set,
             Expr::Var(id),
@@ -2133,9 +2147,7 @@ pub fn parse_variable_expr(working_set: &mut StateWorkingSet, span: Span) -> Exp
         working_set.error(ParseError::EnvVarNotVar(name, span));
         garbage(working_set, span)
     } else {
-        let ws = &*working_set;
-        let suggestion = DidYouMean::new(&ws.list_variables(), ws.get_span_contents(span));
-        working_set.error(ParseError::VariableNotFound(suggestion, span));
+        working_set.error(ParseError::VariableNotFound(suggestion(), span));
         garbage(working_set, span)
     }
 }
@@ -5612,18 +5624,6 @@ pub fn parse_expression(working_set: &mut StateWorkingSet, spans: &[Span]) -> Ex
     }
 }
 
-pub fn parse_variable(working_set: &mut StateWorkingSet, span: Span) -> Option<VarId> {
-    let bytes = working_set.get_span_contents(span);
-
-    if is_variable(bytes) {
-        working_set.find_variable(bytes)
-    } else {
-        working_set.error(ParseError::Expected("valid variable name", span));
-
-        None
-    }
-}
-
 pub fn parse_builtin_commands(
     working_set: &mut StateWorkingSet,
     lite_command: &LiteCommand,

crates/nu-protocol/src/errors/parse_error.rs

@@ -55,6 +55,10 @@ pub enum ParseError {
     #[diagnostic(code(nu::parser::parse_mismatch_with_full_string_msg))]
     ExpectedWithStringMsg(String, #[label("expected {0}")] Span),
 
+    #[error("Parse mismatch during operation.")]
+    #[diagnostic(code(nu::parser::parse_mismatch_with_did_you_mean))]
+    ExpectedWithDidYouMean(&'static str, DidYouMean, #[label("expected {0}. {1}")] Span),
+
     #[error("Command does not support {0} input.")]
     #[diagnostic(code(nu::parser::input_type_mismatch))]
     InputMismatch(Type, #[label("command doesn't support {0} input")] Span),
@@ -551,6 +555,7 @@ impl ParseError {
             ParseError::Unbalanced(_, _, s) => *s,
             ParseError::Expected(_, s) => *s,
             ParseError::ExpectedWithStringMsg(_, s) => *s,
+            ParseError::ExpectedWithDidYouMean(_, _, s) => *s,
             ParseError::Mismatch(_, _, s) => *s,
             ParseError::UnsupportedOperationLHS(_, _, s, _) => *s,
             ParseError::UnsupportedOperationRHS(_, _, _, _, s, _) => *s,

crates/nu-protocol/src/module.rs

@@ -167,7 +167,7 @@ impl Module {
                 vec![]
             } else {
                 vec![(
-                    final_name.clone(),
+                    normalize_module_name(&final_name),
                     Value::record(
                         const_rows
                             .into_iter()
@@ -425,3 +425,32 @@ impl Module {
         result
     }
 }
+
+/// normalize module names for exporting as record constant
+fn normalize_module_name(bytes: &[u8]) -> Vec<u8> {
+    bytes
+        .iter()
+        .map(|x| match is_identifier_byte(*x) {
+            true => *x,
+            false => b'_',
+        })
+        .collect()
+}
+
+fn is_identifier_byte(b: u8) -> bool {
+    b != b'.'
+        && b != b'['
+        && b != b'('
+        && b != b'{'
+        && b != b'+'
+        && b != b'-'
+        && b != b'*'
+        && b != b'^'
+        && b != b'/'
+        && b != b'='
+        && b != b'!'
+        && b != b'<'
+        && b != b'>'
+        && b != b'&'
+        && b != b'|'
+}

tests/repl/test_modules.rs

@@ -1,4 +1,5 @@
 use crate::repl::tests::{fail_test, run_test, TestResult};
+use rstest::rstest;
 
 #[test]
 fn module_def_imports_1() -> TestResult {
@@ -145,6 +146,28 @@ fn export_module_which_defined_const() -> TestResult {
     )
 }
 
+#[rstest]
+#[case("spam-mod")]
+#[case("spam/mod")]
+#[case("spam=mod")]
+fn export_module_with_normalized_var_name(#[case] name: &str) -> TestResult {
+    let def = format!(
+        "module {name} {{ export const b = 3; export module {name}2 {{ export const c = 4 }}  }}"
+    );
+    run_test(&format!("{def}; use {name}; $spam_mod.b"), "3")?;
+    run_test(&format!("{def}; use {name} *; $spam_mod2.c"), "4")
+}
+
+#[rstest]
+#[case("spam-mod")]
+#[case("spam/mod")]
+fn use_module_with_invalid_var_name(#[case] name: &str) -> TestResult {
+    fail_test(
+        &format!("module {name} {{ export const b = 3 }}; use {name}; ${name}"),
+        "expected valid variable name. Did you mean '$spam_mod'",
+    )
+}
+
 #[test]
 fn cannot_export_private_const() -> TestResult {
     fail_test(