Repository navigation
Improve error when function overload set incorrectly authored #4797
Description
Users do not realize that implementation signatures of overload sets do not count as an actual signature at call sites. Attempting to call their overloaded function then results in a surprising error and off they go to StackOverflow to ask a question or here to file a bug. This comes up fairly often (most recently with #4786).
Can we improve the error by checking the type at the callsite against the implementation signature if no other signatures are applicable? Ex:
class EventAggregator
{
publish(event: string, data?: any): void;
publish<T>(event: T): void {}
}
var ea: EventAggregator;
ea.publish([1,2,3]); Today the last line is an error: "number[] is not assignable to string." To many this just looks like the compiler for some reason chose the wrong overload, now what?
In this case after seeing array is not assignable to any overload signatures we could check whether it is assignable to the implementation signature. If it is, we could issue a potentially more helpful error mentioning this rule (implementation signatures are not considered overload signatures) and a potential fix.
Activity
omidkrad commentedon Sep 15, 2015
Not suggesting that I want it now, but I especially like the kind of errors that include a link to an online page with more details on the error code, examples of when this error may happen and possible solutions. Makes everybody's lives easier. Also not suggesting that GitHub wikis are perfect for that! 😉
mhegazy commentedon Sep 15, 2015
Dan Quirk (@danquirk) what is the proposed new error message?
DanielRosenwasser commentedon Sep 15, 2015
Maybe:
danquirk commentedon Sep 15, 2015
That's a pretty good start. Really depends on how verbose/casual we want to be. Even the concept of implementation signature vs overload signature is not necessarily obvious to people coming from certain other languages. A squiggle/line number reference to the implementation signature could help. This would also be a good case for a Lightbulb/QuickFix type thing that C# currently does.
omidkrad commentedon Sep 15, 2015
For me, Mohamed Hegazy (@mhegazy)'s two point hint was very useful:
mhegazy commentedon Sep 15, 2015
The error we get today is:
so we want to say something about the implementation signature is not part of the overloads, in additional to it being compatible.
so may be something like:
danquirk commentedon Sep 15, 2015
I think the distinction of implementation signature vs overload signature still isn't necessarily obvious to people. Perhaps if we mention the type (ex 'with implementation signature of type T -> void') that would help clue people in to precisely what we mean.
mhegazy commentedon Sep 15, 2015
not sure i understand what you mean, which type?
DanielRosenwasser commentedon Sep 15, 2015
Mohamed Hegazy (@mhegazy) the type of the call signature implied by the implementation.
mhegazy commentedon Sep 15, 2015
so something like
danquirk commentedon Sep 15, 2015
Right, because the distinction between implementation signature and overload signature is not a concept that's familiar coming from many other languages (which is why people keep messing this up). So it would be useful to include some other clue as to what we mean when we say implementation signature whether it's the type of the implementation signature or the line number.
So combining some of the conversation, imagine:
number[] is not assignable to string. An applicable call signature does exist on the implementation signature with type { (event: T): void; }. However, implementation signatures are not part of the available function overloads. Consider declaring an additional function overload signature with that type.
Note this is far more verbose than we usually are and somewhat more colloquial but I think that style is for the best (relevant: http://elm-lang.org/blog/compiler-errors-for-humans)
10 remaining items
DanielRosenwasser commentedon Oct 1, 2020
Changed the effort tag because you really do need to understand where to plug things in in overload resolution.