Skip to content

Commit f1583f0

Browse files
authored
Signature help turns off current-parameter display for non-trailing rest parameters (microsoft#42592)
* Signature help: support non-trailing rest parameters In signature help, the first rest parameter will always be the *last* 'current' parameter (disregarding types). Previously, the signature help current-parameter highlight was only correct for trailing rest parameters. However, with tuple types, you can now create non-trailing rest parameters. This PR now correctly highlights non-trailing rest parameters as the last 'current' parameter. For example, `names` should be the current parameter in all the calls below: ```ts declare function loading(...args: [...names: string[], allCaps: boolean, extra: boolean]): void; leading(/**/ leading('one', /**/ leading('one', 'two', /**/ ``` And, because signature help doesn't do real overload resolution, `names` is also the current parameter for other calls: ```ts leading(1, 2, 3, 'ill-typed', /**/ leading('fine', true, /**/ ``` * Change 'variadic' to 'rest' * fix missed rename * use single, original tuple instead * Revert "use single, original tuple instead" This reverts commit f0896f3. * Improve sig help of trailing rest too 1. Trailing rest keeps highlight at end instead of going off the end. 2. Non-trailing rest disable highlight entirely (by putting the index one past the end). * update API baselines
1 parent 215ee40 commit f1583f0

File tree

6 files changed

+90
-6
lines changed

6 files changed

+90
-6
lines changed

src/services/signatureHelp.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,6 @@ namespace ts.SignatureHelp {
517517
if (argumentIndex !== 0) {
518518
Debug.assertLessThan(argumentIndex, argumentCount);
519519
}
520-
521520
let selectedItemIndex = 0;
522521
let itemsSeen = 0;
523522
for (let i = 0; i < items.length; i++) {
@@ -541,8 +540,19 @@ namespace ts.SignatureHelp {
541540
}
542541

543542
Debug.assert(selectedItemIndex !== -1); // If candidates is non-empty it should always include bestSignature. We check for an empty candidates before calling this function.
544-
545-
return { items: flatMapToMutable(items, identity), applicableSpan, selectedItemIndex, argumentIndex, argumentCount };
543+
const help = { items: flatMapToMutable(items, identity), applicableSpan, selectedItemIndex, argumentIndex, argumentCount };
544+
const selected = help.items[selectedItemIndex];
545+
if (selected.isVariadic) {
546+
const firstRest = findIndex(selected.parameters, p => !!p.isRest);
547+
if (-1 < firstRest && firstRest < selected.parameters.length - 1) {
548+
// We don't have any code to get this correct; instead, don't highlight a current parameter AT ALL
549+
help.argumentIndex = selected.parameters.length;
550+
}
551+
else {
552+
help.argumentIndex = Math.min(help.argumentIndex, selected.parameters.length - 1);
553+
}
554+
}
555+
return help;
546556
}
547557

548558
function createTypeHelpItems(
@@ -638,15 +648,16 @@ namespace ts.SignatureHelp {
638648
const param = checker.symbolToParameterDeclaration(parameter, enclosingDeclaration, signatureHelpNodeBuilderFlags)!;
639649
printer.writeNode(EmitHint.Unspecified, param, sourceFile, writer);
640650
});
641-
const isOptional = checker.isOptionalParameter(<ParameterDeclaration>parameter.valueDeclaration);
642-
return { name: parameter.name, documentation: parameter.getDocumentationComment(checker), displayParts, isOptional };
651+
const isOptional = checker.isOptionalParameter(parameter.valueDeclaration as ParameterDeclaration);
652+
const isRest = !!((parameter as TransientSymbol).checkFlags & CheckFlags.RestParameter);
653+
return { name: parameter.name, documentation: parameter.getDocumentationComment(checker), displayParts, isOptional, isRest };
643654
}
644655

645656
function createSignatureHelpParameterForTypeParameter(typeParameter: TypeParameter, checker: TypeChecker, enclosingDeclaration: Node, sourceFile: SourceFile, printer: Printer): SignatureHelpParameter {
646657
const displayParts = mapToDisplayParts(writer => {
647658
const param = checker.typeParameterToDeclaration(typeParameter, enclosingDeclaration, signatureHelpNodeBuilderFlags)!;
648659
printer.writeNode(EmitHint.Unspecified, param, sourceFile, writer);
649660
});
650-
return { name: typeParameter.symbol.name, documentation: typeParameter.symbol.getDocumentationComment(checker), displayParts, isOptional: false };
661+
return { name: typeParameter.symbol.name, documentation: typeParameter.symbol.getDocumentationComment(checker), displayParts, isOptional: false, isRest: false };
651662
}
652663
}

