diff --git a/crates/nu-command/tests/commands/alias.rs b/crates/nu-command/tests/commands/alias.rs index 3600e83e49..8978d883a2 100644 --- a/crates/nu-command/tests/commands/alias.rs +++ b/crates/nu-command/tests/commands/alias.rs @@ -43,35 +43,38 @@ fn alias_hiding_2() { #[test] fn alias_fails_with_invalid_name() { + let err_msg = "alias name can't be a number, a filesize, or contain a hash # or caret ^"; let actual = nu!( - cwd: "tests/fixtures/formats", pipeline( + cwd: ".", pipeline( r#" alias 1234 = echo "test" "# )); - assert!(actual - .err - .contains("alias name can't be a number, a filesize, or contain a hash")); + assert!(actual.err.contains(err_msg)); let actual = nu!( - cwd: "tests/fixtures/formats", pipeline( + cwd: ".", pipeline( r#" alias 5gib = echo "test" "# )); - assert!(actual - .err - .contains("alias name can't be a number, a filesize, or contain a hash")); + assert!(actual.err.contains(err_msg)); let actual = nu!( - cwd: "tests/fixtures/formats", pipeline( + cwd: ".", pipeline( r#" alias "te#t" = echo "test" "# )); - assert!(actual - .err - .contains("alias name can't be a number, a filesize, or contain a hash")); + assert!(actual.err.contains(err_msg)); + + let actual = nu!( + cwd: ".", pipeline( + r#" + alias ^foo = "bar" + "# + )); + assert!(actual.err.contains(err_msg)); } #[test] diff --git a/crates/nu-command/tests/commands/def.rs b/crates/nu-command/tests/commands/def.rs index 8323e275a9..5123c1b7bb 100644 --- a/crates/nu-command/tests/commands/def.rs +++ b/crates/nu-command/tests/commands/def.rs @@ -28,38 +28,33 @@ fn def_errors_with_multiple_short_flags() { "# )); - assert!(actual.err.contains("expected one short flag")); + assert!(actual.err.contains("expected only one short flag")); } #[test] fn def_fails_with_invalid_name() { + let err_msg = "command name can't be a number, a filesize, or contain a hash # or caret ^"; let actual = nu!( - cwd: "tests/fixtures/formats", pipeline( + cwd: ".", pipeline( r#" def 1234 = echo "test" "# )); - assert!(actual - .err - .contains("command name can't be a number, a filesize, or contain a hash")); + assert!(actual.err.contains(err_msg)); let actual = nu!( - cwd: "tests/fixtures/formats", pipeline( + cwd: ".", pipeline( r#" def 5gib = echo "test" "# )); - assert!(actual - .err - .contains("command name can't be a number, a filesize, or contain a hash")); + assert!(actual.err.contains(err_msg)); let actual = nu!( - cwd: "tests/fixtures/formats", pipeline( + cwd: ".", pipeline( r#" - def "te#t" = echo "test" + def ^foo [] {} "# )); - assert!(actual - .err - .contains("command name can't be a number, a filesize, or contain a hash")); + assert!(actual.err.contains(err_msg)); } diff --git a/crates/nu-parser/src/errors.rs b/crates/nu-parser/src/errors.rs index c48398f9bb..a4be345148 100644 --- a/crates/nu-parser/src/errors.rs +++ b/crates/nu-parser/src/errors.rs @@ -172,12 +172,15 @@ 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, a filesize, or contain a hash"] Span), + AliasNotValid( + #[label = "alias name can't be a number, a filesize, or contain a hash # or caret ^"] 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, + #[label = "command name can't be a number, a filesize, or contain a hash # or caret ^"] + Span, ), #[error("Module not found.")] diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 84ceb5cbd3..2ef8db284e 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -58,6 +58,7 @@ pub fn parse_def_predecl( working_set.exit_scope(); if let (Some(name), Some(mut signature)) = (name, signature) { if name.contains('#') + || name.contains('^') || name.parse::().is_ok() || name.parse::().is_ok() { @@ -661,6 +662,7 @@ pub fn parse_alias( let checked_name = String::from_utf8_lossy(&alias_name); if checked_name.contains('#') + || checked_name.contains('^') || checked_name.parse::().is_ok() || checked_name.parse::().is_ok() { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 115bb6cbe6..67578c46f2 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -3265,6 +3265,7 @@ pub fn parse_signature_helper( let span = *span; let contents = working_set.get_span_contents(span); + // The : symbol separates types if contents == b":" { match parse_mode { ParseMode::ArgMode => { @@ -3276,7 +3277,9 @@ pub fn parse_signature_helper( error.or_else(|| Some(ParseError::Expected("type".into(), span))); } } - } else if contents == b"=" { + } + // The = symbol separates a variable from its default value + else if contents == b"=" { match parse_mode { ParseMode::ArgMode | ParseMode::TypeMode => { parse_mode = ParseMode::DefaultValueMode; @@ -3291,8 +3294,10 @@ pub fn parse_signature_helper( } else { match parse_mode { ParseMode::ArgMode => { + // Long flag with optional short form, e.g. --output, --age (-a) if contents.starts_with(b"--") && contents.len() > 2 { - // Long flag + // Split the long flag from the short flag with the ( character as delimiter. + // The trailing ) is removed further down. let flags: Vec<_> = contents.split(|x| x == &b'(').map(|x| x.to_vec()).collect(); @@ -3317,6 +3322,7 @@ pub fn parse_signature_helper( let var_id = working_set.add_variable(variable_name, span, Type::Any, false); + // If there's no short flag, exit now. Otherwise, parse it. if flags.len() == 1 { args.push(Arg::Flag(Flag { arg: None, @@ -3329,7 +3335,10 @@ pub fn parse_signature_helper( })); } else if flags.len() >= 3 { error = error.or_else(|| { - Some(ParseError::Expected("one short flag".into(), span)) + Some(ParseError::Expected( + "only one short flag alternative".into(), + span, + )) }); } else { let short_flag = &flags[1]; @@ -3337,12 +3346,18 @@ pub fn parse_signature_helper( || !short_flag.ends_with(b")") { error = error.or_else(|| { - Some(ParseError::Expected("short flag".into(), span)) + Some(ParseError::Expected( + "short flag alternative for the long flag".into(), + span, + )) }); short_flag } else { + // Obtain the flag's name by removing the starting - and trailing ) &short_flag[1..(short_flag.len() - 1)] }; + // Note that it is currently possible to make a short flag with non-alphanumeric characters, + // like -). let short_flag = String::from_utf8_lossy(short_flag).to_string(); @@ -3388,9 +3403,9 @@ pub fn parse_signature_helper( }); } } - } else if contents.starts_with(b"-") && contents.len() > 1 { - // Short flag - + } + // Mandatory short flag, e.g. -e (must be one character) + else if contents.starts_with(b"-") && contents.len() > 1 { let short_flag = &contents[1..]; let short_flag = String::from_utf8_lossy(short_flag).to_string(); let chars: Vec = short_flag.chars().collect(); @@ -3404,6 +3419,7 @@ pub fn parse_signature_helper( let mut encoded_var_name = vec![0u8; 4]; let len = chars[0].encode_utf8(&mut encoded_var_name).len(); let variable_name = encoded_var_name[0..len].to_vec(); + if !is_variable(&variable_name) { error = error.or_else(|| { Some(ParseError::Expected( @@ -3425,7 +3441,10 @@ pub fn parse_signature_helper( var_id: Some(var_id), default_value: None, })); - } else if contents.starts_with(b"(-") { + } + // Short flag alias for long flag, e.g. --b, (-a) + // This is the same as --b (-a) + else if contents.starts_with(b"(-") { let short_flag = &contents[2..]; let short_flag = if !short_flag.ends_with(b")") { @@ -3468,7 +3487,9 @@ pub fn parse_signature_helper( Some(ParseError::Expected("short flag".into(), span)) }); } - } else if contents.ends_with(b"?") { + } + // Positional arg, optional + else if contents.ends_with(b"?") { let contents: Vec<_> = contents[..(contents.len() - 1)].into(); let name = String::from_utf8_lossy(&contents).to_string(); @@ -3484,7 +3505,6 @@ pub fn parse_signature_helper( let var_id = working_set.add_variable(contents, span, Type::Any, false); - // Positional arg, optional args.push(Arg::Positional( PositionalArg { desc: String::new(), @@ -3495,9 +3515,12 @@ pub fn parse_signature_helper( }, false, )) - } else if let Some(contents) = contents.strip_prefix(b"...") { + } + // Rest param + else if let Some(contents) = contents.strip_prefix(b"...") { let name = String::from_utf8_lossy(contents).to_string(); let contents_vec: Vec = contents.to_vec(); + if !is_variable(&contents_vec) { error = error.or_else(|| { Some(ParseError::Expected( @@ -3517,7 +3540,9 @@ pub fn parse_signature_helper( var_id: Some(var_id), default_value: None, })); - } else { + } + // Normal param + else { let name = String::from_utf8_lossy(contents).to_string(); let contents_vec = contents.to_vec(); diff --git a/src/tests/test_engine.rs b/src/tests/test_engine.rs index 7d05be0489..49f19c164e 100644 --- a/src/tests/test_engine.rs +++ b/src/tests/test_engine.rs @@ -319,6 +319,11 @@ fn default_value12() -> TestResult { fail_test(r#"def foo [--x:int = "a"] { $x }"#, "default value not int") } +#[test] +fn default_value_expression() -> TestResult { + run_test(r#"def foo [x = ("foo" | str length)] { $x }; foo"#, "3") +} + #[test] fn loose_each() -> TestResult { run_test(r#"[[1, 2, 3], [4, 5, 6]] | each { $in.1 } | math sum"#, "7")