diff --git a/crates/nu-command/tests/commands/let_.rs b/crates/nu-command/tests/commands/let_.rs index e05d34d258..d81fb9401c 100644 --- a/crates/nu-command/tests/commands/let_.rs +++ b/crates/nu-command/tests/commands/let_.rs @@ -1,10 +1,11 @@ use nu_test_support::nu; +use rstest::rstest; -#[test] -fn let_name_builtin_var() { - let actual = nu!("let in = 3"); - - assert!(actual +#[rstest] +#[case("let in = 3")] +#[case("let in: int = 3")] +fn let_name_builtin_var(#[case] assignment: &str) { + assert!(nu!(assignment) .err .contains("'in' is the name of a builtin Nushell variable")); } diff --git a/crates/nu-command/tests/commands/mut_.rs b/crates/nu-command/tests/commands/mut_.rs index 7078cd1df1..e66348f8f2 100644 --- a/crates/nu-command/tests/commands/mut_.rs +++ b/crates/nu-command/tests/commands/mut_.rs @@ -1,4 +1,5 @@ use nu_test_support::nu; +use rstest::rstest; #[test] fn mut_variable() { @@ -7,11 +8,11 @@ fn mut_variable() { assert_eq!(actual.out, "4"); } -#[test] -fn mut_name_builtin_var() { - let actual = nu!("mut in = 3"); - - assert!(actual +#[rstest] +#[case("mut in = 3")] +#[case("mut in: int = 3")] +fn mut_name_builtin_var(#[case] assignment: &str) { + assert!(nu!(assignment) .err .contains("'in' is the name of a builtin Nushell variable")); } diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 8ca5249bd2..3292405cab 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1204,7 +1204,7 @@ pub fn parse_export_in_block( match full_name { "export alias" => parse_alias(working_set, lite_command, None), "export def" => parse_def(working_set, lite_command, None).0, - "export const" => parse_const(working_set, &lite_command.parts[1..]), + "export const" => parse_const(working_set, &lite_command.parts[1..]).0, "export use" => parse_use(working_set, lite_command, None).0, "export module" => parse_module(working_set, lite_command, None).0, "export extern" => parse_extern(working_set, lite_command, None), @@ -1514,7 +1514,7 @@ pub fn parse_export_in_module( result } b"const" => { - let pipeline = parse_const(working_set, &spans[1..]); + let (pipeline, var_name_span) = parse_const(working_set, &spans[1..]); let export_const_decl_id = if let Some(id) = working_set.find_decl(b"export const") { id @@ -1542,8 +1542,8 @@ pub fn parse_export_in_module( let mut result = vec![]; - if let Some(var_name_span) = spans.get(2) { - let var_name = working_set.get_span_contents(*var_name_span); + if let Some(var_name_span) = var_name_span { + let var_name = working_set.get_span_contents(var_name_span); let var_name = trim_quotes(var_name); if let Some(var_id) = working_set.find_variable(var_name) { @@ -1762,7 +1762,7 @@ pub fn parse_module_block( } b"const" => block .pipelines - .push(parse_const(working_set, &command.parts)), + .push(parse_const(working_set, &command.parts).0), b"extern" => block .pipelines .push(parse_extern(working_set, command, None)), @@ -3154,7 +3154,8 @@ pub fn parse_let(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline garbage_pipeline(working_set, spans) } -pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline { +/// Additionally returns a span encompassing the variable name, if successful. +pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> (Pipeline, Option) { trace!("parsing: const"); // JT: Disabling check_name because it doesn't work with optional types in the declaration @@ -3271,28 +3272,37 @@ pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipelin let call = Box::new(Call { decl_id, head: spans[0], - arguments: vec![Argument::Positional(lvalue), Argument::Positional(rvalue)], + arguments: vec![ + Argument::Positional(lvalue.clone()), + Argument::Positional(rvalue), + ], parser_info: HashMap::new(), }); - return Pipeline::from_vec(vec![Expression::new( - working_set, - Expr::Call(call), - Span::concat(spans), - Type::Any, - )]); + return ( + Pipeline::from_vec(vec![Expression::new( + working_set, + Expr::Call(call), + Span::concat(spans), + Type::Any, + )]), + Some(lvalue.span), + ); } } } let ParsedInternalCall { call, output } = parse_internal_call(working_set, spans[0], &spans[1..], decl_id); - return Pipeline::from_vec(vec![Expression::new( - working_set, - Expr::Call(call), - Span::concat(spans), - output, - )]); + return ( + Pipeline::from_vec(vec![Expression::new( + working_set, + Expr::Call(call), + Span::concat(spans), + output, + )]), + None, + ); } else { working_set.error(ParseError::UnknownState( "internal error: let or const statements not found in core language".into(), @@ -3305,7 +3315,7 @@ pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipelin Span::concat(spans), )); - garbage_pipeline(working_set, spans) + (garbage_pipeline(working_set, spans), None) } pub fn parse_mut(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index d2ed11188a..c0bf4f59ff 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -3113,7 +3113,8 @@ pub fn parse_var_with_opt_type( spans_idx: &mut usize, mutable: bool, ) -> (Expression, Option) { - let bytes = working_set.get_span_contents(spans[*spans_idx]).to_vec(); + let name_span = spans[*spans_idx]; + let bytes = working_set.get_span_contents(name_span).to_vec(); if bytes.contains(&b' ') || bytes.contains(&b'"') @@ -3125,9 +3126,11 @@ pub fn parse_var_with_opt_type( } if bytes.ends_with(b":") { + let name_span = Span::new(name_span.start, name_span.end - 1); + let var_name = bytes[0..(bytes.len() - 1)].to_vec(); + // We end with colon, so the next span should be the type if *spans_idx + 1 < spans.len() { - let span_beginning = *spans_idx; *spans_idx += 1; // signature like record is broken into multiple spans due to // whitespaces. Collect the rest into one span and work on it @@ -3144,8 +3147,6 @@ pub fn parse_var_with_opt_type( let ty = parse_type(working_set, &type_bytes, tokens[0].span); *spans_idx = spans.len() - 1; - let var_name = bytes[0..(bytes.len() - 1)].to_vec(); - if !is_variable(&var_name) { working_set.error(ParseError::Expected( "valid variable name", @@ -3157,17 +3158,10 @@ pub fn parse_var_with_opt_type( let id = working_set.add_variable(var_name, spans[*spans_idx - 1], ty.clone(), mutable); ( - Expression::new( - working_set, - Expr::VarDecl(id), - Span::concat(&spans[span_beginning..*spans_idx + 1]), - ty.clone(), - ), + Expression::new(working_set, Expr::VarDecl(id), name_span, ty.clone()), Some(ty), ) } else { - let var_name = bytes[0..(bytes.len() - 1)].to_vec(); - if !is_variable(&var_name) { working_set.error(ParseError::Expected( "valid variable name", @@ -5615,7 +5609,7 @@ pub fn parse_builtin_commands( .parts_including_redirection() .collect::>(), ), - b"const" => parse_const(working_set, &lite_command.parts), + b"const" => parse_const(working_set, &lite_command.parts).0, b"mut" => parse_mut( working_set, &lite_command diff --git a/tests/repl/test_modules.rs b/tests/repl/test_modules.rs index 289efd291a..c2fc6b8dc2 100644 --- a/tests/repl/test_modules.rs +++ b/tests/repl/test_modules.rs @@ -117,6 +117,10 @@ fn export_consts() -> TestResult { run_test( r#"module spam { export const b = 3; }; use spam b; $b"#, "3", + )?; + run_test( + r#"module spam { export const b: int = 3; }; use spam b; $b"#, + "3", ) }