fix #19220: polish arity error message

[HerringtonDarkholme]

Fix #19220 .

Only overloaded call without spread arguments has new error message. I also change the wording so the message seems more concise than that proposed in #19220. It's up to @DanielRosenwasser.

For single non-overloaded call, the original error message is better than the new one because it provides the range of acceptable argument number.

I cannot find a proper description for spread argument call. The message No overload accepts {0} arguments requires accurate argument number which isn't available in spread argument. So current error message is better IMHO.

Finally, range of argument number in overload function call isn't provided. Showing that range is too complex to preserve a sensible error message.

Thanks for your review in advance!

[sandersn]

I would argue that this incorrect error is (1) rare (2) not that bad of an error message, and that this fix adds more complexity than it's worth. The bad error makes the compiler look silly, but it's not that confusing because it's so obviously incorrect. A quick look at the overload definitions will show which overloads exist -- and that quick look was always needed until recently. The pathological case is something like "1-5 arguments expected, but got 3." where only 1 and 5 parameter-overloads are defined, but nothing in between. But I expect this case to be vanishingly rare. The normal case just requires the user to change from 2 to 1 or 3.

Even if we do decide that "1-3 arguments expected, but got 2." needs to be fixed, the fixed error should not replace errors like "1-2 arguments expected, but got 3" or "... but got 0". These errors are just as correct, are more compact, and convey more information.

[HerringtonDarkholme]

Fair argument. Should I close this pull request?

[sandersn]

No, I had a long discussion with @mhegazy and we decided that reporting ranges that include overloads is not that useful. Here's what I'd like to see:

1. Convert the new error to an elaboration (using chainDiagnosticMessages) with the wording "Got 3 arguments, but no overload matched." then "The closest overload requires ${n} parameters."
2. Recursively call the existing code on a single overload. The overload should be the one with the minimum number of parameters that is greater than the number of arguments, if there is one. Otherwise it should be the overload with the greatest number of parameters. I'm not sure what to do about spread calls.
3. Modify the existing code to operate on a single signature. It should generate strings of the form "n" or "n-m"; the caller may still need to figure out whether to use an "at least" or "a minimum of" message based on the presence of rest/spread.
4. Apply this change to type arguments as well, simplifying as needed since there is no rest/spread (YET 👩‍🚀).

Examples:

• f: { (a,b,c); (a,b,c,d,e) } called with f(1,2): "The closest overload requires 3 parameters".
• f: { (a,b,c?); (a,b,c,d,e) } called with f(1,2,3,4): "The closest overload requires 5 parameters".
• f: { (a,b,c?); (a,b,c,d,e) } called with f(1): "The closest overload requires 1-2 parameters".
• f: { (a,b,c); (a,b,c,d) } called with f(1,2,3,4,5): "The closest overload requires 4 parameters".
• f: { (a,b,...c); (a,b,c,d) } called with f(1): "The closest overload requires at least 2 parameters".

Note that @mhegazy and I discussed applying this technique to both overloads and optional/rest parameters. That is, give an elaboration for optional parameters as well: "Got 1 arguments, but no signature matched. \n At least 2 parameters are required." This would change error reporting considerably, so I'd have to see it decide if it's a good idea. And it's significantly more complicated to implement, so I think it would be better to try the above technique instead.

In the meantime, I will:

1. For the existing error messages, flip the wording so that it reads "Got 3 arguments, but expected 2." @mhegazy suggested this to make the reading order more logical.
2. Apply the existing error messages to extends clause checking as well; I forgot to do this when adding them, so they use an even older message.

[HerringtonDarkholme]

Sorry for the late reply. But I'm afraid I am and will be quite busy these day. So I won't have time to refine this pull request.

So let me close this and leave it for other contributors who are interested. Thank you!

[sandersn]

No problem; I have 2 other arity fixes that I need to work on, so I will start from this code when I do.

Here's the summary for cross-referencing purposes:

1. Flip error messages and mention the erroneous function. "${functionName} got 2 arguments, but expected 3-4". Compiler gives two different errors for wrong number of type arguments #19374
2. Disallow spreads in calls that already have enough arguments: f(1, ...rest) where f = n => n + 1 Excess spread arguments are not errored #19900