Fix exceptions and incorrect results within the calculator plugin (#7438

) * remove functions which mages cannot interpret and add in functions which mages can * set validResult when the result is explicitly created to differentiate it form an empty CalculateResult * Add condition to check that the input is not ending with a binary operation * add tests for all the cases * use valid result while calculating hash as well * add test for e is valid within regex * removed i from regex * remove valid result to use decimal? instead * remove duplicate rand and exp
microsoft · Oct 21, 2020 · dd2627d · dd2627d
1 parent 86d7710
commit dd2627d
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 24 deletions.
diff --git a/...es/launcher/Plugins/Microsoft.Plugin.Calculator.UnitTest/ExtendedCalculatorParserTests.cs b/...es/launcher/Plugins/Microsoft.Plugin.Calculator.UnitTest/ExtendedCalculatorParserTests.cs
@@ -34,6 +34,7 @@ public void Interpret_ThrowError_WhenCalledNullOrEmpty(string input)
 
         [TestCase("42")]
         [TestCase("test")]
+        [TestCase("pi(2)")] // Incorrect input, constant is being treated as a function.
         public void Interpret_NoResult_WhenCalled(string input)
         {
             // Arrange
@@ -60,6 +61,7 @@ public void Interpret_NoResult_WhenCalled(string input)
         [TestCase("2+2.11", 4.11D)]
         [TestCase("8.43 + 4.43 - 12.86", 0D)]
         [TestCase("8.43 + 4.43 - 12.8", 0.06D)]
+        [TestCase("exp(5)", 148.413159102577D)]
         public void Interpret_NoErrors_WhenCalledWithRounding(string input, decimal expectedResult)
         {
             // Arrange
@@ -110,7 +112,15 @@ public void Interpret_DifferentCulture_WhenCalled(string input, decimal expected
         [TestCase("((1 * 2)", false)]
         [TestCase("(1 * 2)))", false)]
         [TestCase("abcde", false)]
-        [TestCase("plot( 2 * 3)", true)]
+        [TestCase("1 + 2 +", false)]
+        [TestCase("1+2*", false)]
+        [TestCase("1 && 3 &&", false)]
+        [TestCase("sqrt( 36)", true)]
+        [TestCase("max 4", false)]
+        [TestCase("sin(0)", true)]
+        [TestCase("cos", false)]
+        [TestCase("abs", false)]
+        [TestCase("1+1.1e3", true)]
         public void InputValid_TestValid_WhenCalled(string input, bool valid)
         {
             // Arrange
@@ -121,5 +131,38 @@ public void InputValid_TestValid_WhenCalled(string input, bool valid)
             // Assert
             Assert.AreEqual(valid, result);
         }
+
+        [TestCase("1-1")]
+        [TestCase("sin(0)")]
+        public void Interpret_MustReturnResult_WhenResultIsZero(string input)
+        {
+            // Arrange
+            var engine = new CalculateEngine();
+
+            // Act
+            var result = engine.Interpret(input, CultureInfo.InvariantCulture);
+
+            // Assert
+            Assert.IsNotNull(result);
+            Assert.AreEqual(0.0, result.Result);
+        }
+
+        [TestCase("factorial(5)", 120)]
+        [TestCase("sign(-2)", -1)]
+        [TestCase("sign(2)", +1)]
+        [TestCase("abs(-2)", 2)]
+        [TestCase("abs(2)", 2)]
+        public void Interpret_MustReturnExpectedResult_WhenCalled(string input, decimal expectedResult)
+        {
+            // Arrange
+            var engine = new CalculateEngine();
+
+            // Act
+            var result = engine.Interpret(input, CultureInfo.InvariantCulture);
+
+            // Assert
+            Assert.IsNotNull(result);
+            Assert.AreEqual(expectedResult, result.Result);
+        }
     }
 }
diff --git a/src/modules/launcher/Plugins/Microsoft.Plugin.Calculator/CalculateHelper.cs b/src/modules/launcher/Plugins/Microsoft.Plugin.Calculator/CalculateHelper.cs
@@ -11,12 +11,12 @@ public static class CalculateHelper
     {
         private static readonly Regex RegValidExpressChar = new Regex(
             @"^(" +
-            @"ceil|floor|exp|pi|e|max|min|det|abs|log|ln|sqrt|" +
-            @"sin|cos|tan|arcsin|arccos|arctan|" +
-            @"eigval|eigvec|eig|sum|polar|plot|round|sort|real|zeta|" +
-            @"bin2dec|hex2dec|oct2dec|" +
+            @"ceil\s*\(|floor\s*\(|exp\s*\(|max\s*\(|min\s*\(|abs\s*\(|log\s*\(|ln\s*\(|sqrt\s*\(|pow\s*\(|" +
+            @"factorial\s*\(|sign\s*\(|round\s*\(|rand\s*\(|" +
+            @"sin\s*\(|cos\s*\(|tan\s*\(|arcsin\s*\(|arccos\s*\(|arctan\s*\(|" +
+            @"pi|" +
             @"==|~=|&&|\|\||" +
-            @"[ei]|[0-9]|[\+\-\*\/\^\., ""]|[\(\)\|\!\[\]]" +
+            @"e|[0-9]|[\+\-\*\/\^\., ""]|[\(\)\|\!\[\]]" +
             @")+$", RegexOptions.Compiled);
 
         public static bool InputValid(string input)
@@ -41,6 +41,13 @@ public static bool InputValid(string input)
                 return false;
             }
 
+            // If the input ends with a binary operator then it is not a valid input to mages and the Interpret function would throw an exception.
+            string trimmedInput = input.TrimEnd();
+            if (trimmedInput.EndsWith('+') || trimmedInput.EndsWith('-') || trimmedInput.EndsWith('*') || trimmedInput.EndsWith('|') || trimmedInput.EndsWith('\\') || trimmedInput.EndsWith('^') || trimmedInput.EndsWith('=') || trimmedInput.EndsWith('&'))
+            {
+                return false;
+            }
+
             return true;
         }
     }

