tighter restrictions on alias and def names (#7392)

# Description

Prevent a situation where a `def` can't be run due to a poor choice of
name. Related: #6335. Hashtags, numbers and filesizes are no longer
allowed. `alias` check has been moved because previously `alias 123`
would be caught but `alias "123"` would be permitted.

# User-Facing Changes

Some definitions can no longer be made, but because they couldn't be run
previously anyway, it doesn't really matter.

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.

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

@@ -51,7 +51,8 @@ fn alias_fails_with_invalid_name() {
     ));
     assert!(actual
         .err
-        .contains("alias name can't be a number or a filesize"));
+        .contains("alias name can't be a number, a filesize, or contain a hash"));
+
     let actual = nu!(
         cwd: "tests/fixtures/formats", pipeline(
         r#"
@@ -60,7 +61,17 @@ fn alias_fails_with_invalid_name() {
     ));
     assert!(actual
         .err
-        .contains("alias name can't be a number or a filesize"));
+        .contains("alias name can't be a number, a filesize, or contain a hash"));
+
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+            alias "te#t" = echo "test"   
+        "#
+    ));
+    assert!(actual
+        .err
+        .contains("alias name can't be a number, a filesize, or contain a hash"));
 }
 
 #[test]

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

@@ -30,3 +30,36 @@ fn def_errors_with_multiple_short_flags() {
 
     assert!(actual.err.contains("expected one short flag"));
 }
+
+#[test]
+fn def_fails_with_invalid_name() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+            def 1234 = echo "test"
+        "#
+    ));
+    assert!(actual
+        .err
+        .contains("command name can't be a number, a filesize, or contain a hash"));
+
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+            def 5gib = echo "test"
+        "#
+    ));
+    assert!(actual
+        .err
+        .contains("command name can't be a number, a filesize, or contain a hash"));
+
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+            def "te#t" = echo "test"
+        "#
+    ));
+    assert!(actual
+        .err
+        .contains("command name can't be a number, a filesize, or contain a hash"));
+}

crates/nu-parser/src/errors.rs

@@ -172,7 +172,13 @@ pub enum ParseError {
 
     #[error("Alias name not supported.")]
     #[diagnostic(code(nu::parser::variable_not_valid), url(docsrs))]
-    AliasNotValid(#[label = "alias name can't be a number or a filesize"] Span),
+    AliasNotValid(#[label = "alias name can't be a number, a filesize, or contain a hash"] Span),
+
+    #[error("Command name not supported.")]
+    #[diagnostic(code(nu::parser::variable_not_valid), url(docsrs))]
+    CommandDefNotValid(
+        #[label = "command name can't be a number, a filesize, or contain a hash"] Span,
+    ),
 
     #[error("Module not found.")]
     #[diagnostic(
@@ -419,6 +425,7 @@ impl ParseError {
             ParseError::VariableNotFound(s) => *s,
             ParseError::VariableNotValid(s) => *s,
             ParseError::AliasNotValid(s) => *s,
+            ParseError::CommandDefNotValid(s) => *s,
             ParseError::ModuleNotFound(s) => *s,
             ParseError::CyclicalModuleImport(_, s) => *s,
             ParseError::ModuleOrOverlayNotFound(s) => *s,

crates/nu-parser/src/parse_keywords.rs

@@ -57,6 +57,13 @@ pub fn parse_def_predecl(
         let signature = sig.as_signature();
         working_set.exit_scope();
         if let (Some(name), Some(mut signature)) = (name, signature) {
+            if name.contains('#')
+                || name.parse::<bytesize::ByteSize>().is_ok()
+                || name.parse::<f64>().is_ok()
+            {
+                return Some(ParseError::CommandDefNotValid(spans[1]));
+            }
+
             signature.name = name;
             let decl = signature.predeclare();
 
@@ -80,6 +87,13 @@ pub fn parse_def_predecl(
         working_set.exit_scope();
 
         if let (Some(name), Some(mut signature)) = (name, signature) {
+            if name.contains('#')
+                || name.parse::<bytesize::ByteSize>().is_ok()
+                || name.parse::<f64>().is_ok()
+            {
+                return Some(ParseError::CommandDefNotValid(spans[1]));
+            }
+
             signature.name = name.clone();
             //let decl = signature.predeclare();
             let decl = KnownExternal {
@@ -593,27 +607,16 @@ pub fn parse_alias(
         return (Pipeline::from_vec(vec![expr]), None);
     }
 
-    let (name_span, alias_name, split_id) =
+    let (name_span, split_id) =
         if spans.len() > 1 && working_set.get_span_contents(spans[0]) == b"export" {
-            (spans[1], spans.get(2), 2)
+            (spans[1], 2)
         } else {
-            (spans[0], spans.get(1), 1)
+            (spans[0], 1)
         };
 
     let name = working_set.get_span_contents(name_span);
 
     if name == b"alias" {
-        if let Some(alias_name) = alias_name {
-            let alias_name = String::from_utf8_lossy(working_set.get_span_contents(*alias_name));
-            if alias_name.parse::<bytesize::ByteSize>().is_ok() || alias_name.parse::<f64>().is_ok()
-            {
-                return (
-                    Pipeline::from_vec(vec![garbage(name_span)]),
-                    Some(ParseError::AliasNotValid(name_span)),
-                );
-            }
-        }
-
         if let Some((span, err)) = check_name(working_set, spans) {
             return (Pipeline::from_vec(vec![garbage(*span)]), Some(err));
         }
@@ -656,6 +659,17 @@ pub fn parse_alias(
 
                 let replacement = spans[(split_id + 2)..].to_vec();
 
+                let checked_name = String::from_utf8_lossy(&alias_name);
+                if checked_name.contains('#')
+                    || checked_name.parse::<bytesize::ByteSize>().is_ok()
+                    || checked_name.parse::<f64>().is_ok()
+                {
+                    return (
+                        Pipeline::from_vec(vec![garbage(name_span)]),
+                        Some(ParseError::AliasNotValid(name_span)),
+                    );
+                }
+
                 working_set.add_alias(alias_name, replacement);
             }