Improve arity error messages#15861
Conversation
For calls with one signature.
Not just ones with a single call signature. This is pretty great!
The old "supplied parameters do not match any call signature" was either inaccurate, redundant or vague. The previous commits fix the vagueness problem. This commit fixes the inaccuracy and redundancy. 1. When there are NO candidates, the error should say so. (This only happens once in our tests, when calling `super()` with a base class of type `any` in a JS file.) 2. When the call is to a decorator, `resolveCall` already receives a specific fallback error message from the decorator handling code. Adding "supplied parameters do not match ..." is not helpful. I also cleaned up the new code a bit after I noticed that all the error creation functions take `string | number`, so I didn't need calls to `toString` in my code.
|
@RyanCavanaugh @DanielRosenwasser mind taking a look? This PR is pain to keep up to date because it changes so many error numbers ... |
| max = Math.max(max, sig.parameters.length); | ||
| } | ||
| const paramMessage = some(signatures, sig => sig.hasRestParameter) ? "at least " + min : | ||
| /////////// |
There was a problem hiding this comment.
Why not newlines? 🦀 👽 🦑
There was a problem hiding this comment.
(Removed in the current version)
| "category": "Error", | ||
| "code": 2555 | ||
| }, | ||
| "Expected at least {0} arguments, but got a minimum of {1}.": { |
There was a problem hiding this comment.
What does this mean? I can't tell from the message nor the condition in the code. Also, every error message we report this on seems to have "a minimum of 0"
There was a problem hiding this comment.
I used "a minimum of" for spreads to indicate that a spread adds 0 or more arguments to those already present. After reading the example you might have a better idea for the wording.
Here's an example that would give "a minimum of 1":
declare function f(first, second, ...rest): any[]
declare const car: number
declare const cdr: number[]
f(car, ...cdr) // Expected at least 2 arguments, but got a minimum of 1.Note that this is legal Javascript and could succeed at runtime, but we can't prove that it will because cdr might be empty.
| "code": 2553 | ||
| }, | ||
| "Expected {0} type arguments, but got {1}.": { | ||
| "Expected at least {0} arguments, but got {1}.": { |
There was a problem hiding this comment.
"Expected {0} type arguments, but got {1}." is closer to the old message. I'd argue that the error code didn't need to change. Consider switching this with the message assigned to code 2557
There was a problem hiding this comment.
I think the diff is wonky here. All these messages are new, so there aren't any codes to reuse.
| "category": "Error", | ||
| "code": 2556 | ||
| }, | ||
| "Expected {0} type arguments, but got {1}.": { |
There was a problem hiding this comment.
In some cases, type arguments also need "at least" some number. Is that something that would be done in a different PR?
There was a problem hiding this comment.
"at least" indicates a rest argument, which TS doesn't have yet. I use a range, generated in the code, for optional [type] arguments, so you'd see a message like "Expected 1-3 type arguments, but got 4".
|
@DanielRosenwasser I replied to your now-hidden replies, but my replies are now also hidden. (!) |
Add a specific errors for incorrect number of arguments or of type arguments in
resolveCall. I'm not sure how we lived without this before.Update from @DanielRosenwasser on August 3, 2017: fixes #6045.