Error on excess spread arguments #20071
Conversation
Make the *technically* correct construction illegal: ```ts declare function f(n: number): void; declare var ns: number[]; f(1, ...ns); ``` This call only makes sense if `ns = []`, but in that case, why pass `ns` at all? Allowing this call masks other errors when functions are refactored to have fewer parameters, or to stop using rest parameters: ```ts declare function old(...ns: number[]): void; declare function new(ns: number | number[]): void; old(1, ...ns); // Fine! new(1, ...ns); // Should error! ``` This change the error for excess spread arguments to be more understandable: "Expected 3 arguments, but got least 4". Previously the error would have been "Expected 3 argument, but got at least 3", which is, again, technically correct, but not understandable.
| normal("g", ...ns) | ||
| normal("h", ...mixed) |
There was a problem hiding this comment.
Should these mixed and tuple cases not still be somewhere? (If only to assert they behave similarly)
There was a problem hiding this comment.
No, correctness is determined syntactically, so the types of the spread arguments don't matter.
| thunk(...mixed) | ||
| thunk(...tuple) | ||
| ~~~~~~~~~~~~ | ||
| !!! error TS2556: Expected 0 arguments, but got a minimum of 1. |
There was a problem hiding this comment.
This may be pedantic, but then again so is this error: I think it'd be more clear to say Expected N arguments, but got Y or more. when a spread is involved (where N is the maximum arity, and Y is the number of non-spread arguments).
There was a problem hiding this comment.
Hm, good idea. I'll see how that looks if I change all of them.
There was a problem hiding this comment.
Yes, it is much easier to read. Nice suggestion. (I feel like it's a tiiny bit less accurate, but that doesn't matter much.)
Thanks @weswigham for the improved wording.
Fixes #19900
Make the following call with spread illegal:
This call, while technically legal, only makes sense if
ns = [], but in that case, why passnsat all? Allowing this call masks other errors when functions are refactored to have fewer parameters, or to stop using rest parameters:This PR also changes the error for excess spread arguments to be more understandable:
"Expected 3 arguments, but got least 4".
Previously the error would have been "Expected 3 argument, but got at least 3", which is, again, technically correct, but not understandable.