forked from extern/nushell
improve error when name and parameters are not space-separated (#8958)
# Description closes #8934 this pr improves the diagnostic emitted when the name and parameters of either `def`, `def-env` or `extern` are not separated by a space ```nu Error: × no space between name and parameters ╭─[entry #1:1:1] 1 │ def err[] {} · ▲ · ╰── expected space ╰──── help: consider adding a space between the `def` command's name and its parameters ``` from ```nu Error: nu::parser::missing_positional × Missing required positional argument. ╭─[entry #1:1:1] 1 │ def err[] {} ╰──── help: Usage: def <def_name> <params> <body> ``` --------- Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com> Co-authored-by: Jelle Besseling <jelle@pingiun.com>
This commit is contained in:
mike
GitHub
parent
5e8754bd85
commit
a3bf2bff49
@ -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) ] {}");
|
||||
|
||||
@ -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<ParseError> {
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
@ -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,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -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")
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user