From 8783cf0138af4f683a4b2fe8cb1376649f74a6ef Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 00:53:28 +0200 Subject: [PATCH 01/14] Add basic `in` operator support --- crates/nu-engine/src/eval.rs | 1 + crates/nu-parser/src/type_check.rs | 21 +++++++++++++++ crates/nu-protocol/src/value/mod.rs | 40 ++++++++++++++++++++++++++++- src/tests.rs | 34 ++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 1a44d6fffb..86acdc628d 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -190,6 +190,7 @@ pub fn eval_expression( Operator::GreaterThanOrEqual => lhs.gte(op_span, &rhs), Operator::Equal => lhs.eq(op_span, &rhs), Operator::NotEqual => lhs.ne(op_span, &rhs), + Operator::In => lhs.r#in(op_span, &rhs), x => Err(ShellError::UnsupportedOperator(x, op_span)), } } diff --git a/crates/nu-parser/src/type_check.rs b/crates/nu-parser/src/type_check.rs index 03ce14c6e6..f41e6787d6 100644 --- a/crates/nu-parser/src/type_check.rs +++ b/crates/nu-parser/src/type_check.rs @@ -273,6 +273,27 @@ pub fn math_result_type( ) } }, + Operator::In => match (&lhs.ty, &rhs.ty) { + (t, Type::List(u)) if type_compatible(t, u) => (Type::Bool, None), + (Type::Int | Type::Float, Type::Range) => (Type::Bool, None), + (Type::String, Type::String) => (Type::Bool, None), + + (Type::Unknown, _) => (Type::Bool, None), + (_, Type::Unknown) => (Type::Bool, None), + _ => { + *op = Expression::garbage(op.span); + ( + Type::Unknown, + Some(ParseError::UnsupportedOperation( + op.span, + lhs.span, + lhs.ty.clone(), + rhs.span, + rhs.ty.clone(), + )), + ) + } + }, _ => { *op = Expression::garbage(op.span); diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 334790ab27..4ac2d563eb 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -13,7 +13,7 @@ pub use unit::*; use std::fmt::Debug; -use crate::ast::{CellPath, PathMember}; +use crate::ast::{CellPath, PathMember, RangeInclusion}; use crate::{span, BlockId, Span, Type}; use crate::ShellError; @@ -1014,6 +1014,44 @@ impl Value { }), } } + + pub fn r#in(&self, op: Span, rhs: &Value) -> Result { + let span = span(&[self.span(), rhs.span()]); + + match (self, rhs) { + (lhs, Value::Range { val: rhs, .. }) => Ok(Value::Bool { + val: lhs + .gte(Span::unknown(), &rhs.from) + .map_or(false, |v| v.is_true()) + && match rhs.inclusion { + RangeInclusion::Inclusive => lhs + .lte(Span::unknown(), &rhs.to) + .map_or(false, |v| v.is_true()), + RangeInclusion::RightExclusive => lhs + .lt(Span::unknown(), &rhs.to) + .map_or(false, |v| v.is_true()), + }, + span, + }), + (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => Ok(Value::Bool { + val: rhs.contains(lhs), + span, + }), + (lhs, Value::List { vals: rhs, .. }) => Ok(Value::Bool { + val: rhs + .iter() + .any(|x| lhs.eq(Span::unknown(), x).map_or(false, |v| v.is_true())), + span, + }), + _ => Err(ShellError::OperatorMismatch { + op_span: op, + lhs_ty: self.get_type(), + lhs_span: self.span(), + rhs_ty: rhs.get_type(), + rhs_span: rhs.span(), + }), + } + } } /// Format a duration in nanoseconds into a string diff --git a/src/tests.rs b/src/tests.rs index 4c2c96191d..5d628b22b0 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -569,3 +569,37 @@ fn split_column() -> TestResult { fn for_loops() -> TestResult { run_test(r#"(for x in [1, 2, 3] { $x + 10 }).1"#, "12") } + +fn type_in_list_of_this_type() -> TestResult { + run_test(r#"42 in [41 42 43]"#, "true") +} + +#[test] +fn type_in_list_of_non_this_type() -> TestResult { + fail_test(r#"'hello' in [41 42 43]"#, "mismatched for operation") +} + +#[test] +fn string_in_string() -> TestResult { + run_test(r#"'z' in 'abc'"#, "false") +} + +#[test] +fn non_string_in_string() -> TestResult { + fail_test(r#"42 in 'abc'"#, "mismatched for operation") +} + +#[test] +fn int_in_range() -> TestResult { + run_test(r#"1 in -4..9.42"#, "true") +} + +#[test] +fn int_in_exclusive_range() -> TestResult { + run_test(r#"3 in 0..<3"#, "false") +} + +#[test] +fn non_number_in_range() -> TestResult { + fail_test(r#"'a' in 1..3"#, "mismatched for operation") +} From d3bc096d477a74c7c1ead00d2b42d79bc379b4c6 Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 01:18:17 +0200 Subject: [PATCH 02/14] Handle reverse ranges This is really ugly and should be refactored. --- crates/nu-protocol/src/value/mod.rs | 43 ++++++++++++++++++++++------- src/tests.rs | 7 ++++- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 4ac2d563eb..9aa98f0c2f 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -1020,17 +1020,40 @@ impl Value { match (self, rhs) { (lhs, Value::Range { val: rhs, .. }) => Ok(Value::Bool { - val: lhs - .gte(Span::unknown(), &rhs.from) + val: if rhs + .incr + .gt( + Span::unknown(), + &Value::Int { + val: 0, + span: Span::unknown(), + }, + ) .map_or(false, |v| v.is_true()) - && match rhs.inclusion { - RangeInclusion::Inclusive => lhs - .lte(Span::unknown(), &rhs.to) - .map_or(false, |v| v.is_true()), - RangeInclusion::RightExclusive => lhs - .lt(Span::unknown(), &rhs.to) - .map_or(false, |v| v.is_true()), - }, + { + lhs.gte(Span::unknown(), &rhs.from) + .map_or(false, |v| v.is_true()) + && match rhs.inclusion { + RangeInclusion::Inclusive => lhs + .lte(Span::unknown(), &rhs.to) + .map_or(false, |v| v.is_true()), + RangeInclusion::RightExclusive => lhs + .lt(Span::unknown(), &rhs.to) + .map_or(false, |v| v.is_true()), + } + } else { + lhs.lte(Span::unknown(), &rhs.from) + .map_or(false, |v| v.is_true()) + && match rhs.inclusion { + RangeInclusion::Inclusive => lhs + .gte(Span::unknown(), &rhs.to) + .map_or(false, |v| v.is_true()), + RangeInclusion::RightExclusive => lhs + .gt(Span::unknown(), &rhs.to) + .map_or(false, |v| v.is_true()), + } + }, + span, }), (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => Ok(Value::Bool { diff --git a/src/tests.rs b/src/tests.rs index 5d628b22b0..9acb6913d0 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -590,10 +590,15 @@ fn non_string_in_string() -> TestResult { } #[test] -fn int_in_range() -> TestResult { +fn int_in_inc_range() -> TestResult { run_test(r#"1 in -4..9.42"#, "true") } +#[test] +fn int_in_dec_range() -> TestResult { + run_test(r#"1 in 9.42..-4"#, "true") +} + #[test] fn int_in_exclusive_range() -> TestResult { run_test(r#"3 in 0..<3"#, "false") From 7db6b876abfc782badf3f0b360d2e396f4eb13f7 Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 01:31:18 +0200 Subject: [PATCH 03/14] Simplify `Result` comparaison using `matches!` --- crates/nu-protocol/src/value/mod.rs | 59 ++++++++++++++++------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 9aa98f0c2f..01aa379ab5 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -1020,38 +1020,43 @@ impl Value { match (self, rhs) { (lhs, Value::Range { val: rhs, .. }) => Ok(Value::Bool { - val: if rhs - .incr - .gt( + val: if matches!( + rhs.incr.gt( Span::unknown(), &Value::Int { val: 0, - span: Span::unknown(), - }, - ) - .map_or(false, |v| v.is_true()) - { - lhs.gte(Span::unknown(), &rhs.from) - .map_or(false, |v| v.is_true()) - && match rhs.inclusion { - RangeInclusion::Inclusive => lhs - .lte(Span::unknown(), &rhs.to) - .map_or(false, |v| v.is_true()), - RangeInclusion::RightExclusive => lhs - .lt(Span::unknown(), &rhs.to) - .map_or(false, |v| v.is_true()), + span: Span::unknown() } + ), + Ok(Value::Bool { val: true, .. }) + ) { + matches!( + lhs.gte(Span::unknown(), &rhs.from), + Ok(Value::Bool { val: true, .. }) + ) && match rhs.inclusion { + RangeInclusion::Inclusive => matches!( + lhs.lte(Span::unknown(), &rhs.to), + Ok(Value::Bool { val: true, .. }) + ), + RangeInclusion::RightExclusive => matches!( + lhs.lt(Span::unknown(), &rhs.to), + Ok(Value::Bool { val: true, .. }) + ), + } } else { - lhs.lte(Span::unknown(), &rhs.from) - .map_or(false, |v| v.is_true()) - && match rhs.inclusion { - RangeInclusion::Inclusive => lhs - .gte(Span::unknown(), &rhs.to) - .map_or(false, |v| v.is_true()), - RangeInclusion::RightExclusive => lhs - .gt(Span::unknown(), &rhs.to) - .map_or(false, |v| v.is_true()), - } + matches!( + lhs.lte(Span::unknown(), &rhs.from), + Ok(Value::Bool { val: true, .. }) + ) && match rhs.inclusion { + RangeInclusion::Inclusive => matches!( + lhs.gte(Span::unknown(), &rhs.to), + Ok(Value::Bool { val: true, .. }) + ), + RangeInclusion::RightExclusive => matches!( + lhs.gt(Span::unknown(), &rhs.to), + Ok(Value::Bool { val: true, .. }) + ), + } }, span, From 7f06d6144f050595b40e84ef193379785b22c5a8 Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 11:23:32 +0200 Subject: [PATCH 04/14] Support `in` operator for record and value stream --- crates/nu-parser/src/type_check.rs | 1 + crates/nu-protocol/src/value/mod.rs | 22 +++++++++++++++++++--- src/tests.rs | 23 +++++++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/crates/nu-parser/src/type_check.rs b/crates/nu-parser/src/type_check.rs index f41e6787d6..d2f972cabf 100644 --- a/crates/nu-parser/src/type_check.rs +++ b/crates/nu-parser/src/type_check.rs @@ -277,6 +277,7 @@ pub fn math_result_type( (t, Type::List(u)) if type_compatible(t, u) => (Type::Bool, None), (Type::Int | Type::Float, Type::Range) => (Type::Bool, None), (Type::String, Type::String) => (Type::Bool, None), + (Type::String, Type::Record(_, _)) => (Type::Bool, None), (Type::Unknown, _) => (Type::Bool, None), (_, Type::Unknown) => (Type::Bool, None), diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 01aa379ab5..9ccb623daf 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -1066,9 +1066,25 @@ impl Value { span, }), (lhs, Value::List { vals: rhs, .. }) => Ok(Value::Bool { - val: rhs - .iter() - .any(|x| lhs.eq(Span::unknown(), x).map_or(false, |v| v.is_true())), + val: rhs.iter().any(|x| { + matches!( + lhs.eq(Span::unknown(), x), + Ok(Value::Bool { val: true, .. }) + ) + }), + span, + }), + (Value::String { val: lhs, .. }, Value::Record { cols: rhs, .. }) => Ok(Value::Bool { + val: rhs.contains(lhs), + span, + }), + (lhs, Value::Stream { stream: rhs, .. }) => Ok(Value::Bool { + val: rhs.clone().any(|x| { + matches!( + lhs.eq(Span::unknown(), &x), + Ok(Value::Bool { val: true, .. }) + ) + }), span, }), _ => Err(ShellError::OperatorMismatch { diff --git a/src/tests.rs b/src/tests.rs index 9acb6913d0..2de6466fbe 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -608,3 +608,26 @@ fn int_in_exclusive_range() -> TestResult { fn non_number_in_range() -> TestResult { fail_test(r#"'a' in 1..3"#, "mismatched for operation") } + +#[test] +fn string_in_record() -> TestResult { + run_test(r#""a" in ('{ "a": 13, "b": 14 }' | from json)"#, "true") +} + +#[test] +fn non_string_in_record() -> TestResult { + fail_test( + r#"4 in ('{ "a": 13, "b": 14 }' | from json)"#, + "mismatch during operation", + ) +} + +#[test] +fn string_in_valuestream() -> TestResult { + run_test( + r#" + 'Hello' in ("Hello + World" | lines)"#, + "true", + ) +} From 29cbcb845950d00ae6b8e7956f93901071ae0b71 Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 12:27:47 +0200 Subject: [PATCH 05/14] Implement `RangeIterator::contains` --- crates/nu-protocol/src/value/range.rs | 44 +++++++++++++++++++-------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index 369ff03e87..ff41c6499f 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -152,6 +152,36 @@ impl RangeIterator { incr: range.incr, } } + + pub fn contains(&self, x: &Value) -> bool { + let ordering_against_curr = compare_numbers(x, &self.curr); + let ordering_against_end = compare_numbers(x, &self.end); + + match (ordering_against_curr, ordering_against_end) { + (Some(Ordering::Greater | Ordering::Equal), Some(Ordering::Less)) if self.moves_up => { + true + } + (Some(Ordering::Less | Ordering::Equal), Some(Ordering::Greater)) if !self.moves_up => { + true + } + (Some(_), Some(Ordering::Equal)) if self.is_end_inclusive => true, + (_, _) => false, + } + } +} + +fn compare_numbers(val: &Value, other: &Value) -> Option { + match (val, other) { + (Value::Int { val, .. }, Value::Int { val: other, .. }) => Some(val.cmp(other)), + (Value::Float { val, .. }, Value::Float { val: other, .. }) => compare_floats(*val, *other), + (Value::Float { val, .. }, Value::Int { val: other, .. }) => { + compare_floats(*val, *other as f64) + } + (Value::Int { val, .. }, Value::Float { val: other, .. }) => { + compare_floats(*val as f64, *other) + } + _ => None, + } } // Compare two floating point numbers. The decision interval for equality is dynamically scaled @@ -176,19 +206,7 @@ impl Iterator for RangeIterator { let ordering = if matches!(self.end, Value::Nothing { .. }) { Some(Ordering::Less) } else { - match (&self.curr, &self.end) { - (Value::Int { val: curr, .. }, Value::Int { val: end, .. }) => Some(curr.cmp(end)), - (Value::Float { val: curr, .. }, Value::Float { val: end, .. }) => { - compare_floats(*curr, *end) - } - (Value::Float { val: curr, .. }, Value::Int { val: end, .. }) => { - compare_floats(*curr, *end as f64) - } - (Value::Int { val: curr, .. }, Value::Float { val: end, .. }) => { - compare_floats(*curr as f64, *end) - } - _ => None, - } + compare_numbers(&self.curr, &self.end) }; let ordering = if let Some(ord) = ordering { From d1f0740765d1c5a1c3f2702eeea10f28e8713f57 Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 12:28:21 +0200 Subject: [PATCH 06/14] Refactor `in` operator for `Range` --- crates/nu-protocol/src/value/mod.rs | 42 +++-------------------------- 1 file changed, 3 insertions(+), 39 deletions(-) diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 9ccb623daf..4b4e0b6e06 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -1020,45 +1020,8 @@ impl Value { match (self, rhs) { (lhs, Value::Range { val: rhs, .. }) => Ok(Value::Bool { - val: if matches!( - rhs.incr.gt( - Span::unknown(), - &Value::Int { - val: 0, - span: Span::unknown() - } - ), - Ok(Value::Bool { val: true, .. }) - ) { - matches!( - lhs.gte(Span::unknown(), &rhs.from), - Ok(Value::Bool { val: true, .. }) - ) && match rhs.inclusion { - RangeInclusion::Inclusive => matches!( - lhs.lte(Span::unknown(), &rhs.to), - Ok(Value::Bool { val: true, .. }) - ), - RangeInclusion::RightExclusive => matches!( - lhs.lt(Span::unknown(), &rhs.to), - Ok(Value::Bool { val: true, .. }) - ), - } - } else { - matches!( - lhs.lte(Span::unknown(), &rhs.from), - Ok(Value::Bool { val: true, .. }) - ) && match rhs.inclusion { - RangeInclusion::Inclusive => matches!( - lhs.gte(Span::unknown(), &rhs.to), - Ok(Value::Bool { val: true, .. }) - ), - RangeInclusion::RightExclusive => matches!( - lhs.gt(Span::unknown(), &rhs.to), - Ok(Value::Bool { val: true, .. }) - ), - } - }, - + // TODO(@arthur-targaryen): Not sure about this clone. + val: rhs.clone().into_iter().contains(lhs), span, }), (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => Ok(Value::Bool { @@ -1079,6 +1042,7 @@ impl Value { span, }), (lhs, Value::Stream { stream: rhs, .. }) => Ok(Value::Bool { + // TODO(@arthur-targaryen): Not sure about this clone too. val: rhs.clone().any(|x| { matches!( lhs.eq(Span::unknown(), &x), From 357b9ccaa96f47611a7711ecb67525a2d465d594 Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 12:29:59 +0200 Subject: [PATCH 07/14] Remove unused import --- crates/nu-protocol/src/value/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 4b4e0b6e06..d23d6b1aa2 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -13,7 +13,7 @@ pub use unit::*; use std::fmt::Debug; -use crate::ast::{CellPath, PathMember, RangeInclusion}; +use crate::ast::{CellPath, PathMember}; use crate::{span, BlockId, Span, Type}; use crate::ShellError; From 4235cf11911b7e9ed1bf0766483dce737c84a035 Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 17:32:56 +0200 Subject: [PATCH 08/14] Implement and use `PartialOrd` for `Value` --- crates/nu-protocol/src/value/mod.rs | 371 ++++++-------------------- crates/nu-protocol/src/value/range.rs | 68 ++--- 2 files changed, 108 insertions(+), 331 deletions(-) diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index d23d6b1aa2..0ad12a9864 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize}; pub use stream::*; pub use unit::*; -use std::fmt::Debug; +use std::{cmp::Ordering, fmt::Debug}; use crate::ast::{CellPath, PathMember}; use crate::{span, BlockId, Span, Type}; @@ -441,60 +441,59 @@ impl Value { } } -impl PartialEq for Value { - fn eq(&self, other: &Self) -> bool { +impl PartialOrd for Value { + fn partial_cmp(&self, other: &Self) -> Option { + // Compare two floating point numbers. The decision interval for equality is dynamically + // scaled as the value being compared increases in magnitude. + fn compare_floats(val: f64, other: f64) -> Option { + let prec = f64::EPSILON.max(val.abs() * f64::EPSILON); + + if (other - val).abs() < prec { + return Some(Ordering::Equal); + } + + val.partial_cmp(&other) + } + match (self, other) { - (Value::Bool { val: lhs, .. }, Value::Bool { val: rhs, .. }) => lhs == rhs, - (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => lhs == rhs, - (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => lhs == rhs, - (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => lhs == rhs, - (Value::Block { val: b1, .. }, Value::Block { val: b2, .. }) => b1 == b2, - (Value::List { vals: vals_lhs, .. }, Value::List { vals: vals_rhs, .. }) => { - for (lhs, rhs) in vals_lhs.iter().zip(vals_rhs) { - if lhs != rhs { - return false; - } - } - - true + (Value::Bool { val: lhs, .. }, Value::Bool { val: rhs, .. }) => lhs.partial_cmp(rhs), + (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => lhs.partial_cmp(rhs), + (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => { + compare_floats(*lhs, *rhs) + } + (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => { + lhs.partial_cmp(rhs) + } + (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => { + compare_floats(*lhs as f64, *rhs) + } + (Value::Float { val: lhs, .. }, Value::Int { val: rhs, .. }) => { + compare_floats(*lhs, *rhs as f64) + } + (Value::Duration { val: lhs, .. }, Value::Duration { val: rhs, .. }) => { + lhs.partial_cmp(rhs) + } + (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => { + lhs.partial_cmp(rhs) + } + (Value::Block { val: b1, .. }, Value::Block { val: b2, .. }) if b1 == b2 => { + Some(Ordering::Equal) + } + (Value::List { vals: lhs, .. }, Value::List { vals: rhs, .. }) if lhs == rhs => { + Some(Ordering::Equal) } ( Value::Record { - cols: cols_lhs, - vals: vals_lhs, + vals: lhs, + cols: lhs_headers, .. }, Value::Record { - cols: cols_rhs, - vals: vals_rhs, + vals: rhs, + cols: rhs_headers, .. }, - ) => { - if cols_lhs != cols_rhs { - return false; - } - - for (lhs, rhs) in vals_lhs.iter().zip(vals_rhs) { - if lhs != rhs { - return false; - } - } - - true - } - ( - Value::Stream { - stream: stream_lhs, .. - }, - Value::Stream { - stream: stream_rhs, .. - }, - ) => { - let vals_lhs: Vec = stream_lhs.clone().collect(); - let vals_rhs: Vec = stream_rhs.clone().collect(); - - vals_lhs == vals_rhs - } + ) if lhs_headers == rhs_headers && lhs == rhs => Some(Ordering::Equal), // Note: This may look a bit strange, but a Stream is still just a List, // it just happens to be in an iterator form instead of a concrete form. If the contained // values are the same then it should be treated as equal @@ -510,7 +509,7 @@ impl PartialEq for Value { let vals_rhs: Vec = stream_rhs.clone().into_iter().into_value_stream().collect(); - vals_lhs == vals_rhs + vals_lhs.partial_cmp(&vals_rhs) } // Note: This may look a bit strange, but a Stream is still just a List, // it just happens to be in an iterator form instead of a concrete form. If the contained @@ -527,13 +526,22 @@ impl PartialEq for Value { stream_lhs.clone().into_iter().into_value_stream().collect(); let vals_rhs: Vec = stream_rhs.clone().collect(); - vals_lhs == vals_rhs + vals_lhs.partial_cmp(&vals_rhs) } - _ => false, + (Value::Stream { stream: lhs, .. }, Value::Stream { stream: rhs, .. }) => { + lhs.clone().partial_cmp(rhs.clone()) + } + (_, _) => None, } } } +impl PartialEq for Value { + fn eq(&self, other: &Self) -> bool { + self.partial_cmp(other).map_or(false, Ordering::is_eq) + } +} + impl Value { pub fn add(&self, op: Span, rhs: &Value) -> Result { let span = span(&[self.span(), rhs.span()]); @@ -717,37 +725,12 @@ impl Value { pub fn lt(&self, op: Span, rhs: &Value) -> Result { let span = span(&[self.span(), rhs.span()]); - match (self, rhs) { - (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: lhs < rhs, + match self.partial_cmp(rhs) { + Some(ordering) => Ok(Value::Bool { + val: matches!(ordering, Ordering::Less), span, }), - (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: (*lhs as f64) < *rhs, - span, - }), - (Value::Float { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: *lhs < *rhs as f64, - span, - }), - (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: lhs < rhs, - span, - }), - (Value::Duration { val: lhs, .. }, Value::Duration { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs < rhs, - span, - }) - } - (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs < rhs, - span, - }) - } - - _ => Err(ShellError::OperatorMismatch { + None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), lhs_span: self.span(), @@ -759,36 +742,12 @@ impl Value { pub fn lte(&self, op: Span, rhs: &Value) -> Result { let span = span(&[self.span(), rhs.span()]); - match (self, rhs) { - (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: lhs <= rhs, + match self.partial_cmp(rhs) { + Some(ordering) => Ok(Value::Bool { + val: matches!(ordering, Ordering::Less | Ordering::Equal), span, }), - (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: (*lhs as f64) <= *rhs, - span, - }), - (Value::Float { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: *lhs <= *rhs as f64, - span, - }), - (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: lhs <= rhs, - span, - }), - (Value::Duration { val: lhs, .. }, Value::Duration { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs <= rhs, - span, - }) - } - (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs <= rhs, - span, - }) - } - _ => Err(ShellError::OperatorMismatch { + None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), lhs_span: self.span(), @@ -800,36 +759,12 @@ impl Value { pub fn gt(&self, op: Span, rhs: &Value) -> Result { let span = span(&[self.span(), rhs.span()]); - match (self, rhs) { - (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: lhs > rhs, + match self.partial_cmp(rhs) { + Some(ordering) => Ok(Value::Bool { + val: matches!(ordering, Ordering::Greater), span, }), - (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: (*lhs as f64) > *rhs, - span, - }), - (Value::Float { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: *lhs > *rhs as f64, - span, - }), - (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: lhs > rhs, - span, - }), - (Value::Duration { val: lhs, .. }, Value::Duration { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs > rhs, - span, - }) - } - (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs > rhs, - span, - }) - } - _ => Err(ShellError::OperatorMismatch { + None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), lhs_span: self.span(), @@ -841,36 +776,12 @@ impl Value { pub fn gte(&self, op: Span, rhs: &Value) -> Result { let span = span(&[self.span(), rhs.span()]); - match (self, rhs) { - (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: lhs >= rhs, + match self.partial_cmp(rhs) { + Some(ordering) => Ok(Value::Bool { + val: matches!(ordering, Ordering::Greater | Ordering::Equal), span, }), - (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: (*lhs as f64) >= *rhs, - span, - }), - (Value::Float { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: *lhs >= *rhs as f64, - span, - }), - (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: lhs >= rhs, - span, - }), - (Value::Duration { val: lhs, .. }, Value::Duration { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs >= rhs, - span, - }) - } - (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs >= rhs, - span, - }) - } - _ => Err(ShellError::OperatorMismatch { + None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), lhs_span: self.span(), @@ -882,62 +793,12 @@ impl Value { pub fn eq(&self, op: Span, rhs: &Value) -> Result { let span = span(&[self.span(), rhs.span()]); - match (self, rhs) { - (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: lhs == rhs, + match self.partial_cmp(rhs) { + Some(ordering) => Ok(Value::Bool { + val: matches!(ordering, Ordering::Equal), span, }), - (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => Ok(Value::Bool { - val: lhs == rhs, - span, - }), - // FIXME: these should consider machine epsilon - (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: (*lhs as f64) == *rhs, - span, - }), - // FIXME: these should consider machine epsilon - (Value::Float { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: *lhs == *rhs as f64, - span, - }), - // FIXME: these should consider machine epsilon - (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: lhs == rhs, - span, - }), - (Value::Duration { val: lhs, .. }, Value::Duration { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs == rhs, - span, - }) - } - (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs == rhs, - span, - }) - } - (Value::List { vals: lhs, .. }, Value::List { vals: rhs, .. }) => Ok(Value::Bool { - val: lhs == rhs, - span, - }), - ( - Value::Record { - vals: lhs, - cols: lhs_headers, - .. - }, - Value::Record { - vals: rhs, - cols: rhs_headers, - .. - }, - ) => Ok(Value::Bool { - val: lhs_headers == rhs_headers && lhs == rhs, - span, - }), - _ => Err(ShellError::OperatorMismatch { + None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), lhs_span: self.span(), @@ -949,63 +810,12 @@ impl Value { pub fn ne(&self, op: Span, rhs: &Value) -> Result { let span = span(&[self.span(), rhs.span()]); - match (self, rhs) { - (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: lhs != rhs, + match self.partial_cmp(rhs) { + Some(ordering) => Ok(Value::Bool { + val: !matches!(ordering, Ordering::Less), span, }), - (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => Ok(Value::Bool { - val: lhs != rhs, - span, - }), - // FIXME: these should consider machine epsilon - (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: (*lhs as f64) != *rhs, - span, - }), - // FIXME: these should consider machine epsilon - (Value::Float { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool { - val: *lhs != *rhs as f64, - span, - }), - // FIXME: these should consider machine epsilon - (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool { - val: lhs != rhs, - span, - }), - (Value::Duration { val: lhs, .. }, Value::Duration { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs != rhs, - span, - }) - } - (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => { - Ok(Value::Bool { - val: lhs != rhs, - span, - }) - } - (Value::List { vals: lhs, .. }, Value::List { vals: rhs, .. }) => Ok(Value::Bool { - val: lhs != rhs, - span, - }), - ( - Value::Record { - vals: lhs, - cols: lhs_headers, - .. - }, - Value::Record { - vals: rhs, - cols: rhs_headers, - .. - }, - ) => Ok(Value::Bool { - val: lhs_headers != rhs_headers || lhs != rhs, - span, - }), - - _ => Err(ShellError::OperatorMismatch { + None => Err(ShellError::OperatorMismatch { op_span: op, lhs_ty: self.get_type(), lhs_span: self.span(), @@ -1020,8 +830,7 @@ impl Value { match (self, rhs) { (lhs, Value::Range { val: rhs, .. }) => Ok(Value::Bool { - // TODO(@arthur-targaryen): Not sure about this clone. - val: rhs.clone().into_iter().contains(lhs), + val: rhs.contains(lhs), span, }), (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => Ok(Value::Bool { @@ -1029,12 +838,7 @@ impl Value { span, }), (lhs, Value::List { vals: rhs, .. }) => Ok(Value::Bool { - val: rhs.iter().any(|x| { - matches!( - lhs.eq(Span::unknown(), x), - Ok(Value::Bool { val: true, .. }) - ) - }), + val: rhs.contains(lhs), span, }), (Value::String { val: lhs, .. }, Value::Record { cols: rhs, .. }) => Ok(Value::Bool { @@ -1042,13 +846,8 @@ impl Value { span, }), (lhs, Value::Stream { stream: rhs, .. }) => Ok(Value::Bool { - // TODO(@arthur-targaryen): Not sure about this clone too. - val: rhs.clone().any(|x| { - matches!( - lhs.eq(Span::unknown(), &x), - Ok(Value::Bool { val: true, .. }) - ) - }), + // TODO(@arthur-targaryen): Not sure about this clone. + val: rhs.clone().any(|x| lhs == &x), span, }), _ => Err(ShellError::OperatorMismatch { diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index ff41c6499f..f414dd0dce 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -103,6 +103,23 @@ impl Range { inclusion: operator.inclusion, }) } + + pub fn moves_up(&self) -> bool { + self.from <= self.to + } + + pub fn is_end_inclusive(&self) -> bool { + matches!(self.inclusion, RangeInclusion::Inclusive) + } + + pub fn contains(&self, item: &Value) -> bool { + match (item.partial_cmp(&self.from), item.partial_cmp(&self.to)) { + (Some(Ordering::Greater | Ordering::Equal), Some(Ordering::Less)) => self.moves_up(), + (Some(Ordering::Less | Ordering::Equal), Some(Ordering::Greater)) => self.moves_up(), + (Some(_), Some(Ordering::Equal)) => self.is_end_inclusive(), + (_, _) => false, + } + } } impl IntoIterator for Range { @@ -129,6 +146,9 @@ pub struct RangeIterator { impl RangeIterator { pub fn new(range: Range, span: Span) -> RangeIterator { + let moves_up = range.moves_up(); + let is_end_inclusive = range.is_end_inclusive(); + let start = match range.from { Value::Nothing { .. } => Value::Int { val: 0, span }, x => x, @@ -143,57 +163,15 @@ impl RangeIterator { }; RangeIterator { - moves_up: matches!(start.lte(span, &end), Ok(Value::Bool { val: true, .. })), + moves_up, curr: start, end, span, - is_end_inclusive: matches!(range.inclusion, RangeInclusion::Inclusive), + is_end_inclusive, done: false, incr: range.incr, } } - - pub fn contains(&self, x: &Value) -> bool { - let ordering_against_curr = compare_numbers(x, &self.curr); - let ordering_against_end = compare_numbers(x, &self.end); - - match (ordering_against_curr, ordering_against_end) { - (Some(Ordering::Greater | Ordering::Equal), Some(Ordering::Less)) if self.moves_up => { - true - } - (Some(Ordering::Less | Ordering::Equal), Some(Ordering::Greater)) if !self.moves_up => { - true - } - (Some(_), Some(Ordering::Equal)) if self.is_end_inclusive => true, - (_, _) => false, - } - } -} - -fn compare_numbers(val: &Value, other: &Value) -> Option { - match (val, other) { - (Value::Int { val, .. }, Value::Int { val: other, .. }) => Some(val.cmp(other)), - (Value::Float { val, .. }, Value::Float { val: other, .. }) => compare_floats(*val, *other), - (Value::Float { val, .. }, Value::Int { val: other, .. }) => { - compare_floats(*val, *other as f64) - } - (Value::Int { val, .. }, Value::Float { val: other, .. }) => { - compare_floats(*val as f64, *other) - } - _ => None, - } -} - -// Compare two floating point numbers. The decision interval for equality is dynamically scaled -// as the value being compared increases in magnitude. -fn compare_floats(val: f64, other: f64) -> Option { - let prec = f64::EPSILON.max(val.abs() * f64::EPSILON); - - if (other - val).abs() < prec { - return Some(Ordering::Equal); - } - - val.partial_cmp(&other) } impl Iterator for RangeIterator { @@ -206,7 +184,7 @@ impl Iterator for RangeIterator { let ordering = if matches!(self.end, Value::Nothing { .. }) { Some(Ordering::Less) } else { - compare_numbers(&self.curr, &self.end) + self.curr.partial_cmp(&self.end) }; let ordering = if let Some(ord) = ordering { From 5f9ad0947dc803fe51b1de1465bc7606533766ea Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 17:58:39 +0200 Subject: [PATCH 09/14] Fix `Range::contains` --- crates/nu-protocol/src/value/range.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index f414dd0dce..56fc052fdc 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -115,7 +115,7 @@ impl Range { pub fn contains(&self, item: &Value) -> bool { match (item.partial_cmp(&self.from), item.partial_cmp(&self.to)) { (Some(Ordering::Greater | Ordering::Equal), Some(Ordering::Less)) => self.moves_up(), - (Some(Ordering::Less | Ordering::Equal), Some(Ordering::Greater)) => self.moves_up(), + (Some(Ordering::Less | Ordering::Equal), Some(Ordering::Greater)) => !self.moves_up(), (Some(_), Some(Ordering::Equal)) => self.is_end_inclusive(), (_, _) => false, } From 9e7e8ed48f1981e13d255a249c8c23966268e5b3 Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 17:58:58 +0200 Subject: [PATCH 10/14] Handle `not-in` operator --- crates/nu-engine/src/eval.rs | 1 + crates/nu-parser/src/type_check.rs | 22 ++++++++++++++++ crates/nu-protocol/src/value/mod.rs | 39 +++++++++++++++++++++++++++-- src/tests.rs | 10 ++++++++ 4 files changed, 70 insertions(+), 2 deletions(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 86acdc628d..16572d8ad8 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -191,6 +191,7 @@ pub fn eval_expression( Operator::Equal => lhs.eq(op_span, &rhs), Operator::NotEqual => lhs.ne(op_span, &rhs), Operator::In => lhs.r#in(op_span, &rhs), + Operator::NotIn => lhs.not_in(op_span, &rhs), x => Err(ShellError::UnsupportedOperator(x, op_span)), } } diff --git a/crates/nu-parser/src/type_check.rs b/crates/nu-parser/src/type_check.rs index d2f972cabf..ca087d0a87 100644 --- a/crates/nu-parser/src/type_check.rs +++ b/crates/nu-parser/src/type_check.rs @@ -295,6 +295,28 @@ pub fn math_result_type( ) } }, + Operator::NotIn => match (&lhs.ty, &rhs.ty) { + (t, Type::List(u)) if type_compatible(t, u) => (Type::Bool, None), + (Type::Int | Type::Float, Type::Range) => (Type::Bool, None), + (Type::String, Type::String) => (Type::Bool, None), + (Type::String, Type::Record(_, _)) => (Type::Bool, None), + + (Type::Unknown, _) => (Type::Bool, None), + (_, Type::Unknown) => (Type::Bool, None), + _ => { + *op = Expression::garbage(op.span); + ( + Type::Unknown, + Some(ParseError::UnsupportedOperation( + op.span, + lhs.span, + lhs.ty.clone(), + rhs.span, + rhs.ty.clone(), + )), + ) + } + }, _ => { *op = Expression::garbage(op.span); diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 0ad12a9864..4b8e6b7948 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -812,7 +812,7 @@ impl Value { match self.partial_cmp(rhs) { Some(ordering) => Ok(Value::Bool { - val: !matches!(ordering, Ordering::Less), + val: !matches!(ordering, Ordering::Equal), span, }), None => Err(ShellError::OperatorMismatch { @@ -846,7 +846,7 @@ impl Value { span, }), (lhs, Value::Stream { stream: rhs, .. }) => Ok(Value::Bool { - // TODO(@arthur-targaryen): Not sure about this clone. + // TODO(@arthur-targaryen): Not sure about this clone (see also `Value::not_in`). val: rhs.clone().any(|x| lhs == &x), span, }), @@ -859,6 +859,41 @@ impl Value { }), } } + + pub fn not_in(&self, op: Span, rhs: &Value) -> Result { + let span = span(&[self.span(), rhs.span()]); + + match (self, rhs) { + (lhs, Value::Range { val: rhs, .. }) => Ok(Value::Bool { + val: !rhs.contains(lhs), + span, + }), + (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => Ok(Value::Bool { + val: !rhs.contains(lhs), + span, + }), + (lhs, Value::List { vals: rhs, .. }) => Ok(Value::Bool { + val: !rhs.contains(lhs), + span, + }), + (Value::String { val: lhs, .. }, Value::Record { cols: rhs, .. }) => Ok(Value::Bool { + val: !rhs.contains(lhs), + span, + }), + (lhs, Value::Stream { stream: rhs, .. }) => Ok(Value::Bool { + // TODO(@arthur-targaryen): Not sure about this clone (see also `Value::r#in`). + val: rhs.clone().all(|x| lhs != &x), + span, + }), + _ => Err(ShellError::OperatorMismatch { + op_span: op, + lhs_ty: self.get_type(), + lhs_span: self.span(), + rhs_ty: rhs.get_type(), + rhs_span: rhs.span(), + }), + } + } } /// Format a duration in nanoseconds into a string diff --git a/src/tests.rs b/src/tests.rs index 2de6466fbe..7d9ca9707e 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -631,3 +631,13 @@ fn string_in_valuestream() -> TestResult { "true", ) } + +#[test] +fn string_not_in_string() -> TestResult { + run_test(r#"'d' not-in 'abc'"#, "true") +} + +#[test] +fn float_not_in_inc_range() -> TestResult { + run_test(r#"1.4 not-in 2..9.42"#, "true") +} From 4e443b2088711e480a51fe79c1996a4d128e721e Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 18:02:05 +0200 Subject: [PATCH 11/14] Change helper method visibility --- crates/nu-protocol/src/value/range.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index 56fc052fdc..c4636f61ea 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -104,11 +104,11 @@ impl Range { }) } - pub fn moves_up(&self) -> bool { + fn moves_up(&self) -> bool { self.from <= self.to } - pub fn is_end_inclusive(&self) -> bool { + fn is_end_inclusive(&self) -> bool { matches!(self.inclusion, RangeInclusion::Inclusive) } From 75de7f7e612e7542be91ca2b4a982acc3ab65966 Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 19:11:15 +0200 Subject: [PATCH 12/14] Implement `PartialOrd` for `Value::Stream` --- crates/nu-protocol/src/value/mod.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 4b8e6b7948..28e1a173f6 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -479,9 +479,7 @@ impl PartialOrd for Value { (Value::Block { val: b1, .. }, Value::Block { val: b2, .. }) if b1 == b2 => { Some(Ordering::Equal) } - (Value::List { vals: lhs, .. }, Value::List { vals: rhs, .. }) if lhs == rhs => { - Some(Ordering::Equal) - } + (Value::List { vals: lhs, .. }, Value::List { vals: rhs, .. }) => lhs.partial_cmp(rhs), ( Value::Record { vals: lhs, @@ -531,6 +529,18 @@ impl PartialOrd for Value { (Value::Stream { stream: lhs, .. }, Value::Stream { stream: rhs, .. }) => { lhs.clone().partial_cmp(rhs.clone()) } + (Value::Stream { stream: lhs, .. }, Value::String { val: rhs, .. }) => { + lhs.clone().collect_string().partial_cmp(rhs) + } + (Value::String { val: lhs, .. }, Value::Stream { stream: rhs, .. }) => { + lhs.partial_cmp(&rhs.clone().collect_string()) + } + (Value::Stream { stream: lhs, .. }, Value::List { vals: rhs, .. }) => { + lhs.clone().collect::>().partial_cmp(rhs) + } + (Value::List { vals: lhs, .. }, Value::Stream { stream: rhs, .. }) => { + lhs.partial_cmp(&rhs.clone().collect::>()) + } (_, _) => None, } } @@ -846,7 +856,6 @@ impl Value { span, }), (lhs, Value::Stream { stream: rhs, .. }) => Ok(Value::Bool { - // TODO(@arthur-targaryen): Not sure about this clone (see also `Value::not_in`). val: rhs.clone().any(|x| lhs == &x), span, }), @@ -881,7 +890,6 @@ impl Value { span, }), (lhs, Value::Stream { stream: rhs, .. }) => Ok(Value::Bool { - // TODO(@arthur-targaryen): Not sure about this clone (see also `Value::r#in`). val: rhs.clone().all(|x| lhs != &x), span, }), From d5fdfdb6146b4e252fb5dff7b242a2d8c2b3180a Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 19:22:02 +0200 Subject: [PATCH 13/14] Add missing test attribute --- src/tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests.rs b/src/tests.rs index 7d9ca9707e..eb431e8a47 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -570,6 +570,7 @@ fn for_loops() -> TestResult { run_test(r#"(for x in [1, 2, 3] { $x + 10 }).1"#, "12") } +#[test] fn type_in_list_of_this_type() -> TestResult { run_test(r#"42 in [41 42 43]"#, "true") } From a0a63c966f915286804a9a9024575169451edd06 Mon Sep 17 00:00:00 2001 From: Arthur Targaryen Date: Sat, 9 Oct 2021 19:26:20 +0200 Subject: [PATCH 14/14] Add inline attribute and address warning --- crates/nu-protocol/src/value/mod.rs | 37 +++------------------------ crates/nu-protocol/src/value/range.rs | 2 ++ 2 files changed, 5 insertions(+), 34 deletions(-) diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 28e1a173f6..f767f80e2b 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -492,40 +492,6 @@ impl PartialOrd for Value { .. }, ) if lhs_headers == rhs_headers && lhs == rhs => Some(Ordering::Equal), - // Note: This may look a bit strange, but a Stream is still just a List, - // it just happens to be in an iterator form instead of a concrete form. If the contained - // values are the same then it should be treated as equal - ( - Value::Stream { - stream: stream_lhs, .. - }, - Value::List { - vals: stream_rhs, .. - }, - ) => { - let vals_lhs: Vec = stream_lhs.clone().collect(); - let vals_rhs: Vec = - stream_rhs.clone().into_iter().into_value_stream().collect(); - - vals_lhs.partial_cmp(&vals_rhs) - } - // Note: This may look a bit strange, but a Stream is still just a List, - // it just happens to be in an iterator form instead of a concrete form. If the contained - // values are the same then it should be treated as equal - ( - Value::List { - vals: stream_lhs, .. - }, - Value::Stream { - stream: stream_rhs, .. - }, - ) => { - let vals_lhs: Vec = - stream_lhs.clone().into_iter().into_value_stream().collect(); - let vals_rhs: Vec = stream_rhs.clone().collect(); - - vals_lhs.partial_cmp(&vals_rhs) - } (Value::Stream { stream: lhs, .. }, Value::Stream { stream: rhs, .. }) => { lhs.clone().partial_cmp(rhs.clone()) } @@ -535,6 +501,9 @@ impl PartialOrd for Value { (Value::String { val: lhs, .. }, Value::Stream { stream: rhs, .. }) => { lhs.partial_cmp(&rhs.clone().collect_string()) } + // NOTE: This may look a bit strange, but a `Stream` is still just a `List`, it just + // happens to be in an iterator form instead of a concrete form. The contained values + // can be compared. (Value::Stream { stream: lhs, .. }, Value::List { vals: rhs, .. }) => { lhs.clone().collect::>().partial_cmp(rhs) } diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index c4636f61ea..8d2bbbb811 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -104,10 +104,12 @@ impl Range { }) } + #[inline] fn moves_up(&self) -> bool { self.from <= self.to } + #[inline] fn is_end_inclusive(&self) -> bool { matches!(self.inclusion, RangeInclusion::Inclusive) }