src/services/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,6 +1078,7 @@ namespace ts {
10781078
documentation: SymbolDisplayPart[];
10791079
displayParts: SymbolDisplayPart[];
10801080
isOptional: boolean;
1081+
isRest?: boolean;
10811082
}
10821083

10831084
export interface SelectionRange {

tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6025,6 +6025,7 @@ declare namespace ts {
60256025
documentation: SymbolDisplayPart[];
60266026
displayParts: SymbolDisplayPart[];
60276027
isOptional: boolean;
6028+
isRest?: boolean;
60286029
}
60296030
interface SelectionRange {
60306031
textSpan: TextSpan;

tests/baselines/reference/api/typescript.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6025,6 +6025,7 @@ declare namespace ts {
60256025
documentation: SymbolDisplayPart[];
60266026
displayParts: SymbolDisplayPart[];
60276027
isOptional: boolean;
6028+
isRest?: boolean;
60286029
}
60296030
interface SelectionRange {
60306031
textSpan: TextSpan;
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////export function leading(...args: [...names: string[], allCaps: boolean]): void {
4+
////}
5+
////
6+
////leading(/*1*/);
7+
////leading("ok", /*2*/);
8+
////leading("ok", "ok", /*3*/);
9+
10+
verify.signatureHelp(
11+
{
12+
marker: "1",
13+
text: "leading(...names: string[], allCaps: boolean): void",
14+
overloadsCount: 1,
15+
parameterCount: 2,
16+
isVariadic: true,
17+
},
18+
{
19+
marker: "2",
20+
text: "leading(...names: string[], allCaps: boolean): void",
21+
overloadsCount: 1,
22+
parameterCount: 2,
23+
isVariadic: true,
24+
},
25+
{
26+
marker: "3",
27+
text: "leading(...names: string[], allCaps: boolean): void",
28+
overloadsCount: 1,
29+
parameterCount: 2,
30+
isVariadic: true,
31+
},
32+
);
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////export function leading(allCaps: boolean, ...names: string[]): void {
4+
////}
5+
////
6+
////leading(/*1*/);
7+
////leading(false, /*2*/);
8+
////leading(false, "ok", /*3*/);
9+
10+
verify.signatureHelp(
11+
{
12+
marker: "1",
13+
text: "leading(allCaps: boolean, ...names: string[]): void",
14+
overloadsCount: 1,
15+
parameterCount: 2,
16+
parameterName: "allCaps",
17+
parameterSpan: "allCaps: boolean",
18+
isVariadic: true,
19+
},
20+
{
21+
marker: "2",
22+
text: "leading(allCaps: boolean, ...names: string[]): void",
23+
overloadsCount: 1,
24+
parameterCount: 2,
25+
parameterName: "names",
26+
parameterSpan: "...names: string[]",
27+
isVariadic: true,
28+
},
29+
{
30+
marker: "3",
31+
text: "leading(allCaps: boolean, ...names: string[]): void",
32+
overloadsCount: 1,
33+
parameterCount: 2,
34+
parameterName: "names",
35+
parameterSpan: "...names: string[]",
36+
isVariadic: true,
37+
},
38+
);

0 commit comments

Comments
 (0)