From 2ccb91dc6a6331594a01fb680db581e01291c88e Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Sat, 26 Nov 2022 17:02:37 +0100 Subject: [PATCH] Add logical `xor` operator (#7242) We already have the binary `bit-xor` and the shortcircuiting logical `or`(`||`) and `and`(`&&`). This introduces `xor` as a compact form for both brevity and clarity. You can express the operation through `not`/`and`/`or` with a slight risk of introducing bugs through typos. Operator precedence `and` > `xor` > `or` Added logic and precedence tests. --- crates/nu-engine/src/eval.rs | 4 ++++ crates/nu-parser/src/parser.rs | 1 + crates/nu-parser/src/type_check.rs | 4 +++- crates/nu-protocol/src/ast/expression.rs | 1 + crates/nu-protocol/src/ast/operator.rs | 2 ++ crates/nu-protocol/src/value/mod.rs | 19 +++++++++++++++++++ src/tests/test_engine.rs | 5 +++++ src/tests/test_math.rs | 10 ++++++++++ src/tests/test_parser.rs | 12 ++++++++++++ 9 files changed, 57 insertions(+), 1 deletion(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 01b1b436b..33f656f27 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -404,6 +404,10 @@ pub fn eval_expression( lhs.or(op_span, &rhs, expr.span) } } + Boolean::Xor => { + let rhs = eval_expression(engine_state, stack, rhs)?; + lhs.xor(op_span, &rhs, expr.span) + } } } Operator::Math(math) => { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 3c5465173..364409762 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -4502,6 +4502,7 @@ pub fn parse_operator( b"ends-with" => Operator::Comparison(Comparison::EndsWith), b"&&" | b"and" => Operator::Boolean(Boolean::And), b"||" | b"or" => Operator::Boolean(Boolean::Or), + b"xor" => Operator::Boolean(Boolean::Xor), b"**" => Operator::Math(Math::Pow), _ => { return ( diff --git a/crates/nu-parser/src/type_check.rs b/crates/nu-parser/src/type_check.rs index 75978db1f..9655321f2 100644 --- a/crates/nu-parser/src/type_check.rs +++ b/crates/nu-parser/src/type_check.rs @@ -244,7 +244,9 @@ pub fn math_result_type( ) } }, - Operator::Boolean(Boolean::And) | Operator::Boolean(Boolean::Or) => { + Operator::Boolean(Boolean::And) + | Operator::Boolean(Boolean::Or) + | Operator::Boolean(Boolean::Xor) => { match (&lhs.ty, &rhs.ty) { (Type::Bool, Type::Bool) => (Type::Bool, None), diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index 04b954641..2d224f99c 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -54,6 +54,7 @@ impl Expression { Operator::Bits(Bits::BitXor) => 70, Operator::Bits(Bits::BitOr) => 60, Operator::Boolean(Boolean::And) => 50, + Operator::Boolean(Boolean::Xor) => 45, Operator::Boolean(Boolean::Or) => 40, Operator::Assignment(_) => 10, } diff --git a/crates/nu-protocol/src/ast/operator.rs b/crates/nu-protocol/src/ast/operator.rs index 9c0286dd0..2b6450744 100644 --- a/crates/nu-protocol/src/ast/operator.rs +++ b/crates/nu-protocol/src/ast/operator.rs @@ -35,6 +35,7 @@ pub enum Math { pub enum Boolean { And, Or, + Xor, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -94,6 +95,7 @@ impl Display for Operator { Operator::Math(Math::Pow) => write!(f, "**"), Operator::Boolean(Boolean::And) => write!(f, "&&"), Operator::Boolean(Boolean::Or) => write!(f, "||"), + Operator::Boolean(Boolean::Xor) => write!(f, "xor"), Operator::Bits(Bits::BitOr) => write!(f, "bit-or"), Operator::Bits(Bits::BitXor) => write!(f, "bit-xor"), Operator::Bits(Bits::BitAnd) => write!(f, "bit-and"), diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 691146a79..b2bea6d71 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -2865,6 +2865,25 @@ impl Value { } } + pub fn xor(&self, op: Span, rhs: &Value, span: Span) -> Result { + match (self, rhs) { + (Value::Bool { val: lhs, .. }, Value::Bool { val: rhs, .. }) => Ok(Value::Bool { + val: (*lhs && !*rhs) || (!*lhs && *rhs), + span, + }), + (Value::CustomValue { val: lhs, span }, rhs) => { + lhs.operation(*span, Operator::Boolean(Boolean::Xor), op, rhs) + } + _ => Err(ShellError::OperatorMismatch { + op_span: op, + lhs_ty: self.get_type(), + lhs_span: self.span()?, + rhs_ty: rhs.get_type(), + rhs_span: rhs.span()?, + }), + } + } + pub fn pow(&self, op: Span, rhs: &Value, span: Span) -> Result { match (self, rhs) { (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => { diff --git a/src/tests/test_engine.rs b/src/tests/test_engine.rs index 15f5463cf..b026ea93a 100644 --- a/src/tests/test_engine.rs +++ b/src/tests/test_engine.rs @@ -246,6 +246,11 @@ fn shortcircuiting_or() -> TestResult { run_test(r#"true || (5 / 0; false)"#, "true") } +#[test] +fn nonshortcircuiting_xor() -> TestResult { + run_test(r#"true xor (print "hello"; false) | ignore"#, "hello") +} + #[test] fn open_ended_range() -> TestResult { run_test(r#"1.. | first 100000 | length"#, "100000") diff --git a/src/tests/test_math.rs b/src/tests/test_math.rs index 2f2550282..8dee151be 100644 --- a/src/tests/test_math.rs +++ b/src/tests/test_math.rs @@ -55,6 +55,16 @@ fn or() -> TestResult { run_test("true || false", "true") } +#[test] +fn xor_1() -> TestResult { + run_test("false xor true", "true") +} + +#[test] +fn xor_2() -> TestResult { + run_test("true xor true", "false") +} + #[test] fn bit_xor() -> TestResult { run_test("4 bit-xor 4", "0") diff --git a/src/tests/test_parser.rs b/src/tests/test_parser.rs index 46a4f31d2..25b56eb27 100644 --- a/src/tests/test_parser.rs +++ b/src/tests/test_parser.rs @@ -430,3 +430,15 @@ fn date_literal() -> TestResult { fn and_and_or() -> TestResult { run_test(r#"true and false or true"#, "true") } + +#[test] +fn and_and_xor() -> TestResult { + // Assumes the precedence NOT > AND > XOR > OR + run_test(r#"true and true xor true and false"#, "true") +} + +#[test] +fn or_and_xor() -> TestResult { + // Assumes the precedence NOT > AND > XOR > OR + run_test(r#"true or false xor true or false"#, "true") +}