Show expanded parameter lists in signature help#30299
Conversation
weswigham
left a comment
weswigham
left a comment
There was a problem hiding this comment.
We should probably add a fourslash test this affects.
|
@weswigham Yes, except I've spent enough time in fourslash hell to last a lifetime. |
|
Guess this #30084 is no longer needed then :( @ahejlsberg |
| } | ||
| }); | ||
| const parameters = candidateSignature.parameters.map(p => createSignatureHelpParameterForParameter(p, checker, enclosingDeclaration, sourceFile, printer)); | ||
| const parameters = checker.getExpandedParameters(candidateSignature).map(p => createSignatureHelpParameterForParameter(p, checker, enclosingDeclaration, sourceFile, printer)); |
There was a problem hiding this comment.
Not sure if this matters or not, but isVariadic will be true for functions with expanded parameters even though the expanded signature will not always be. Not sure if this will be an issue, in my similar PR, I reevaluated whether the signature is variadic based on the last argument. I found this with the fourslash test I added, don't know if any clients of the API will suffer from isVariadic not being entirely acurate in this case.
There was a problem hiding this comment.
We don't use the isVariadic property ourselves but it is part of the public API so we should make sure it is set correctly.
|
@dragomirtitian Sorry, I completely missed that you already had created a PR for this issue. Yours has tests and handles the |
|
@ahejlsberg No problem these things happen :). I'm happy if you think I can still contribute in some way. I will fix the PR shortly. |
3 participants
The PR fixes signature help to show expanded parameter lists for rest parameters of tuple types (reported here #30215 (comment)). For example, instead of
(...args: [string]) => stringwe now show(a: string) => string. This is similar to what we already do in quick info.