Change error message when throw when same functions are registered

[sagilio]

Fixes #159

1. Change error message according to ParseException with ArgsIncompatibleWithDelegate message is throw when multiple functions are registered with the same signature #159
2. Public FunctionIdentifier to allow use replace function directly instead of add overload.

@davideicardi @metoule

[davideicardi]

Thank you for your help @sagilio !

I'm just not sure about some details. See my comments.

[davideicardi]

Given the issue description I thought that we should throw AmbiguousDelegateInvocation when applicableMethods.Length > 1 . Why we throw when == 0?
Maybe I missing something .

@metoule maybe you can help us ;-)

[metoule]

That's precisely the issue I highlighted, and why it's actually quite difficult to fix properly :D
The issue resides in the FindBestMethod method. When there are multiple applicable overloads, they're filtered thanks to the MethodHasPriority method, which can also result in an empty array if no overload has the priority:

private static MethodData[] FindBestMethod(IEnumerable<MethodData> methods, Expression[] args)
{
  // empty array if there are no applicable methods
  var applicable = methods
    .Where(m => CheckIfMethodIsApplicableAndPrepareIt(m, args))
    .ToArray();
  if (applicable.Length > 1)
  {
    // also returns an empty array if no overload has priority (ie. if there are multiple applicable methods)!
    return applicable
      .Where(m => applicable.All(n => m == n || MethodHasPriority(args, m, n)))
      .ToArray();
  }

  return applicable;
}

The issue is that the caller of FindBestMethod doesn't know if the resulting array is empty because no function was found, or if multiple applicable overloads were found.

We might be able to fix the issue like this, but it would require checking all calls to FindBestMethod to be sure:

private static MethodData[] FindBestMethod(IEnumerable<MethodData> methods, Expression[] args)
{
  // empty array if there are no applicable methods
  var applicable = methods
    .Where(m => CheckIfMethodIsApplicableAndPrepareIt(m, args))
    .ToArray();
  if (applicable.Length > 1)
  {
    var bestCandidates = applicable
      .Where(m => applicable.All(n => m == n || MethodHasPriority(args, m, n)))
      .ToArray();
    if (bestCandidates.length > 0)
      return bestCandidates; // should always have a length of 1 
  }

  return applicable;
}

[davideicardi]

I like it. Thank you for the explanation. @sagilio Do you think you can review the code to follow this suggestion?

[sagilio]

Thank you for the explanation, I already understand why to return 0 length array here. I think #231 has fixed this well, and will not have much performance impact when the overloads count is not very large.

[metoule]

I've created PR #231 to apply all suggestions.

[metoule]

@sagilio I'm closing this PR, because #231 has now been applied. Let us know if it does fix your issues!