In doc generation, `#methodname(ArgType)` can link to the wrong implementation if the arg count is the same

[jneen]

Bug Report

class A
  # see `#foo(String)`
  def bar
  end

  def foo(x : Int)
  end

  def foo(x : String)
  end
end

Generates the following html under crystal docs (snipped):

<p>see <code><a href="A.html#foo%28x%3AInt%29-instance-method">#foo(String)</a></code></p>

This is linking to #foo(Int) instead of #foo(String) because it happens to be first.

Implementing overload detection like this might be a bit of a lift though - it doesn't look like argument type information is passed into Crystal::Type#lookup_method at any point.

[jneen]

A workaround thanks to @Blacksmoke16 is to manually insert a markdown link. Ugly, but it links to the correct place.

# see [`#foo(String)`](#foo%29x%3AString%29-instance-method)

[straight-shoota]

@Blacksmoke16 Shouldn't this be a bug?
I think it would be different if an unspecific #foo would map to an arbitrary overload. But #foo(String) very clearly targets a specific overload and matching to a different one looks like a bug to me...
This classification is not a big deal either way, I'm just wondering if I'm missing something here?

[Blacksmoke16]

My understanding is the current implementation just sees #foo(String) as "take me to the first overload of #foo that has a single parameter." I.e. the value within the () could be anything. Making it aware that if it's a string that matches the type restriction of the nth parameter is a net-new feature. Assuming that is how we want to handle it.

[straight-shoota]

But this interpretation, even if it's intentional, doesn't really make sense. #foo(String) is an unambiguous signature and should not match an arbitrary other signature.

[jneen]

Having looked through the code I could see this being an "unimplemented feature" but it could go either way really. The intention of the doc author from #foo(String) is clear though.

Likely the parser will have to interpret capitalized words differently, as well as handle #foo(x : String) and probably a handful of other argument typing things (varargs, keyword args, various splats, etc). If it's possible to use some of the actual method resolution machinery for this, it would probably be best - I'm pretty sure we just have a list of regular Crystal::Def objects here.

[Blacksmoke16]

Technically we might need to go a step further than #foo(String) to handle the case of like:

def foo(a : String); end
def foo(b : Int32); end
def foo(b : String); end

So here's my proposal:

1. If the string you provide matches the parameter name of a method of the same arity, use the first match
   • #foo(b) # => second overload
2. If the string you provided matches the type of a parameter of a method of the same arity, use the first match
   • #foo(Int32) # => second overload
   • #foo(String) # => first overload
3. If the string you provide matches both name and type of a method of the same arity, use the first match
   • #foo(b : String) # => third overload
4. If nothing matches, use current logic and match first overload of the same arity, whatever the name of the parameter is