-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Description
Suggestion
The error message when passing a function which takes 3 arguments to a position which expects 2 just says "[...] is not assignable", instead of explaining why. It would be nice if the error message would point out the mismatch.
(This isn't exactly a "bug" or a "feature request"; rather it's a request for better errors. The templates aren't exactly suited for that.)
🔍 Search Terms
label:"Domain: Error Messages" number arguments parameters callback function
✅ Viability Checklist
My suggestion meets these guidelines:
- This wouldn't be a breaking change in existing TypeScript/JavaScript code
- This wouldn't change the runtime behavior of existing JavaScript code
- This could be implemented without emitting different JS based on the types of the expressions
- This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
- This feature would agree with the rest of TypeScript's Design Goals.
⭐ Suggestion
📃 Motivating Example
function takesCallback(fn: (a: number) => void) {
// ...
}
takesCallback((a: number, b: number) => {});The error message for this file is just
error.ts:5:15 - error TS2345: Argument of type '(a: number, b: number) => void' is not assignable to parameter of type '(a: number) => void'.
It would be nice if the error pointed out the different number of parameters.
In this particular case you can probably notice the problem by inspection, but it gets harder to tell when the types are more complicated, like in this example from real code.
In that example the relevant error is
Argument of type '(state: any, event: any) => void' is not assignable to parameter of type '(state: State<unknown, EventObject, any, Typestate<unknown>, unknown>) => void'.
and it's easy to miss that the second type actually only names a single parameter, despite all the commas in it. The error message could say something like "function type takes 2 arguments, but this position requires a function which takes at most 1 argument".
💻 Use Cases
This comes up pretty often.
Sidebar: I'd be happy to take a go at implementing this myself, but I could not for the life of me figure out where this check happens. If someone points me to the appropriate place I would be happy to submit a PR.
Edit: thanks to Nathan Shively-Sanders I am pretty sure the way to fix this is by adding a call to reportError in the bit of compareSignaturesRelated which looks like
if (sourceHasMoreParameters) {
return Ternary.False;
}as in
if (sourceHasMoreParameters) {
if (reportErrors) {
errorReporter!(Diagnostics.Call_signature_0_expects_more_arguments_than_signature_1, signatureToString(source), signatureToString(target));
}
return Ternary.False;
}where Call_signature_0_expects_more_arguments_than_signature_1 would be a new diagnostic. (And of course you'd have to branch on call vs construct and give the appropriate message in each case, which would require threading some additional context through.)
I'll hold off on actually sending a PR until/unless this issue gets approved, though.