diff --git a/crates/nu-command/tests/commands/def.rs b/crates/nu-command/tests/commands/def.rs index 25bc392be..9fe38d571 100644 --- a/crates/nu-command/tests/commands/def.rs +++ b/crates/nu-command/tests/commands/def.rs @@ -37,6 +37,20 @@ param:string #My cool attractive param }) } +#[test] +fn def_errors_with_no_space_between_params_and_name_1() { + let actual = nu!("def test-command[] {}"); + + assert!(actual.err.contains("expected space")); +} + +#[test] +fn def_errors_with_no_space_between_params_and_name_2() { + let actual = nu!("def-env test-command() {}"); + + assert!(actual.err.contains("expected space")); +} + #[test] fn def_errors_with_multiple_short_flags() { let actual = nu!("def test-command [ --long(-l)(-o) ] {}"); diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index b6cecf229..35a3e4201 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1,3 +1,4 @@ +use itertools::Itertools; use log::trace; use nu_path::canonicalize_with; use nu_protocol::{ @@ -129,15 +130,24 @@ pub fn parse_def_predecl(working_set: &mut StateWorkingSet, spans: &[Span]) { let name = working_set.get_span_contents(spans[0]); // handle "export def" same as "def" - let (name, spans) = if name == b"export" && spans.len() >= 2 { + let (decl_name, spans) = if name == b"export" && spans.len() >= 2 { (working_set.get_span_contents(spans[1]), &spans[1..]) } else { (name, spans) }; - if (name == b"def" || name == b"def-env") && spans.len() >= 4 { + if (decl_name == b"def" || decl_name == b"def-env") && spans.len() >= 4 { let starting_error_count = working_set.parse_errors.len(); - let name = working_set.get_span_contents(spans[1]); + let name = if let Some(err) = detect_params_in_name( + working_set, + spans[1], + String::from_utf8_lossy(decl_name).as_ref(), + ) { + working_set.error(err); + return; + } else { + working_set.get_span_contents(spans[1]) + }; let name = trim_quotes(name); let name = String::from_utf8_lossy(name).to_string(); @@ -170,7 +180,7 @@ pub fn parse_def_predecl(working_set: &mut StateWorkingSet, spans: &[Span]) { working_set.error(ParseError::DuplicateCommandDef(spans[1])); } } - } else if name == b"extern" && spans.len() >= 3 { + } else if decl_name == b"extern" && spans.len() >= 3 { let name_expr = parse_string(working_set, spans[1]); let name = name_expr.as_string(); @@ -347,6 +357,18 @@ pub fn parse_def( Some(decl_id) => { working_set.enter_scope(); let (command_spans, rest_spans) = spans.split_at(split_id); + + if let Some(name_span) = rest_spans.get(0) { + if let Some(err) = detect_params_in_name( + working_set, + *name_span, + String::from_utf8_lossy(&def_call).as_ref(), + ) { + working_set.error(err); + return garbage_pipeline(spans); + } + } + let starting_error_count = working_set.parse_errors.len(); let ParsedInternalCall { call, output } = parse_internal_call(working_set, span(command_spans), rest_spans, decl_id); @@ -509,6 +531,17 @@ pub fn parse_extern( let (command_spans, rest_spans) = spans.split_at(split_id); + if let Some(name_span) = rest_spans.get(0) { + if let Some(err) = detect_params_in_name( + working_set, + *name_span, + String::from_utf8_lossy(&extern_call).as_ref(), + ) { + working_set.error(err); + return garbage_pipeline(spans); + } + } + let ParsedInternalCall { call, .. } = parse_internal_call(working_set, span(command_spans), rest_spans, decl_id); working_set.exit_scope(); @@ -3440,3 +3473,35 @@ pub fn find_in_dirs( find_in_dirs_with_id(filename, working_set, cwd, dirs_var_name) .or_else(|| find_in_dirs_old(filename, working_set, cwd, dirs_var_name)) } + +fn detect_params_in_name( + working_set: &StateWorkingSet, + name_span: Span, + decl_name: &str, +) -> Option { + let name = working_set.get_span_contents(name_span); + + let extract_span = |delim: u8| { + // it is okay to unwrap because we know the slice contains the byte + let (idx, _) = name + .iter() + .find_position(|c| **c == delim) + .unwrap_or((name.len(), &b' ')); + let param_span = Span::new(name_span.start + idx - 1, name_span.start + idx - 1); + let error = ParseError::LabeledErrorWithHelp{ + error: "no space between name and parameters".into(), + label: "expected space".into(), + help: format!("consider adding a space between the `{decl_name}` command's name and its parameters"), + span: param_span, + }; + Some(error) + }; + + if name.contains(&b'[') { + extract_span(b'[') + } else if name.contains(&b'(') { + extract_span(b'(') + } else { + None + } +} diff --git a/crates/nu-protocol/src/parse_error.rs b/crates/nu-protocol/src/parse_error.rs index 5c6278ed2..3736d31ec 100644 --- a/crates/nu-protocol/src/parse_error.rs +++ b/crates/nu-protocol/src/parse_error.rs @@ -445,6 +445,16 @@ pub enum ParseError { #[error("{0}")] #[diagnostic()] LabeledError(String, String, #[label("{1}")] Span), + + #[error("{error}")] + #[diagnostic(help("{help}"))] + LabeledErrorWithHelp { + error: String, + label: String, + help: String, + #[label("{label}")] + span: Span, + }, } impl ParseError { @@ -524,6 +534,7 @@ impl ParseError { ParseError::UnknownOperator(_, _, s) => *s, ParseError::InvalidLiteral(_, _, s) => *s, ParseError::NotAConstant(s) => *s, + ParseError::LabeledErrorWithHelp { span: s, .. } => *s, } } } diff --git a/src/tests/test_parser.rs b/src/tests/test_parser.rs index 1899efebf..ab5b4f0a4 100644 --- a/src/tests/test_parser.rs +++ b/src/tests/test_parser.rs @@ -525,3 +525,13 @@ register $file "; fail_test(input, "expected string, found int") } + +#[test] +fn extern_errors_with_no_space_between_params_and_name_1() -> TestResult { + fail_test("extern cmd[]", "expected space") +} + +#[test] +fn extern_errors_with_no_space_between_params_and_name_2() -> TestResult { + fail_test("extern cmd(--flag)", "expected space") +}