From d53eaac7a1ef089485e3bd3a442364c1be918d6d Mon Sep 17 00:00:00 2001
From: JT <547158+jntrnr@users.noreply.github.com>
Date: Fri, 18 Feb 2022 17:11:27 -0500
Subject: [PATCH] Improve comparison errors (#4541)

---
 crates/nu-command/src/filters/where_.rs      | 52 +++++++++++---------
 crates/nu-command/tests/commands/str_/mod.rs |  2 +-
 crates/nu-protocol/src/value/mod.rs          | 36 ++++++++++++++
 3 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/crates/nu-command/src/filters/where_.rs b/crates/nu-command/src/filters/where_.rs
index f17943c332..cbda0c7f3c 100644
--- a/crates/nu-command/src/filters/where_.rs
+++ b/crates/nu-command/src/filters/where_.rs
@@ -1,7 +1,9 @@
 use nu_engine::{eval_block_with_redirect, CallExt};
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{CaptureBlock, Command, EngineState, Stack};
-use nu_protocol::{Category, Example, PipelineData, Signature, SyntaxShape};
+use nu_protocol::{
+    Category, Example, IntoInterruptiblePipelineData, PipelineData, Signature, SyntaxShape, Value,
+};
 
 #[derive(Clone)]
 pub struct Where;
@@ -39,29 +41,35 @@ impl Command for Where {
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();
 
-        input
-            .filter(
-                move |value| {
-                    if let Some(var) = block.signature.get_positional(0) {
-                        if let Some(var_id) = &var.var_id {
-                            stack.add_var(*var_id, value.clone());
+        Ok(input
+            .into_iter()
+            .filter_map(move |value| {
+                if let Some(var) = block.signature.get_positional(0) {
+                    if let Some(var_id) = &var.var_id {
+                        stack.add_var(*var_id, value.clone());
+                    }
+                }
+                let result = eval_block_with_redirect(
+                    &engine_state,
+                    &mut stack,
+                    &block,
+                    PipelineData::new(span),
+                );
+
+                match result {
+                    Ok(result) => {
+                        let result = result.into_value(span);
+                        if result.is_true() {
+                            Some(value)
+                        } else {
+                            None
                         }
                     }
-                    let result = eval_block_with_redirect(
-                        &engine_state,
-                        &mut stack,
-                        &block,
-                        PipelineData::new(span),
-                    );
-
-                    match result {
-                        Ok(result) => result.into_value(span).is_true(),
-                        _ => false,
-                    }
-                },
-                ctrlc,
-            )
-            .map(|x| x.set_metadata(metadata))
+                    Err(err) => Some(Value::Error { error: err }),
+                }
+            })
+            .into_pipeline_data(ctrlc))
+        .map(|x| x.set_metadata(metadata))
     }
 
     fn examples(&self) -> Vec<Example> {
diff --git a/crates/nu-command/tests/commands/str_/mod.rs b/crates/nu-command/tests/commands/str_/mod.rs
index 9f8c23d5fc..f56becb20c 100644
--- a/crates/nu-command/tests/commands/str_/mod.rs
+++ b/crates/nu-command/tests/commands/str_/mod.rs
@@ -127,7 +127,7 @@ fn converts_to_int() {
             | into int number_as_string
             | rename number
             | where number == 1
-            | get number
+            | get number.0
 
         "#
     ));
diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs
index cf7bc1cf22..808a08fce6 100644
--- a/crates/nu-protocol/src/value/mod.rs
+++ b/crates/nu-protocol/src/value/mod.rs
@@ -1537,6 +1537,13 @@ impl Value {
             return lhs.operation(*span, Operator::LessThan, op, rhs);
         }
 
+        if !type_compatible(self.get_type(), rhs.get_type())
+            && (self.get_type() != Type::Unknown)
+            && (rhs.get_type() != Type::Unknown)
+        {
+            return Err(ShellError::TypeMismatch("compatible type".to_string(), op));
+        }
+
         match self.partial_cmp(rhs) {
             Some(ordering) => Ok(Value::Bool {
                 val: matches!(ordering, Ordering::Less),
@@ -1558,6 +1565,13 @@ impl Value {
             return lhs.operation(*span, Operator::LessThanOrEqual, op, rhs);
         }
 
+        if !type_compatible(self.get_type(), rhs.get_type())
+            && (self.get_type() != Type::Unknown)
+            && (rhs.get_type() != Type::Unknown)
+        {
+            return Err(ShellError::TypeMismatch("compatible type".to_string(), op));
+        }
+
         match self.partial_cmp(rhs) {
             Some(ordering) => Ok(Value::Bool {
                 val: matches!(ordering, Ordering::Less | Ordering::Equal),
@@ -1579,6 +1593,13 @@ impl Value {
             return lhs.operation(*span, Operator::GreaterThan, op, rhs);
         }
 
+        if !type_compatible(self.get_type(), rhs.get_type())
+            && (self.get_type() != Type::Unknown)
+            && (rhs.get_type() != Type::Unknown)
+        {
+            return Err(ShellError::TypeMismatch("compatible type".to_string(), op));
+        }
+
         match self.partial_cmp(rhs) {
             Some(ordering) => Ok(Value::Bool {
                 val: matches!(ordering, Ordering::Greater),
@@ -1600,6 +1621,13 @@ impl Value {
             return lhs.operation(*span, Operator::GreaterThanOrEqual, op, rhs);
         }
 
+        if !type_compatible(self.get_type(), rhs.get_type())
+            && (self.get_type() != Type::Unknown)
+            && (rhs.get_type() != Type::Unknown)
+        {
+            return Err(ShellError::TypeMismatch("compatible type".to_string(), op));
+        }
+
         match self.partial_cmp(rhs) {
             Some(ordering) => Ok(Value::Bool {
                 val: matches!(ordering, Ordering::Greater | Ordering::Equal),
@@ -1985,6 +2013,14 @@ impl From<Spanned<HashMap<String, Value>>> for Value {
     }
 }
 
+fn type_compatible(a: Type, b: Type) -> bool {
+    if a == b {
+        return true;
+    }
+
+    matches!((a, b), (Type::Int, Type::Float) | (Type::Float, Type::Int))
+}
+
 /// Create a Value::Record from a spanned indexmap
 impl From<Spanned<IndexMap<String, Value>>> for Value {
     fn from(input: Spanned<IndexMap<String, Value>>) -> Self {