From fb72da0e82d418582afa59e1c69f1b6826c6668e Mon Sep 17 00:00:00 2001 From: mike <98623181+1Kinoti@users.noreply.github.com> Date: Thu, 20 Apr 2023 20:44:31 +0300 Subject: [PATCH] unify the `*-BuiltinVar` parser errors (#8944) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description this pr condenses `MutBuiltinVar`, `LetBuiltinVar` and `ConstBuiltinVar` into one error: ```nu Error: nu::parser::name_is_builtin_var × `in` used as variable name. ╭─[entry #69:1:1] 1 │ let in = 420 · ─┬ · ╰── already a builtin variable ╰──── help: 'in' is the name of a builtin Nushell variable and cannot be used as a variable name ``` it also fixes this case which was previously not handled ```nu let $nu = 420 # this variable would have been 'lost' ``` --- crates/nu-command/tests/commands/let_.rs | 2 +- crates/nu-command/tests/commands/mut_.rs | 28 ++++++++++++++++++++++++ crates/nu-parser/src/parse_keywords.rs | 11 ++++------ crates/nu-protocol/src/parse_error.rs | 28 ++++++------------------ 4 files changed, 40 insertions(+), 29 deletions(-) diff --git a/crates/nu-command/tests/commands/let_.rs b/crates/nu-command/tests/commands/let_.rs index ec207a666..749b96a4d 100644 --- a/crates/nu-command/tests/commands/let_.rs +++ b/crates/nu-command/tests/commands/let_.rs @@ -1,7 +1,7 @@ use nu_test_support::{nu, pipeline}; #[test] -fn let_parse_error() { +fn let_name_builtin_var() { let actual = nu!( cwd: ".", pipeline( r#" diff --git a/crates/nu-command/tests/commands/mut_.rs b/crates/nu-command/tests/commands/mut_.rs index 99f2ba6dc..bc274c891 100644 --- a/crates/nu-command/tests/commands/mut_.rs +++ b/crates/nu-command/tests/commands/mut_.rs @@ -12,6 +12,34 @@ fn mut_variable() { assert_eq!(actual.out, "4"); } +#[test] +fn mut_name_builtin_var() { + let actual = nu!( + cwd: ".", pipeline( + r#" + mut in = 3 + "# + )); + + assert!(actual + .err + .contains("'in' is the name of a builtin Nushell variable")); +} + +#[test] +fn mut_name_builtin_var_with_dollar() { + let actual = nu!( + cwd: ".", pipeline( + r#" + mut $env = 3 + "# + )); + + assert!(actual + .err + .contains("'env' is the name of a builtin Nushell variable")) +} + #[test] fn mut_variable_in_loop() { let actual = nu!( diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 2704e3f07..a85f8160b 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -2439,15 +2439,11 @@ pub fn parse_let_or_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> let var_name = String::from_utf8_lossy(working_set.get_span_contents(lvalue.span)) + .trim_start_matches('$') .to_string(); if ["in", "nu", "env", "nothing"].contains(&var_name.as_str()) { - if is_const { - working_set - .error(ParseError::ConstBuiltinVar(var_name, lvalue.span)) - } else { - working_set.error(ParseError::LetBuiltinVar(var_name, lvalue.span)) - }; + working_set.error(ParseError::NameIsBuiltinVar(var_name, lvalue.span)) } let var_id = lvalue.as_var(); @@ -2551,10 +2547,11 @@ pub fn parse_mut(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline let var_name = String::from_utf8_lossy(working_set.get_span_contents(lvalue.span)) + .trim_start_matches('$') .to_string(); if ["in", "nu", "env", "nothing"].contains(&var_name.as_str()) { - working_set.error(ParseError::MutBuiltinVar(var_name, lvalue.span)); + working_set.error(ParseError::NameIsBuiltinVar(var_name, lvalue.span)) } let var_id = lvalue.as_var(); diff --git a/crates/nu-protocol/src/parse_error.rs b/crates/nu-protocol/src/parse_error.rs index 68f1f070e..467092e91 100644 --- a/crates/nu-protocol/src/parse_error.rs +++ b/crates/nu-protocol/src/parse_error.rs @@ -142,26 +142,14 @@ pub enum ParseError { )] AssignInPipeline(String, String, String, #[label("'{0}' in pipeline")] Span), - #[error("Let used with builtin variable name.")] + #[error("`{0}` used as variable name.")] #[diagnostic( - code(nu::parser::let_builtin_var), - help("'{0}' is the name of a builtin Nushell variable. `let` cannot assign to it.") + code(nu::parser::name_is_builtin_var), + help( + "'{0}' is the name of a builtin Nushell variable and cannot be used as a variable name" + ) )] - LetBuiltinVar(String, #[label("already a builtin variable")] Span), - - #[error("Const used with builtin variable name.")] - #[diagnostic( - code(nu::parser::let_builtin_var), - help("'{0}' is the name of a builtin Nushell variable. `const` cannot assign to it.") - )] - ConstBuiltinVar(String, #[label("already a builtin variable")] Span), - - #[error("Mut used with builtin variable name.")] - #[diagnostic( - code(nu::parser::let_builtin_var), - help("'{0}' is the name of a builtin Nushell variable. `mut` cannot assign to it.") - )] - MutBuiltinVar(String, #[label("already a builtin variable")] Span), + NameIsBuiltinVar(String, #[label("already a builtin variable")] Span), #[error("Incorrect value")] #[diagnostic(code(nu::parser::incorrect_value), help("{2}"))] @@ -445,9 +433,7 @@ impl ParseError { ParseError::CantAliasExpression(_, s) => *s, ParseError::BuiltinCommandInPipeline(_, s) => *s, ParseError::AssignInPipeline(_, _, _, s) => *s, - ParseError::LetBuiltinVar(_, s) => *s, - ParseError::MutBuiltinVar(_, s) => *s, - ParseError::ConstBuiltinVar(_, s) => *s, + ParseError::NameIsBuiltinVar(_, s) => *s, ParseError::CaptureOfMutableVar(s) => *s, ParseError::IncorrectValue(_, s, _) => *s, ParseError::MultipleRestParams(s) => *s,