Allow custom functions to be overloaded. (#347)

Allow custom functions to be overloaded.

Turns out the conflict was unnecessary - using a Dict with a name key when a HashSet was sufficient. 
This relies that TexlFunctions have object reference equality and we don't add the same function instance multiple times. .

src/libraries/Microsoft.PowerFx.Core/Public/Config/PowerFxConfig.cs

@@ -19,7 +19,7 @@ namespace Microsoft.PowerFx.Core
     public sealed class PowerFxConfig
     {
         private bool _isLocked;
-        private readonly Dictionary<string, TexlFunction> _extraFunctions;
+        private readonly HashSet<TexlFunction> _extraFunctions = new HashSet<TexlFunction>();
         private readonly Dictionary<DName, IExternalEntity> _environmentSymbols;
         private DisplayNameProvider _environmentSymbolDisplayNameProvider;
 
@@ -27,7 +27,7 @@ namespace Microsoft.PowerFx.Core
         // These can be overridden. 
         private IEnumerable<TexlFunction> _coreFunctions = BuiltinFunctionsCore.BuiltinFunctionsLibrary;
 
-        internal IEnumerable<TexlFunction> Functions => _coreFunctions.Concat(_extraFunctions.Values);
+        internal IEnumerable<TexlFunction> Functions => _coreFunctions.Concat(_extraFunctions);
 
         internal EnumStoreBuilder EnumStoreBuilder { get; }
 
@@ -37,7 +37,6 @@ namespace Microsoft.PowerFx.Core
         {
             CultureInfo = cultureInfo ?? CultureInfo.CurrentCulture;
             _isLocked = false;
-            _extraFunctions = new Dictionary<string, TexlFunction>();
             _environmentSymbols = new Dictionary<DName, IExternalEntity>();
             _environmentSymbolDisplayNameProvider = new SingleSourceDisplayNameProvider();
             EnumStoreBuilder = enumStoreBuilder;
@@ -89,7 +88,7 @@ namespace Microsoft.PowerFx.Core
         /// <returns></returns>
         public IEnumerable<string> GetAllFunctionNames()
         {
-            return _extraFunctions.Values.Select(func => func.Name).Distinct();
+            return _extraFunctions.Select(func => func.Name).Distinct();
         }
 
         internal IEnumerable<IExternalEntity> GetSymbols() => _environmentSymbols.Values;
@@ -134,7 +133,7 @@ namespace Microsoft.PowerFx.Core
         {
             CheckUnlocked();
 
-            _extraFunctions.Add(function.GetUniqueTexlRuntimeName(), function);
+            _extraFunctions.Add(function);
             EnumStoreBuilder.WithRequiredEnums(new List<TexlFunction>() { function });
         }
                 

src/libraries/Microsoft.PowerFx.Interpreter/ReflectionFunction.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Reflection;
+using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.PowerFx.Core.Functions;
@@ -32,7 +33,7 @@ namespace Microsoft.PowerFx
 
         public CustomTexlFunction(string name, DType returnType, params DType[] paramTypes)
             : base(DPath.Root, name, name, SG("Custom func " + name), FunctionCategories.MathAndStat, returnType, 0, paramTypes.Length, paramTypes.Length, paramTypes)
-        {
+        {            
         }
 
         public override bool IsSelfContained => true;

src/tests/Microsoft.PowerFx.Interpreter.Tests/CustomFunctions.cs

@@ -0,0 +1,106 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Microsoft.PowerFx.Core;
+using Microsoft.PowerFx.Core.Functions;
+using Microsoft.PowerFx.Core.Public;
+using Microsoft.PowerFx.Core.Public.Types;
+using Microsoft.PowerFx.Core.Public.Values;
+using Microsoft.PowerFx.Core.Tests;
+using Microsoft.PowerFx.Core.Texl;
+using Microsoft.PowerFx.Core.Types.Enums;
+using Microsoft.PowerFx.Core.Utils;
+using Xunit;
+using Xunit.Sdk;
+
+namespace Microsoft.PowerFx.Tests
+{
+    public class CustomFunctions : PowerFxTest
+    {
+        [Fact]
+        public void CustomFunction()
+        {
+            var config = new PowerFxConfig(null);
+            config.AddFunction(new TestCustomFunction());
+            var engine = new RecalcEngine(config);
+
+            // Shows up in enuemeration
+            var func = engine.GetAllFunctionNames().First(name => name == "TestCustom");
+            Assert.NotNull(func);
+
+            // Can be invoked. 
+            var result = engine.Eval("TestCustom(2,3)");
+            Assert.Equal(6.0, result.ToObject());
+        }
+
+        // Must have "Function" suffix. 
+        private class TestCustomFunction : ReflectionFunction
+        {
+            // Must have "Execute" method. 
+            public static NumberValue Execute(NumberValue x, NumberValue y)
+            {
+                var val = x.Value * y.Value;
+                return FormulaValue.New(val);
+            }
+        }
+
+        // Verify we can add overloads of a custom function. 
+        [Theory]
+        [InlineData("SetField(123)", "SetFieldNumberFunction,123")]
+        [InlineData("SetField(\"-123\")", "SetFieldStrFunction,-123")]
+        [InlineData("SetField(\"abc\")", "SetFieldStrFunction,abc")]
+        [InlineData("SetField(true)", "SetFieldNumberFunction,1")] // true coerces to number 1
+        public void Overloads(string expr, string expected)
+        {
+            var config = new PowerFxConfig();
+            config.AddFunction(new SetFieldNumberFunction());
+            config.AddFunction(new SetFieldStrFunction());
+            var engine = new RecalcEngine(config);
+
+            var count = engine.GetAllFunctionNames().Count(name => name == "SetField");
+            Assert.Equal(1, count); // no duplicates
+
+            // Duplicates?
+            var result = engine.Eval(expr);
+            var actual = ((StringValue)result).Value;
+
+            Assert.Equal(expected, actual);
+        }
+
+        private abstract class SetFieldBaseFunction : ReflectionFunction
+        {
+            public SetFieldBaseFunction(FormulaType fieldType) 
+                : base("SetField", FormulaType.String, fieldType)
+            {                
+            }
+
+            public StringValue Execute(FormulaValue newValue)
+            {
+                var overload = GetType().Name;
+                var result = overload + "," + newValue.ToObject().ToString();
+                return FormulaValue.New(result);
+            }
+        }
+
+        private class SetFieldNumberFunction : SetFieldBaseFunction
+        {
+            public SetFieldNumberFunction()
+                : base(FormulaType.Number)
+            {
+            }
+        }
+
+        private class SetFieldStrFunction : SetFieldBaseFunction
+        {
+            public SetFieldStrFunction()
+                : base(FormulaType.String)
+            {
+            }
+        }
+    }
+}

src/tests/Microsoft.PowerFx.Interpreter.Tests/RecalcEngineTests.cs

@@ -394,33 +394,6 @@ namespace Microsoft.PowerFx.Tests
             Assert.Empty(result.TopLevelIdentifiers);
         }
 
-        [Fact]
-        public void CustomFunction()
-        {
-            var config = new PowerFxConfig(null);
-            config.AddFunction(new TestCustomFunction());
-            var engine = new RecalcEngine(config);
-
-            // Shows up in enuemeration
-            var func = engine.GetAllFunctionNames().First(name => name == "TestCustom");
-            Assert.NotNull(func);
-
-            // Can be invoked. 
-            var result = engine.Eval("TestCustom(2,3)");
-            Assert.Equal(6.0, result.ToObject());
-        }
-
-        // Must have "Function" suffix. 
-        private class TestCustomFunction : ReflectionFunction
-        {
-            // Must have "Execute" method. 
-            public static NumberValue Execute(NumberValue x, NumberValue y)
-            {
-                var val = x.Value * y.Value;
-                return FormulaValue.New(val);
-            }
-        }
-
         [Fact]
         public void CheckBindErrorWithParseExpression()
         {