Implement and use PartialOrd for Value

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 {
+impl PartialOrd for Value {
-    fn eq(&self, other: &Self) -> bool {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        // 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<Ordering> {
+            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::Bool { val: lhs, .. }, Value::Bool { val: rhs, .. }) => lhs.partial_cmp(rhs),
-            (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => lhs == rhs,
+            (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => lhs.partial_cmp(rhs),
-            (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => lhs == rhs,
+            (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
-            (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => lhs == rhs,
+                compare_floats(*lhs, *rhs)
-            (Value::Block { val: b1, .. }, Value::Block { val: b2, .. }) => b1 == b2,
+            }
-            (Value::List { vals: vals_lhs, .. }, Value::List { vals: vals_rhs, .. }) => {
+            (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => {
-                for (lhs, rhs) in vals_lhs.iter().zip(vals_rhs) {
+                lhs.partial_cmp(rhs)
-                    if lhs != rhs {
+            }
-                        return false;
+            (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
-                    }
+                compare_floats(*lhs as f64, *rhs)
-                }
+            }
-
+            (Value::Float { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
-                true
+                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: lhs,
-                    vals: vals_lhs,
+                    cols: lhs_headers,
                     ..
                 },
                 Value::Record {
-                    cols: cols_rhs,
+                    vals: rhs,
-                    vals: vals_rhs,
+                    cols: rhs_headers,
                     ..
                 },
-            ) => {
+            ) if lhs_headers == rhs_headers && lhs == rhs => Some(Ordering::Equal),
-                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<Value> = stream_lhs.clone().collect();
-                let vals_rhs: Vec<Value> = stream_rhs.clone().collect();
-
-                vals_lhs == 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
@@ -510,7 +509,7 @@ impl PartialEq for Value {
                 let vals_rhs: Vec<Value> =
                     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<Value> = 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<Value, ShellError> {
         let span = span(&[self.span(), rhs.span()]);
@@ -717,37 +725,12 @@ impl Value {
     pub fn lt(&self, op: Span, rhs: &Value) -> Result<Value, ShellError> {
         let span = span(&[self.span(), rhs.span()]);
 
-        match (self, rhs) {
+        match self.partial_cmp(rhs) {
-            (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool {
+            Some(ordering) => Ok(Value::Bool {
-                val: lhs < rhs,
+                val: matches!(ordering, Ordering::Less),
                 span,
             }),
-            (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool {
+            None => Err(ShellError::OperatorMismatch {
-                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 {
                 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<Value, ShellError> {
         let span = span(&[self.span(), rhs.span()]);
 
-        match (self, rhs) {
+        match self.partial_cmp(rhs) {
-            (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool {
+            Some(ordering) => Ok(Value::Bool {
-                val: lhs <= rhs,
+                val: matches!(ordering, Ordering::Less | Ordering::Equal),
                 span,
             }),
-            (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool {
+            None => Err(ShellError::OperatorMismatch {
-                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 {
                 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<Value, ShellError> {
         let span = span(&[self.span(), rhs.span()]);
 
-        match (self, rhs) {
+        match self.partial_cmp(rhs) {
-            (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool {
+            Some(ordering) => Ok(Value::Bool {
-                val: lhs > rhs,
+                val: matches!(ordering, Ordering::Greater),
                 span,
             }),
-            (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool {
+            None => Err(ShellError::OperatorMismatch {
-                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 {
                 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<Value, ShellError> {
         let span = span(&[self.span(), rhs.span()]);
 
-        match (self, rhs) {
+        match self.partial_cmp(rhs) {
-            (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool {
+            Some(ordering) => Ok(Value::Bool {
-                val: lhs >= rhs,
+                val: matches!(ordering, Ordering::Greater | Ordering::Equal),
                 span,
             }),
-            (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => Ok(Value::Bool {
+            None => Err(ShellError::OperatorMismatch {
-                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 {
                 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<Value, ShellError> {
         let span = span(&[self.span(), rhs.span()]);
 
-        match (self, rhs) {
+        match self.partial_cmp(rhs) {
-            (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool {
+            Some(ordering) => Ok(Value::Bool {
-                val: lhs == rhs,
+                val: matches!(ordering, Ordering::Equal),
                 span,
             }),
-            (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => Ok(Value::Bool {
+            None => Err(ShellError::OperatorMismatch {
-                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 {
                 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<Value, ShellError> {
         let span = span(&[self.span(), rhs.span()]);
 
-        match (self, rhs) {
+        match self.partial_cmp(rhs) {
-            (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => Ok(Value::Bool {
+            Some(ordering) => Ok(Value::Bool {
-                val: lhs != rhs,
+                val: !matches!(ordering, Ordering::Less),
                 span,
             }),
-            (Value::String { val: lhs, .. }, Value::String { val: rhs, .. }) => Ok(Value::Bool {
+            None => Err(ShellError::OperatorMismatch {
-                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 {
                 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.contains(lhs),
-                val: rhs.clone().into_iter().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| {
+                val: rhs.contains(lhs),
-                    matches!(
-                        lhs.eq(Span::unknown(), x),
-                        Ok(Value::Bool { val: true, .. })
-                    )
-                }),
                 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.
+                // TODO(@arthur-targaryen): Not sure about this clone.
-                val: rhs.clone().any(|x| {
+                val: rhs.clone().any(|x| lhs == &x),
-                    matches!(
-                        lhs.eq(Span::unknown(), &x),
-                        Ok(Value::Bool { val: true, .. })
-                    )
-                }),
                 span,
             }),
             _ => Err(ShellError::OperatorMismatch {

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<Ordering> {
-    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<Ordering> {
-    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 {