From 2d1704b9a38fce3b15358f9cb4362161f31b838d Mon Sep 17 00:00:00 2001
From: Sebastian Schmidt <isibboi@gmail.com>
Date: Thu, 29 Aug 2019 16:36:35 +0300
Subject: [PATCH] Implement better error messages for addition with
 incompatible types

Implements #60
---
 src/error/mod.rs     |  5 +++++
 src/operator/mod.rs  | 11 ++++++++---
 tests/integration.rs | 17 ++++++++++-------
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/error/mod.rs b/src/error/mod.rs
index 13b50b9..cf008a3 100644
--- a/src/error/mod.rs
+++ b/src/error/mod.rs
@@ -214,6 +214,11 @@ impl EvalexprError {
         EvalexprError::TypeError { actual, expected }
     }
 
+    /// Constructs `EvalexprError::WrongTypeCombination{operator, actual}`.
+    pub fn wrong_type_combination(operator: Operator, actual: Vec<ValueType>) -> Self {
+        EvalexprError::WrongTypeCombination { operator, actual }
+    }
+
     /// Constructs `EvalexprError::ExpectedString{actual}`.
     pub fn expected_string(actual: Value) -> Self {
         EvalexprError::ExpectedString { actual }
diff --git a/src/operator/mod.rs b/src/operator/mod.rs
index db7b7c3..cc3aa01 100644
--- a/src/operator/mod.rs
+++ b/src/operator/mod.rs
@@ -1,11 +1,12 @@
 use crate::function::builtin::builtin_function;
 
 use crate::{context::Context, error::*, value::Value};
+use std::borrow::Borrow;
 
 mod display;
 
 /// An enum that represents operators in the operator tree.
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone)]
 pub enum Operator {
     /// A root node in the operator tree.
     /// The whole expression is stored under a root node, as well as each subexpression surrounded by parentheses.
@@ -205,9 +206,13 @@ impl Operator {
                             arguments[1].clone(),
                         ))
                     }
+                } else if let (Ok(a), Ok(b)) = (arguments[0].as_number(), arguments[1].as_number())
+                {
+                    Ok(Value::Float(a + b))
                 } else {
-                    Ok(Value::Float(
-                        arguments[0].as_number()? + arguments[1].as_number()?,
+                    Err(EvalexprError::wrong_type_combination(
+                        self.clone(),
+                        vec![arguments[0].borrow().into(), arguments[1].borrow().into()],
                     ))
                 }
             },
diff --git a/tests/integration.rs b/tests/integration.rs
index f9101ba..e5aaf9c 100644
--- a/tests/integration.rs
+++ b/tests/integration.rs
@@ -698,17 +698,20 @@ fn test_operator_assignments() {
 
 #[test]
 fn test_type_errors_in_binary_operators() {
-    // This error is bad. In future, maybe add a special error message for this kind of call.
+    // Only addition supports incompatible types, all others work only on numbers or only on booleans.
+    // So only addition requires the more fancy error message.
     assert_eq!(
         eval("4 + \"abc\""),
-        Err(EvalexprError::expected_number(Value::from(
-            "abc".to_string()
-        )))
+        Err(EvalexprError::wrong_type_combination(
+            Operator::Add,
+            vec![ValueType::Int, ValueType::String]
+        ))
     );
     assert_eq!(
         eval("\"abc\" + 4"),
-        Err(EvalexprError::expected_number(Value::from(
-            "abc".to_string()
-        )))
+        Err(EvalexprError::wrong_type_combination(
+            Operator::Add,
+            vec![ValueType::String, ValueType::Int]
+        ))
     );
 }