diff --git a/src/modules/launcher/Plugins/Microsoft.Plugin.Calculator/CalculateResult.cs b/src/modules/launcher/Plugins/Microsoft.Plugin.Calculator/CalculateResult.cs
@@ -9,9 +9,9 @@ namespace Microsoft.Plugin.Calculator
 {
     public struct CalculateResult : IEquatable<CalculateResult>
     {
-        public decimal Result { get; set; }
+        public decimal? Result { get; set; }
 
-        public decimal RoundedResult { get; set; }
+        public decimal? RoundedResult { get; set; }
 
         public bool Equals(CalculateResult other)
         {

diff --git a/src/modules/launcher/Plugins/Microsoft.Plugin.Calculator/ResultHelper.cs b/src/modules/launcher/Plugins/Microsoft.Plugin.Calculator/ResultHelper.cs
@@ -17,38 +17,47 @@ public static Result CreateResult(CalculateResult result, string iconPath)
             return CreateResult(result.RoundedResult, iconPath);
         }
 
-        public static Result CreateResult(decimal roundedResult, string iconPath)
+        public static Result CreateResult(decimal? roundedResult, string iconPath)
         {
+            // Return null when the expression is not a valid calculator query.
+            if (roundedResult == null)
+            {
+                return null;
+            }
+
             return new Result
             {
-                Title = roundedResult.ToString(CultureInfo.CurrentCulture),
+                Title = roundedResult?.ToString(CultureInfo.CurrentCulture),
                 IcoPath = iconPath,
                 Score = 300,
                 SubTitle = Properties.Resources.wox_plugin_calculator_copy_number_to_clipboard,
                 Action = c => Action(roundedResult),
             };
         }
 
-        public static bool Action(decimal roundedResult)
+        public static bool Action(decimal? roundedResult)
         {
             var ret = false;
 
-            var thread = new Thread(() =>
+            if (roundedResult != null)
             {
-                try
-                {
-                    Clipboard.SetText(roundedResult.ToString(CultureInfo.CurrentUICulture.NumberFormat));
-                    ret = true;
-                }
-                catch (ExternalException)
+                var thread = new Thread(() =>
                 {
-                    MessageBox.Show(Properties.Resources.wox_plugin_calculator_copy_failed);
-                }
-            });
+                    try
+                    {
+                        Clipboard.SetText(roundedResult?.ToString(CultureInfo.CurrentUICulture.NumberFormat));
+                        ret = true;
+                    }
+                    catch (ExternalException)
+                    {
+                        MessageBox.Show(Properties.Resources.wox_plugin_calculator_copy_failed);
+                    }
+                });
 
-            thread.SetApartmentState(ApartmentState.STA);
-            thread.Start();
-            thread.Join();
+                thread.SetApartmentState(ApartmentState.STA);
+                thread.Start();
+                thread.Join();
+            }
 
             return ret;
         }