raise ParseError if assign to a non-variable or non-mutable-variable (#14405)

# Description
While reviewing #14388, I think we can make some improvement on parser.

For the following code:
```nushell
let a = 3
a = 10   # should be error
$a = 10 # another error
```
I think they can raise `ParseError`, so nushell doesn't need to move
forward compiling IR block.

# User-Facing Changes
```nushell
let a = 3
a = 10
```
Will raise parse error instead of compile error.

# Tests + Formatting
Added 1 test.
This commit is contained in:
Wind 2024-11-30 06:02:21 +08:00 committed by GitHub
parent dc9e8161d9
commit 817830940b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 51 additions and 0 deletions

View File

@ -148,3 +148,14 @@ fn def_should_not_mutate_mut() {
assert!(actual.err.contains("capture of mutable variable")); assert!(actual.err.contains("capture of mutable variable"));
assert!(!actual.status.success()) assert!(!actual.status.success())
} }
#[test]
fn assign_to_non_mut_variable_raises_parse_error() {
let actual = nu!("let x = 3; $x = 4");
assert!(actual
.err
.contains("parser::assignment_requires_mutable_variable"));
let actual = nu!("mut x = 3; x = 5");
assert!(actual.err.contains("parser::assignment_requires_variable"));
}

View File

@ -4988,6 +4988,20 @@ pub fn parse_assignment_expression(
// Parse the lhs and operator as usual for a math expression // Parse the lhs and operator as usual for a math expression
let mut lhs = parse_expression(working_set, lhs_spans); let mut lhs = parse_expression(working_set, lhs_spans);
// make sure that lhs is a mutable variable.
match &lhs.expr {
Expr::FullCellPath(p) => {
if let Expr::Var(var_id) = p.head.expr {
if var_id != nu_protocol::ENV_VARIABLE_ID
&& !working_set.get_variable(var_id).mutable
{
working_set.error(ParseError::AssignmentRequiresMutableVar(lhs.span))
}
}
}
_ => working_set.error(ParseError::AssignmentRequiresVar(lhs.span)),
}
let mut operator = parse_assignment_operator(working_set, op_span); let mut operator = parse_assignment_operator(working_set, op_span);
// Re-parse the right-hand side as a subexpression // Re-parse the right-hand side as a subexpression

View File

@ -515,6 +515,30 @@ pub enum ParseError {
help("To spread arguments, the command needs to define a multi-positional parameter in its signature, such as ...rest") help("To spread arguments, the command needs to define a multi-positional parameter in its signature, such as ...rest")
)] )]
UnexpectedSpreadArg(String, #[label = "unexpected spread argument"] Span), UnexpectedSpreadArg(String, #[label = "unexpected spread argument"] Span),
/// Invalid assignment left-hand side
///
/// ## Resolution
///
/// Assignment requires that you assign to a mutable variable or cell path.
#[error("Assignment to an immutable variable.")]
#[diagnostic(
code(nu::parser::assignment_requires_mutable_variable),
help("declare the variable with `mut`, or shadow it again with `let`")
)]
AssignmentRequiresMutableVar(#[label("needs to be a mutable variable")] Span),
/// Invalid assignment left-hand side
///
/// ## Resolution
///
/// Assignment requires that you assign to a variable or variable cell path.
#[error("Assignment operations require a variable.")]
#[diagnostic(
code(nu::parser::assignment_requires_variable),
help("try assigning to a variable or a cell path of a variable")
)]
AssignmentRequiresVar(#[label("needs to be a variable")] Span),
} }
impl ParseError { impl ParseError {
@ -603,6 +627,8 @@ impl ParseError {
ParseError::RedirectingBuiltinCommand(_, s, _) => *s, ParseError::RedirectingBuiltinCommand(_, s, _) => *s,
ParseError::UnexpectedSpreadArg(_, s) => *s, ParseError::UnexpectedSpreadArg(_, s) => *s,
ParseError::ExtraTokensAfterClosingDelimiter(s) => *s, ParseError::ExtraTokensAfterClosingDelimiter(s) => *s,
ParseError::AssignmentRequiresVar(s) => *s,
ParseError::AssignmentRequiresMutableVar(s) => *s,
} }
} }
} }