diff --git a/src/DynamicExpresso.Core/Identifier.cs b/src/DynamicExpresso.Core/Identifier.cs index 85b95e16..facba55f 100644 --- a/src/DynamicExpresso.Core/Identifier.cs +++ b/src/DynamicExpresso.Core/Identifier.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq.Expressions; +using System.Reflection; namespace DynamicExpresso { @@ -56,9 +57,35 @@ internal MethodGroupExpression(Delegate overload) internal void AddOverload(Delegate overload) { - // don't register the same overload twice - if (_overloads.IndexOf(overload) == -1) - _overloads.Add(overload); + // remove any existing delegate with the exact same signature + RemoveDelegateSignature(overload); + _overloads.Add(overload); + } + + private void RemoveDelegateSignature(Delegate overload) + { + _overloads.RemoveAll(del => HasSameSignature(overload.Method, del.Method)); + } + + private static bool HasSameSignature(MethodInfo method, MethodInfo other) + { + if (method.ReturnType != other.ReturnType) + return false; + + var param = method.GetParameters(); + var oParam = other.GetParameters(); + if (param.Length != oParam.Length) + return false; + + for (var i = 0; i < param.Length; i++) + { + var p = param[i]; + var q = oParam[i]; + if (p.ParameterType != q.ParameterType || p.HasDefaultValue != q.HasDefaultValue) + return false; + } + + return true; } } } diff --git a/src/DynamicExpresso.Core/Interpreter.cs b/src/DynamicExpresso.Core/Interpreter.cs index b8fdcff0..d84107fa 100644 --- a/src/DynamicExpresso.Core/Interpreter.cs +++ b/src/DynamicExpresso.Core/Interpreter.cs @@ -166,6 +166,8 @@ public Interpreter EnableReflection() #region Register identifiers /// /// Allow the specified function delegate to be called from a parsed expression. + /// Overloads can be added (ie. multiple delegates can be registered with the same name). + /// A delegate will replace any delegate with the exact same signature that is already registered. /// /// /// @@ -273,6 +275,50 @@ public Interpreter SetIdentifier(Identifier identifier) return this; } + + /// + /// Remove from the list of known identifiers. + /// + /// > + /// + public Interpreter UnsetFunction(string name) + { + return UnsetIdentifier(name); + } + + /// + /// Remove from the list of known identifiers. + /// + /// > + /// + public Interpreter UnsetVariable(string name) + { + return UnsetIdentifier(name); + } + + /// + /// Remove from the list of known identifiers. + /// + /// > + /// + public Interpreter UnsetExpression(string name) + { + return UnsetIdentifier(name); + } + + /// + /// Remove from the list of known identifiers. + /// + /// + /// + public Interpreter UnsetIdentifier(string name) + { + if (string.IsNullOrWhiteSpace(name)) + throw new ArgumentNullException(nameof(name)); + + _settings.Identifiers.Remove(name); + return this; + } #endregion #region Register referenced types diff --git a/src/DynamicExpresso.Core/Parsing/Parser.cs b/src/DynamicExpresso.Core/Parsing/Parser.cs index a2fffa6c..3a585f7b 100644 --- a/src/DynamicExpresso.Core/Parsing/Parser.cs +++ b/src/DynamicExpresso.Core/Parsing/Parser.cs @@ -1220,12 +1220,12 @@ private Expression ParseMethodGroupInvocation(MethodGroupExpression methodGroup, }) .ToList(); - var applicableMethods = FindBestMethod(candidates.Select(_ => _.InvokeMethod), args); + var applicableMethods = FindBestMethod(candidates.Select(_ => _.Method), args); // no method found: retry with the delegate's method // (the parameters might be different, e.g. params array, default value, etc) if (applicableMethods.Length == 0) - applicableMethods = FindBestMethod(candidates.Select(_ => _.Method), args); + applicableMethods = FindBestMethod(candidates.Select(_ => _.InvokeMethod), args); if (applicableMethods.Length == 0) throw CreateParseException(errorPos, ErrorMessages.ArgsIncompatibleWithDelegate); @@ -1952,9 +1952,14 @@ private static MethodData[] FindBestMethod(IEnumerable methods, Expr ToArray(); if (applicable.Length > 1) { - return applicable. - Where(m => applicable.All(n => m == n || MethodHasPriority(args, m, n))). - ToArray(); + var bestCandidates = applicable + .Where(m => applicable.All(n => m == n || MethodHasPriority(args, m, n))) + .ToArray(); + + // bestCandidates.Length == 0 means that no applicable method has priority + // we don't return bestCandidates to prevent callers from thinking no method was found + if (bestCandidates.Length > 0) + return bestCandidates; } return applicable; diff --git a/test/DynamicExpresso.UnitTest/GithubIssues.cs b/test/DynamicExpresso.UnitTest/GithubIssues.cs index ba7e65d1..56b4cce2 100644 --- a/test/DynamicExpresso.UnitTest/GithubIssues.cs +++ b/test/DynamicExpresso.UnitTest/GithubIssues.cs @@ -173,6 +173,38 @@ public void GitHub_Issue_148() Assert.AreEqual(2, target.Eval("SubArray(arr1, 1, 1).First()")); } + [Test] + public void GitHub_Issue_159_ambiguous_call() + { + Func f1 = d => 1; + Func f2 = o => 2; + + var interpreter = new Interpreter(); + interpreter.SetFunction("f", f1); + interpreter.SetFunction("f", f2); + + // we should properly throw an ambiguous invocation exception (multiple matching overloads found) + // and not an Argument list incompatible with delegate expression (no matching overload found) + var exc = Assert.Throws(() => interpreter.Eval("f(null)")); + StringAssert.StartsWith("Ambiguous invocation of delegate (multiple overloads found)", exc.Message); + } + + + [Test] + public void GitHub_Issue_159_unset_identifier() + { + Func f1 = () => 1; + + var interpreter = new Interpreter(); + interpreter.SetFunction("f", f1); + + Assert.AreEqual(1, interpreter.Eval("f()")); + + // calls to f should lead to an unknown identifier exception + interpreter.UnsetFunction("f"); + Assert.Throws(() => interpreter.Eval("f()")); + } + #if NETCOREAPP2_1_OR_GREATER @@ -186,7 +218,6 @@ static bool GetGFunction2(string arg = null) } GFunction gFunc2 = GetGFunction2; - Assert.False(gFunc2.Method.GetParameters()[0].HasDefaultValue); // should be true! var flags = BindingFlags.Public | BindingFlags.DeclaredOnly | BindingFlags.Instance; var invokeMethod2 = (MethodInfo)gFunc2.GetType().FindMembers(MemberTypes.Method, flags, Type.FilterName, "Invoke")[0]; @@ -204,7 +235,7 @@ static bool GetGFunction2(string arg = null) public void GitHub_Issue_144_3() { // GetGFunction2 is defined inside the test function - static bool GetGFunction2(string arg = null) + static bool GetGFunction2(string arg) { return arg == null; } @@ -220,7 +251,7 @@ static bool GetGFunction2(string arg = null) // ambiguous call Assert.Throws(() => interpreter.Eval("GFunction(arg)")); - // there should be an ambiguous call exception, but GFunction1 is used + // GFunction1 is used // because gFunc1.Method.GetParameters()[0].HasDefaultValue == true // and gFunc2.Method.GetParameters()[0].HasDefaultValue == false Assert.False((bool)interpreter.Eval("GFunction()")); diff --git a/test/DynamicExpresso.UnitTest/VariablesTest.cs b/test/DynamicExpresso.UnitTest/VariablesTest.cs index 3b70e146..cc3ebc58 100644 --- a/test/DynamicExpresso.UnitTest/VariablesTest.cs +++ b/test/DynamicExpresso.UnitTest/VariablesTest.cs @@ -151,6 +151,20 @@ public void Keywords_with_same_overload_twice() Assert.AreEqual(9.0, target.Eval("pow(3, 2)")); } + [Test] + public void Replace_same_overload_signature() + { + Func f1 = d => 1; + Func f2 = d => 2; + + var target = new Interpreter() + .SetFunction("f", f1) + .SetFunction("f", f2); + + // f2 should override the f1 registration, because both delegates have the same signature + Assert.AreEqual(2, target.Eval("f(0d)")); + } + [Test] public void Keywords_with_invalid_delegate_call() {