From a1840e9d20d5edaa94b80a36583d82468e8e5d6c Mon Sep 17 00:00:00 2001
From: Sygmei <3835355+Sygmei@users.noreply.github.com>
Date: Wed, 22 Mar 2023 09:47:40 +0100
Subject: [PATCH] fix: fixed typo and improved Value TypeMismatch exceptions
 (#8324)

# Description

This PR aims to improve `TypeMismatch` exception that occurs when
comparing two values with `<`, `>`, `<=` or `>=` operators.

*Before*

![image](https://user-images.githubusercontent.com/3835355/222980803-8cb0f945-5a82-4512-9989-5df0ec4e4969.png)

*After*

![image](https://user-images.githubusercontent.com/3835355/226754903-68e56344-065d-42ee-b184-ab968e91c6de.png)

This PR also bundles a small refactor for histogram forbidden column
names exception, previous implementation forgot a column name in the
message, to avoid this, I'm re-using the same array for checking and
error display

# User-Facing Changes

Not much changes except a better and more readable exception for the
user

# Tests + Formatting

Does not break any tests, formatting passes as well :)
---
 crates/nu-command/src/charting/histogram.rs | 14 +++++---
 crates/nu-protocol/src/value/mod.rs         | 36 ++++++++++++++-------
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/crates/nu-command/src/charting/histogram.rs b/crates/nu-command/src/charting/histogram.rs
index 1353302ce..d4559ddf8 100644
--- a/crates/nu-command/src/charting/histogram.rs
+++ b/crates/nu-command/src/charting/histogram.rs
@@ -98,11 +98,17 @@ impl Command for Histogram {
         let frequency_name_arg = call.opt::<Spanned<String>>(engine_state, stack, 1)?;
         let frequency_column_name = match frequency_name_arg {
             Some(inner) => {
-                if ["value", "count", "quantile", "percentage"].contains(&inner.item.as_str()) {
+                let forbidden_column_names = ["value", "count", "quantile", "percentage"];
+                if forbidden_column_names.contains(&inner.item.as_str()) {
                     return Err(ShellError::TypeMismatch {
-                        err_message:
-                            "frequency-column-name can't be 'value', 'count' or 'percentage'"
-                                .to_string(),
+                        err_message: format!(
+                            "frequency-column-name can't be {}",
+                            forbidden_column_names
+                                .iter()
+                                .map(|val| format!("'{}'", val))
+                                .collect::<Vec<_>>()
+                                .join(", ")
+                        ),
                         span: inner.span,
                     });
                 }
diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs
index 3193c02a9..83ebddb71 100644
--- a/crates/nu-protocol/src/value/mod.rs
+++ b/crates/nu-protocol/src/value/mod.rs
@@ -2654,9 +2654,12 @@ impl Value {
             && (self.get_type() != Type::Any)
             && (rhs.get_type() != Type::Any)
         {
-            return Err(ShellError::TypeMismatch {
-                err_message: "compatible type".to_string(),
-                span: op,
+            return Err(ShellError::OperatorMismatch {
+                op_span: op,
+                lhs_ty: self.get_type(),
+                lhs_span: self.span()?,
+                rhs_ty: rhs.get_type(),
+                rhs_span: rhs.span()?,
             });
         }
 
@@ -2690,9 +2693,12 @@ impl Value {
             && (self.get_type() != Type::Any)
             && (rhs.get_type() != Type::Any)
         {
-            return Err(ShellError::TypeMismatch {
-                err_message: "compatible type".to_string(),
-                span: op,
+            return Err(ShellError::OperatorMismatch {
+                op_span: op,
+                lhs_ty: self.get_type(),
+                lhs_span: self.span()?,
+                rhs_ty: rhs.get_type(),
+                rhs_span: rhs.span()?,
             });
         }
 
@@ -2724,9 +2730,12 @@ impl Value {
             && (self.get_type() != Type::Any)
             && (rhs.get_type() != Type::Any)
         {
-            return Err(ShellError::TypeMismatch {
-                err_message: "compatible type".to_string(),
-                span: op,
+            return Err(ShellError::OperatorMismatch {
+                op_span: op,
+                lhs_ty: self.get_type(),
+                lhs_span: self.span()?,
+                rhs_ty: rhs.get_type(),
+                rhs_span: rhs.span()?,
             });
         }
 
@@ -2758,9 +2767,12 @@ impl Value {
             && (self.get_type() != Type::Any)
             && (rhs.get_type() != Type::Any)
         {
-            return Err(ShellError::TypeMismatch {
-                err_message: "compatible type".to_string(),
-                span: op,
+            return Err(ShellError::OperatorMismatch {
+                op_span: op,
+                lhs_ty: self.get_type(),
+                lhs_span: self.span()?,
+                rhs_ty: rhs.get_type(),
+                rhs_span: rhs.span()?,
             });
         }