From a3faefb89346cd68b72e12304bd03dbe0e1c7f37 Mon Sep 17 00:00:00 2001
From: Sebastian Schmidt <isibboi@gmail.com>
Date: Thu, 29 Aug 2019 13:35:26 +0300
Subject: [PATCH] Fix panic when adding number to string

For no reason, unwraps were used in the operator module.

Fixes #59
---
 CHANGELOG.md         |  1 +
 src/operator/mod.rs  | 25 +++++++++++--------------
 tests/integration.rs | 17 +++++++++++++++++
 3 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index dbb723d..c2a3364 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -32,6 +32,7 @@
  * Allow variable assignments in eval calls without context.
    A `HashMapContext` is created automatically now.
  * The error string for `ExpectedNumber` was wrong
+ * Operators panicked when adding a number to a string
 
 ### Deprecated
 
diff --git a/src/operator/mod.rs b/src/operator/mod.rs
index eb279c2..9a5dca0 100644
--- a/src/operator/mod.rs
+++ b/src/operator/mod.rs
@@ -165,7 +165,7 @@ impl Operator {
                     }
                 } else {
                     Ok(Value::Float(
-                        arguments[0].as_number().unwrap() + arguments[1].as_number().unwrap(),
+                        arguments[0].as_number()? + arguments[1].as_number()?,
                     ))
                 }
             },
@@ -186,7 +186,7 @@ impl Operator {
                     }
                 } else {
                     Ok(Value::Float(
-                        arguments[0].as_number().unwrap() - arguments[1].as_number().unwrap(),
+                        arguments[0].as_number()? - arguments[1].as_number()?,
                     ))
                 }
             },
@@ -202,7 +202,7 @@ impl Operator {
                         Err(EvalexprError::negation_error(arguments[0].clone()))
                     }
                 } else {
-                    Ok(Value::Float(-arguments[0].as_number().unwrap()))
+                    Ok(Value::Float(-arguments[0].as_number()?))
                 }
             },
             Mul => {
@@ -222,7 +222,7 @@ impl Operator {
                     }
                 } else {
                     Ok(Value::Float(
-                        arguments[0].as_number().unwrap() * arguments[1].as_number().unwrap(),
+                        arguments[0].as_number()? * arguments[1].as_number()?,
                     ))
                 }
             },
@@ -243,7 +243,7 @@ impl Operator {
                     }
                 } else {
                     Ok(Value::Float(
-                        arguments[0].as_number().unwrap() / arguments[1].as_number().unwrap(),
+                        arguments[0].as_number()? / arguments[1].as_number()?,
                     ))
                 }
             },
@@ -264,7 +264,7 @@ impl Operator {
                     }
                 } else {
                     Ok(Value::Float(
-                        arguments[0].as_number().unwrap() % arguments[1].as_number().unwrap(),
+                        arguments[0].as_number()? % arguments[1].as_number()?,
                     ))
                 }
             },
@@ -274,10 +274,7 @@ impl Operator {
                 arguments[1].as_number()?;
 
                 Ok(Value::Float(
-                    arguments[0]
-                        .as_number()
-                        .unwrap()
-                        .powf(arguments[1].as_number().unwrap()),
+                    arguments[0].as_number()?.powf(arguments[1].as_number()?),
                 ))
             },
             Eq => {
@@ -316,7 +313,7 @@ impl Operator {
                         Ok(Value::Boolean(false))
                     }
                 } else {
-                    if arguments[0].as_number().unwrap() > arguments[1].as_number().unwrap() {
+                    if arguments[0].as_number()? > arguments[1].as_number()? {
                         Ok(Value::Boolean(true))
                     } else {
                         Ok(Value::Boolean(false))
@@ -341,7 +338,7 @@ impl Operator {
                         Ok(Value::Boolean(false))
                     }
                 } else {
-                    if arguments[0].as_number().unwrap() < arguments[1].as_number().unwrap() {
+                    if arguments[0].as_number()? < arguments[1].as_number()? {
                         Ok(Value::Boolean(true))
                     } else {
                         Ok(Value::Boolean(false))
@@ -366,7 +363,7 @@ impl Operator {
                         Ok(Value::Boolean(false))
                     }
                 } else {
-                    if arguments[0].as_number().unwrap() >= arguments[1].as_number().unwrap() {
+                    if arguments[0].as_number()? >= arguments[1].as_number()? {
                         Ok(Value::Boolean(true))
                     } else {
                         Ok(Value::Boolean(false))
@@ -391,7 +388,7 @@ impl Operator {
                         Ok(Value::Boolean(false))
                     }
                 } else {
-                    if arguments[0].as_number().unwrap() <= arguments[1].as_number().unwrap() {
+                    if arguments[0].as_number()? <= arguments[1].as_number()? {
                         Ok(Value::Boolean(true))
                     } else {
                         Ok(Value::Boolean(false))
diff --git a/tests/integration.rs b/tests/integration.rs
index df5c3f6..f9101ba 100644
--- a/tests/integration.rs
+++ b/tests/integration.rs
@@ -695,3 +695,20 @@ fn test_operator_assignments() {
         Ok(true)
     );
 }
+
+#[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.
+    assert_eq!(
+        eval("4 + \"abc\""),
+        Err(EvalexprError::expected_number(Value::from(
+            "abc".to_string()
+        )))
+    );
+    assert_eq!(
+        eval("\"abc\" + 4"),
+        Err(EvalexprError::expected_number(Value::from(
+            "abc".to_string()
+        )))
+    );
+}