From a4bd51a11d8c6653ac4f5c599b1ea1445496c502 Mon Sep 17 00:00:00 2001 From: 132ikl <132@ikl.sh> Date: Fri, 4 Jul 2025 02:48:49 -0400 Subject: [PATCH] Fix type checking for assignment operators (#16107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Rel: #14429, #16079 Finishes up a TODO in the assignment type checking. - For regular assignment operations (only applies to `mut`), type checking is now done using `type_compatible` (which is what `let` uses) - This allows some mutable assignments to work which weren't allowed before Before: ```nushell let x: glob = "" # => ok, no error mut x: glob = ""; $x = "" # => Error: nu::parser::operator_incompatible_types # => # => × Types 'glob' and 'string' are not compatible for the '=' operator. # => ╭─[entry #6:1:19] # => 1 │ mut x: glob = ""; $x = "" # => · ─┬ ┬ ─┬ # => · │ │ ╰── string # => · │ ╰── does not operate between 'glob' and 'string' # => · ╰── glob # => ╰──── let x: number = 1 # ok, no error mut x: number = 1; $x = 2 # => Error: nu::parser::operator_incompatible_types # => # => × Types 'number' and 'int' are not compatible for the '=' operator. # => ╭─[source:1:20] # => 1 │ mut x: number = 1; $x = 2 # => · ─┬ ┬ ┬ # => · │ │ ╰── int # => · │ ╰── does not operate between 'number' and 'int' # => · ╰── number # => ╰──── ``` After: ```nushell let x: glob = "" # ok, no error (same as before) mut x: glob = ""; $x = "" # ok, no error let x: number = 1 # ok, no error (same as before) mut x: number = 1; $x = 2 # ok, no error ``` - Properly type check compound operations. First checks if the operation (eg. `+` for `+=`) type checks successfully, and then checks if the assignment type checks successfully (also using `type_compatible`) - This fixes some issues where the "long version" of a compound assignment operator would error, but the compound assignment operator itself would not Before: ```nushell mut x = 1; $x = $x / 2 # => Error: nu::parser::operator_incompatible_types # => # => × Types 'int' and 'float' are not compatible for the '=' operator. # => ╭─[entry #15:1:12] # => 1 │ mut x = 1; $x = $x / 2 # => · ─┬ ┬ ───┬── # => · │ │ ╰── float # => · │ ╰── does not operate between 'int' and 'float' # => · ╰── int # => ╰──── mut x = 1; $x /= 2 # uh oh, no error... mut x = (date now); $x = $x - 2019-05-10 # => Error: nu::parser::operator_incompatible_types # => # => × Types 'datetime' and 'duration' are not compatible for the '=' operator. # => ╭─[entry #1:1:21] # => 1 │ mut x = (date now); $x = $x - 2019-05-10 # => · ─┬ ┬ ───────┬─────── # => · │ │ ╰── duration # => · │ ╰── does not operate between 'datetime' and 'duration' # => · ╰── datetime # => ╰──── mut x = (date now); $x -= 2019-05-10 # uh oh, no error... (the result of this is a duration, not a datetime) ``` After: ```nushell mut x = 1; $x = $x / 2 # => Error: nu::parser::operator_incompatible_types # => # => × Types 'int' and 'float' are not compatible for the '=' operator. # => ╭─[entry #5:1:12] # => 1 │ mut x = 1; $x = $x / 2 # => · ─┬ ┬ ───┬── # => · │ │ ╰── float # => · │ ╰── does not operate between 'int' and 'float' # => · ╰── int # => ╰──── mut x = (date now); $x -= 2019-05-10 # => Error: nu::parser::operator_incompatible_types # => # => × Types 'datetime' and 'datetime' are not compatible for the '-=' operator. # => ╭─[entry #11:1:21] # => 1 │ mut x = (date now); $x -= 2019-05-10 # => · ─┬ ─┬ ─────┬──── # => · │ │ ╰── datetime # => · │ ╰── does not operate between 'datetime' and 'datetime' # => · ╰── datetime # => ╰──── # => help: The result type of this operation is not compatible with the type of the variable. ``` This is technically a breaking change if you relied on the old behavior (for example, there was a test that broke after this change because it relied on `/=` improperly type checking) # User-Facing Changes * Mutable assignment operations now use the same type checking rules as normal assignments * For example, `$x = 123` now uses the same type checking rules as `let x = 123` or `mut x = 123` * Compound assignment operations now type check using the same rules as the operation they use * Assignment errors will also now highlight the invalid assignment operator in red # Tests + Formatting Adds some tests for the examples given above # After Submitting N/A --- crates/nu-command/tests/commands/mut_.rs | 30 +++++++++- crates/nu-parser/src/type_check.rs | 76 +++++++++++++++++++----- 2 files changed, 90 insertions(+), 16 deletions(-) diff --git a/crates/nu-command/tests/commands/mut_.rs b/crates/nu-command/tests/commands/mut_.rs index dca545e17a..455f6aa5cb 100644 --- a/crates/nu-command/tests/commands/mut_.rs +++ b/crates/nu-command/tests/commands/mut_.rs @@ -67,11 +67,39 @@ fn mut_multiply_assign() { #[test] fn mut_divide_assign() { - let actual = nu!("mut y = 8; $y /= 2; $y"); + let actual = nu!("mut y: number = 8; $y /= 2; $y"); assert_eq!(actual.out, "4.0"); } +#[test] +fn mut_divide_assign_should_error() { + let actual = nu!("mut y = 8; $y /= 2; $y"); + + assert!(actual.err.contains("parser::operator_incompatible_types")); +} + +#[test] +fn mut_subtract_assign_should_error() { + let actual = nu!("mut x = (date now); $x -= 2019-05-10"); + + assert!(actual.err.contains("parser::operator_incompatible_types")); +} + +#[test] +fn mut_assign_number() { + let actual = nu!("mut x: number = 1; $x = 2.0; $x"); + + assert_eq!(actual.out, "2.0"); +} + +#[test] +fn mut_assign_glob() { + let actual = nu!(r#"mut x: glob = ""; $x = "meow"; $x"#); + + assert_eq!(actual.out, "meow"); +} + #[test] fn mut_path_insert() { let actual = nu!("mut y = {abc: 123}; $y.abc = 456; $y.abc"); diff --git a/crates/nu-parser/src/type_check.rs b/crates/nu-parser/src/type_check.rs index b55a3e146b..ca964798ef 100644 --- a/crates/nu-parser/src/type_check.rs +++ b/crates/nu-parser/src/type_check.rs @@ -1,6 +1,6 @@ use nu_protocol::{ ParseError, Span, Type, - ast::{Block, Comparison, Expr, Expression, Math, Operator, Pipeline, Range}, + ast::{Assignment, Block, Comparison, Expr, Expression, Math, Operator, Pipeline, Range}, engine::StateWorkingSet, }; @@ -721,16 +721,27 @@ pub fn math_result_type( type_error(operator, op.span, lhs, rhs, |ty| matches!(ty, Type::Int)) } }, - // TODO: fix this - Operator::Assignment(_) => match (&lhs.ty, &rhs.ty) { - (x, y) if x == y => (Type::Nothing, None), - (Type::Any, _) => (Type::Nothing, None), - (_, Type::Any) => (Type::Nothing, None), - (Type::List(_) | Type::Table(_), Type::List(_) | Type::Table(_)) => { - (Type::Nothing, None) - } - _ => { - let err = ParseError::OperatorIncompatibleTypes { + Operator::Assignment(Assignment::AddAssign) => { + compound_assignment_result_type(working_set, lhs, op, rhs, operator, Math::Add) + } + Operator::Assignment(Assignment::ConcatenateAssign) => { + compound_assignment_result_type(working_set, lhs, op, rhs, operator, Math::Concatenate) + } + Operator::Assignment(Assignment::DivideAssign) => { + compound_assignment_result_type(working_set, lhs, op, rhs, operator, Math::Divide) + } + Operator::Assignment(Assignment::MultiplyAssign) => { + compound_assignment_result_type(working_set, lhs, op, rhs, operator, Math::Multiply) + } + Operator::Assignment(Assignment::SubtractAssign) => { + compound_assignment_result_type(working_set, lhs, op, rhs, operator, Math::Subtract) + } + Operator::Assignment(Assignment::Assign) => { + let err = if type_compatible(&lhs.ty, &rhs.ty) { + None + } else { + *op = Expression::garbage(working_set, op.span); + Some(ParseError::OperatorIncompatibleTypes { op: operator.as_str(), lhs: lhs.ty.clone(), rhs: rhs.ty.clone(), @@ -738,10 +749,10 @@ pub fn math_result_type( lhs_span: lhs.span, rhs_span: rhs.span, help: None, - }; - (Type::Nothing, Some(err)) - } - }, + }) + }; + (Type::Nothing, err) + } } } @@ -915,3 +926,38 @@ pub fn check_range_types(working_set: &mut StateWorkingSet, range: &mut Range) { _ => (), } } + +/// Get the result type for a compound assignment operator +fn compound_assignment_result_type( + working_set: &mut StateWorkingSet, + lhs: &mut Expression, + op: &mut Expression, + rhs: &mut Expression, + operator: Operator, + operation: Math, +) -> (Type, Option) { + let math_expr = Expr::Operator(Operator::Math(operation)); + let mut math_op = Expression::new(working_set, math_expr, op.span, Type::Any); + match math_result_type(working_set, lhs, &mut math_op, rhs) { + // There was a type error in the math expression, so propagate it + (_, Some(err)) => (Type::Any, Some(err)), + // Operation type check okay, check regular assignment + (ty, None) if type_compatible(&lhs.ty, &ty) => (Type::Nothing, None), + // The math expression is fine, but we can't store the result back into the variable due to type mismatch + (_, None) => { + *op = Expression::garbage(working_set, op.span); + let err = ParseError::OperatorIncompatibleTypes { + op: operator.as_str(), + lhs: lhs.ty.clone(), + rhs: rhs.ty.clone(), + op_span: op.span, + lhs_span: lhs.span, + rhs_span: rhs.span, + help: Some( + "The result type of this operation is not compatible with the type of the variable.", + ), + }; + (Type::Nothing, Some(err)) + } + } +}