-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Provide better services for incomplete generic calls #16535
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@Andy-MS can you check with @ahejlsberg, he had thoughts about this area for a similar change. |
…have `resolveSignature` always get the best signature if !produceDiagnostics
@@ -69,6 +69,9 @@ namespace ts { | |||
undefinedSymbol.declarations = []; | |||
const argumentsSymbol = createSymbol(SymbolFlags.Property, "arguments"); | |||
|
|||
/** This will be set during calls to `getResolvedSignature` where services determines an apparent number of arguments greater than what is actually provided. */ | |||
let apparentArgumentCount: number | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if we just computed this at the moment we need it, but that seems to involve getting the tokens array, which is currently code exclusive to services. See getArgumentCount
in signatureHelp.ts
.
Per discussion with @ahejlsberg, removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a good change, if a little clunky because of the co-ordination between checker and signature help. I assume that the reason the find-longest-candidate code has to move into the checker is that the instantiation has to happen after it runs, and instantiation only happens in the checker. Is this correct?
All else being equal, I'd like to have language-service-specific heuristics like that kept in the language service, so that's why I'm asking.
src/compiler/checker.ts
Outdated
@@ -15631,22 +15642,35 @@ namespace ts { | |||
// declare function f(a: { xa: number; xb: number; }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comments above here need to be updated and typos fixed (specifically, that → than in one case)
src/compiler/checker.ts
Outdated
@@ -15689,6 +15714,24 @@ namespace ts { | |||
|
|||
} | |||
|
|||
function getBestCandidateIndex(candidates: Signature[], argsCount: number): number { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe call this getLongestCandidateIndex
Unfortunately, it wouldn't be easy to break this out of the checker as we don't just need this behavior in signature help; we also need |
Fixes #15412
Add a
getBestGuessSignature
method to the checker which acts likegetResolvedSignature
, but which does a better job in the case of an error.Normally the type checker will automatically fill in
{}
for type arguments that it can't infer. We now track when we do this, so if we have a Signature we can tell whether it has a bogus instantiation. Would also be useful forno-inferred-empty-object-type
which currently has an unreliable implementation.