From 5d5088b5d537970679c8b5896f40f8667f777c43 Mon Sep 17 00:00:00 2001 From: Andrej Kolchin Date: Wed, 6 Dec 2023 21:46:37 +0000 Subject: [PATCH] Match `++=` capabilities with `++` (#11130) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow `++=` to work in all situations `++` does, namely for appending single elements: `$list ++= 1`. Resolve #11087 # Description Bring `++=` to parity with `++`. # User-Facing Changes It is now possible to do `$list ++= 1` (appending a single element). Similarly, this can be done: ```Nushell ~> mut a = [1] ~> $a ++= 2 ~> a ╭───┬───╮ │ 0 │ 1 │ │ 1 │ 2 │ ╰───┴───╯ ``` # Tests + Formatting Added two tests: - `commands::assignment::append_assign::append_assign_single_element` - `commands::assignment::append_assign::append_assign_to_single_element` --- .../commands/assignment/append_assign.rs | 76 +++++++------ crates/nu-parser/src/type_check.rs | 103 ++++++++++-------- 2 files changed, 98 insertions(+), 81 deletions(-) diff --git a/crates/nu-command/tests/commands/assignment/append_assign.rs b/crates/nu-command/tests/commands/assignment/append_assign.rs index 3a0e8c96bf..97abc8eb6f 100644 --- a/crates/nu-command/tests/commands/assignment/append_assign.rs +++ b/crates/nu-command/tests/commands/assignment/append_assign.rs @@ -5,16 +5,10 @@ fn append_assign_int() { let actual = nu!(r#" mut a = [1 2]; $a ++= [3 4]; - $a + $a == [1 2 3 4] "#); - let expected = nu!(r#" - [1 2 3 4] - "#); - - print!("{}", actual.out); - print!("{}", expected.out); - assert_eq!(actual.out, expected.out); + assert_eq!(actual.out, "true") } #[test] @@ -22,16 +16,10 @@ fn append_assign_string() { let actual = nu!(r#" mut a = [a b]; $a ++= [c d]; - $a + $a == [a b c d] "#); - let expected = nu!(r#" - [a b c d] - "#); - - print!("{}", actual.out); - print!("{}", expected.out); - assert_eq!(actual.out, expected.out); + assert_eq!(actual.out, "true") } #[test] @@ -39,16 +27,10 @@ fn append_assign_any() { let actual = nu!(r#" mut a = [1 2 a]; $a ++= [b 3]; - $a + $a == [1 2 a b 3] "#); - let expected = nu!(r#" - [1 2 a b 3] - "#); - - print!("{}", actual.out); - print!("{}", expected.out); - assert_eq!(actual.out, expected.out); + assert_eq!(actual.out, "true") } #[test] @@ -56,16 +38,10 @@ fn append_assign_both_empty() { let actual = nu!(r#" mut a = []; $a ++= []; - $a + $a == [] "#); - let expected = nu!(r#" - [] - "#); - - print!("{}", actual.out); - print!("{}", expected.out); - assert_eq!(actual.out, expected.out); + assert_eq!(actual.out, "true") } #[test] @@ -73,8 +49,40 @@ fn append_assign_type_mismatch() { let actual = nu!(r#" mut a = [1 2]; $a ++= [a]; - $a | to json -r; + $a == [1 2 "a"] "#); - assert_eq!(actual.out, r#"[1,2,"a"]"#); + assert_eq!(actual.out, "true") +} + +#[test] +fn append_assign_single_element() { + let actual = nu!(r#" + mut a = ["list" "and"]; + $a ++= "a single element"; + $a == ["list" "and" "a single element"] + "#); + + assert_eq!(actual.out, "true") +} + +#[test] +fn append_assign_to_single_element() { + let actual = nu!(r#" + mut a = "string"; + $a ++= ["and" "the" "list"]; + $a == ["string" "and" "the" "list"] + "#); + + assert_eq!(actual.out, "true") +} + +#[test] +fn append_assign_single_to_single() { + let actual = nu!(r#" + mut a = 1; + $a ++= "and a single element"; + "#); + + assert!(actual.err.contains("nu::parser::unsupported_operation")); } diff --git a/crates/nu-parser/src/type_check.rs b/crates/nu-parser/src/type_check.rs index 247a2c82ff..be355e087a 100644 --- a/crates/nu-parser/src/type_check.rs +++ b/crates/nu-parser/src/type_check.rs @@ -1,6 +1,6 @@ use nu_protocol::{ ast::{ - Bits, Block, Boolean, Comparison, Expr, Expression, Math, Operator, Pipeline, + Assignment, Bits, Block, Boolean, Comparison, Expr, Expression, Math, Operator, Pipeline, PipelineElement, }, engine::StateWorkingSet, @@ -130,52 +130,7 @@ pub fn math_result_type( ) } }, - Operator::Math(Math::Append) => match (&lhs.ty, &rhs.ty) { - (Type::List(a), Type::List(b)) => { - if a == b { - (Type::List(a.clone()), None) - } else { - (Type::List(Box::new(Type::Any)), None) - } - } - (Type::List(a), b) | (b, Type::List(a)) => { - if a == &Box::new(b.clone()) { - (Type::List(a.clone()), None) - } else { - (Type::List(Box::new(Type::Any)), None) - } - } - (Type::Table(a), Type::Table(_)) => (Type::Table(a.clone()), None), - (Type::String, Type::String) => (Type::String, None), - (Type::Binary, Type::Binary) => (Type::Binary, None), - (Type::Any, _) | (_, Type::Any) => (Type::Any, None), - (Type::Table(_) | Type::String | Type::Binary, _) => { - *op = Expression::garbage(op.span); - ( - Type::Any, - Some(ParseError::UnsupportedOperationRHS( - "append".into(), - op.span, - lhs.span, - lhs.ty.clone(), - rhs.span, - rhs.ty.clone(), - )), - ) - } - _ => { - *op = Expression::garbage(op.span); - ( - Type::Any, - Some(ParseError::UnsupportedOperationLHS( - "append".into(), - op.span, - lhs.span, - lhs.ty.clone(), - )), - ) - } - }, + Operator::Math(Math::Append) => check_append(lhs, rhs, op), Operator::Math(Math::Minus) => match (&lhs.ty, &rhs.ty) { (Type::Int, Type::Int) => (Type::Int, None), (Type::Float, Type::Int) => (Type::Float, None), @@ -924,6 +879,7 @@ pub fn math_result_type( ) } }, + Operator::Assignment(Assignment::AppendAssign) => check_append(lhs, rhs, op), Operator::Assignment(_) => match (&lhs.ty, &rhs.ty) { (x, y) if x == y => (Type::Nothing, None), (Type::Any, _) => (Type::Nothing, None), @@ -1076,3 +1032,56 @@ pub fn check_block_input_output(working_set: &StateWorkingSet, block: &Block) -> output_errors } + +fn check_append( + lhs: &Expression, + rhs: &Expression, + op: &mut Expression, +) -> (Type, Option) { + match (&lhs.ty, &rhs.ty) { + (Type::List(a), Type::List(b)) => { + if a == b { + (Type::List(a.clone()), None) + } else { + (Type::List(Box::new(Type::Any)), None) + } + } + (Type::List(a), b) | (b, Type::List(a)) => { + if a == &Box::new(b.clone()) { + (Type::List(a.clone()), None) + } else { + (Type::List(Box::new(Type::Any)), None) + } + } + (Type::Table(a), Type::Table(_)) => (Type::Table(a.clone()), None), + (Type::String, Type::String) => (Type::String, None), + (Type::Binary, Type::Binary) => (Type::Binary, None), + (Type::Any, _) | (_, Type::Any) => (Type::Any, None), + (Type::Table(_) | Type::String | Type::Binary, _) => { + *op = Expression::garbage(op.span); + ( + Type::Any, + Some(ParseError::UnsupportedOperationRHS( + "append".into(), + op.span, + lhs.span, + lhs.ty.clone(), + rhs.span, + rhs.ty.clone(), + )), + ) + } + _ => { + *op = Expression::garbage(op.span); + ( + Type::Any, + Some(ParseError::UnsupportedOperationLHS( + "append".into(), + op.span, + lhs.span, + lhs.ty.clone(), + )), + ) + } + } +}