From 0fe2884397b15d89a75a7dc04cbc34c6b1b9b7b8 Mon Sep 17 00:00:00 2001
From: mike <98623181+1Kinoti@users.noreply.github.com>
Date: Fri, 20 Jan 2023 02:11:48 +0300
Subject: [PATCH] add dedicated `const in pipeline`, `const builtin var` errors
(#7784)
# Description
Currently `let` and `const` share error handling, and this might lead to
confusing error messages
This PR adds dedicated errors to `const`
---
crates/nu-parser/src/errors.rs | 22 +++++++++++++++++++++-
crates/nu-parser/src/parse_keywords.rs | 7 +++++--
crates/nu-parser/src/parser.rs | 25 ++++++++++++++++++++++++-
3 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/crates/nu-parser/src/errors.rs b/crates/nu-parser/src/errors.rs
index 52232e632b..92ac4597a6 100644
--- a/crates/nu-parser/src/errors.rs
+++ b/crates/nu-parser/src/errors.rs
@@ -128,6 +128,16 @@ pub enum ParseError {
)]
LetInPipeline(String, String, #[label("let in pipeline")] Span),
+ #[error("Const statement used in pipeline.")]
+ #[diagnostic(
+ code(nu::parser::unexpected_keyword),
+ url(docsrs),
+ help(
+ "Assigning '{0}' to '{1}' does not produce a value to be piped. If the pipeline result is meant to be assigned to '{1}', use 'const {1} = ({0} | ...)'."
+ )
+ )]
+ ConstInPipeline(String, String, #[label("const in pipeline")] Span),
+
#[error("Mut statement used in pipeline.")]
#[diagnostic(
code(nu::parser::unexpected_keyword),
@@ -136,7 +146,7 @@ pub enum ParseError {
"Assigning '{0}' to '{1}' does not produce a value to be piped. If the pipeline result is meant to be assigned to '{1}', use 'mut {1} = ({0} | ...)'."
)
)]
- MutInPipeline(String, String, #[label("let in pipeline")] Span),
+ MutInPipeline(String, String, #[label("mut in pipeline")] Span),
#[error("Let used with builtin variable name.")]
#[diagnostic(
@@ -146,6 +156,14 @@ pub enum ParseError {
)]
LetBuiltinVar(String, #[label("already a builtin variable")] Span),
+ #[error("Const used with builtin variable name.")]
+ #[diagnostic(
+ code(nu::parser::let_builtin_var),
+ url(docsrs),
+ 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),
@@ -420,8 +438,10 @@ impl ParseError {
ParseError::BuiltinCommandInPipeline(_, s) => *s,
ParseError::LetInPipeline(_, _, s) => *s,
ParseError::MutInPipeline(_, _, s) => *s,
+ ParseError::ConstInPipeline(_, _, s) => *s,
ParseError::LetBuiltinVar(_, s) => *s,
ParseError::MutBuiltinVar(_, s) => *s,
+ ParseError::ConstBuiltinVar(_, s) => *s,
ParseError::CaptureOfMutableVar(s) => *s,
ParseError::IncorrectValue(_, s, _) => *s,
ParseError::MultipleRestParams(s) => *s,
diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs
index faa4e27bd3..93577f4491 100644
--- a/crates/nu-parser/src/parse_keywords.rs
+++ b/crates/nu-parser/src/parse_keywords.rs
@@ -2842,8 +2842,11 @@ pub fn parse_let_or_const(
.to_string();
if ["in", "nu", "env", "nothing"].contains(&var_name.as_str()) {
- error =
- error.or(Some(ParseError::LetBuiltinVar(var_name, lvalue.span)));
+ error = if is_const {
+ error.or(Some(ParseError::ConstBuiltinVar(var_name, lvalue.span)))
+ } else {
+ error.or(Some(ParseError::LetBuiltinVar(var_name, lvalue.span)))
+ };
}
let var_id = lvalue.as_var();
diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs
index 778a926b32..88370cebd9 100644
--- a/crates/nu-parser/src/parser.rs
+++ b/crates/nu-parser/src/parser.rs
@@ -5001,7 +5001,7 @@ pub fn parse_expression(
.0,
Some(ParseError::BuiltinCommandInPipeline("for".into(), spans[0])),
),
- b"let" | b"const" => (
+ b"let" => (
parse_call(
working_set,
&spans[pos..],
@@ -5024,6 +5024,29 @@ pub fn parse_expression(
spans[0],
)),
),
+ b"const" => (
+ parse_call(
+ working_set,
+ &spans[pos..],
+ spans[0],
+ expand_aliases_denylist,
+ is_subexpression,
+ )
+ .0,
+ Some(ParseError::ConstInPipeline(
+ String::from_utf8_lossy(match spans.len() {
+ 1 | 2 | 3 => b"value",
+ _ => working_set.get_span_contents(spans[3]),
+ })
+ .to_string(),
+ String::from_utf8_lossy(match spans.len() {
+ 1 => b"variable",
+ _ => working_set.get_span_contents(spans[1]),
+ })
+ .to_string(),
+ spans[0],
+ )),
+ ),
b"mut" => (
parse_call(
working_set,