From f1583f08a08e6c327b14e647fed6d6aeca8fb996 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Fri, 5 Feb 2021 09:37:28 -0800 Subject: [PATCH] Signature help turns off current-parameter display for non-trailing rest parameters (#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 f0896f32ea3d523f1186e9bea2446f75f3a182de. * 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 --- src/services/signatureHelp.ts | 23 ++++++++--- src/services/types.ts | 1 + .../reference/api/tsserverlibrary.d.ts | 1 + tests/baselines/reference/api/typescript.d.ts | 1 + .../signatureHelpLeadingRestTuple.ts | 32 ++++++++++++++++ .../signatureHelpTrailingRestTuple.ts | 38 +++++++++++++++++++ 6 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 tests/cases/fourslash/signatureHelpLeadingRestTuple.ts create mode 100644 tests/cases/fourslash/signatureHelpTrailingRestTuple.ts diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index c8b333d0356..9eb93800605 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -517,7 +517,6 @@ namespace ts.SignatureHelp { if (argumentIndex !== 0) { Debug.assertLessThan(argumentIndex, argumentCount); } - let selectedItemIndex = 0; let itemsSeen = 0; for (let i = 0; i < items.length; i++) { @@ -541,8 +540,19 @@ namespace ts.SignatureHelp { } Debug.assert(selectedItemIndex !== -1); // If candidates is non-empty it should always include bestSignature. We check for an empty candidates before calling this function. - - return { items: flatMapToMutable(items, identity), applicableSpan, selectedItemIndex, argumentIndex, argumentCount }; + const help = { items: flatMapToMutable(items, identity), applicableSpan, selectedItemIndex, argumentIndex, argumentCount }; + const selected = help.items[selectedItemIndex]; + if (selected.isVariadic) { + const firstRest = findIndex(selected.parameters, p => !!p.isRest); + if (-1 < firstRest && firstRest < selected.parameters.length - 1) { + // We don't have any code to get this correct; instead, don't highlight a current parameter AT ALL + help.argumentIndex = selected.parameters.length; + } + else { + help.argumentIndex = Math.min(help.argumentIndex, selected.parameters.length - 1); + } + } + return help; } function createTypeHelpItems( @@ -638,8 +648,9 @@ namespace ts.SignatureHelp { const param = checker.symbolToParameterDeclaration(parameter, enclosingDeclaration, signatureHelpNodeBuilderFlags)!; printer.writeNode(EmitHint.Unspecified, param, sourceFile, writer); }); - const isOptional = checker.isOptionalParameter(parameter.valueDeclaration); - return { name: parameter.name, documentation: parameter.getDocumentationComment(checker), displayParts, isOptional }; + const isOptional = checker.isOptionalParameter(parameter.valueDeclaration as ParameterDeclaration); + const isRest = !!((parameter as TransientSymbol).checkFlags & CheckFlags.RestParameter); + return { name: parameter.name, documentation: parameter.getDocumentationComment(checker), displayParts, isOptional, isRest }; } function createSignatureHelpParameterForTypeParameter(typeParameter: TypeParameter, checker: TypeChecker, enclosingDeclaration: Node, sourceFile: SourceFile, printer: Printer): SignatureHelpParameter { @@ -647,6 +658,6 @@ namespace ts.SignatureHelp { const param = checker.typeParameterToDeclaration(typeParameter, enclosingDeclaration, signatureHelpNodeBuilderFlags)!; printer.writeNode(EmitHint.Unspecified, param, sourceFile, writer); }); - return { name: typeParameter.symbol.name, documentation: typeParameter.symbol.getDocumentationComment(checker), displayParts, isOptional: false }; + return { name: typeParameter.symbol.name, documentation: typeParameter.symbol.getDocumentationComment(checker), displayParts, isOptional: false, isRest: false }; } } diff --git a/src/services/types.ts b/src/services/types.ts index dd97197a924..f1ca5f73727 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -1078,6 +1078,7 @@ namespace ts { documentation: SymbolDisplayPart[]; displayParts: SymbolDisplayPart[]; isOptional: boolean; + isRest?: boolean; } export interface SelectionRange { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index e8227710675..4419cf5a9cc 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -6025,6 +6025,7 @@ declare namespace ts { documentation: SymbolDisplayPart[]; displayParts: SymbolDisplayPart[]; isOptional: boolean; + isRest?: boolean; } interface SelectionRange { textSpan: TextSpan; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index b8525b18064..f603ef46bac 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -6025,6 +6025,7 @@ declare namespace ts { documentation: SymbolDisplayPart[]; displayParts: SymbolDisplayPart[]; isOptional: boolean; + isRest?: boolean; } interface SelectionRange { textSpan: TextSpan; diff --git a/tests/cases/fourslash/signatureHelpLeadingRestTuple.ts b/tests/cases/fourslash/signatureHelpLeadingRestTuple.ts new file mode 100644 index 00000000000..45c3382d32f --- /dev/null +++ b/tests/cases/fourslash/signatureHelpLeadingRestTuple.ts @@ -0,0 +1,32 @@ +/// + +////export function leading(...args: [...names: string[], allCaps: boolean]): void { +////} +//// +////leading(/*1*/); +////leading("ok", /*2*/); +////leading("ok", "ok", /*3*/); + +verify.signatureHelp( + { + marker: "1", + text: "leading(...names: string[], allCaps: boolean): void", + overloadsCount: 1, + parameterCount: 2, + isVariadic: true, + }, + { + marker: "2", + text: "leading(...names: string[], allCaps: boolean): void", + overloadsCount: 1, + parameterCount: 2, + isVariadic: true, + }, + { + marker: "3", + text: "leading(...names: string[], allCaps: boolean): void", + overloadsCount: 1, + parameterCount: 2, + isVariadic: true, + }, +); diff --git a/tests/cases/fourslash/signatureHelpTrailingRestTuple.ts b/tests/cases/fourslash/signatureHelpTrailingRestTuple.ts new file mode 100644 index 00000000000..86dde9b3982 --- /dev/null +++ b/tests/cases/fourslash/signatureHelpTrailingRestTuple.ts @@ -0,0 +1,38 @@ +/// + +////export function leading(allCaps: boolean, ...names: string[]): void { +////} +//// +////leading(/*1*/); +////leading(false, /*2*/); +////leading(false, "ok", /*3*/); + +verify.signatureHelp( + { + marker: "1", + text: "leading(allCaps: boolean, ...names: string[]): void", + overloadsCount: 1, + parameterCount: 2, + parameterName: "allCaps", + parameterSpan: "allCaps: boolean", + isVariadic: true, + }, + { + marker: "2", + text: "leading(allCaps: boolean, ...names: string[]): void", + overloadsCount: 1, + parameterCount: 2, + parameterName: "names", + parameterSpan: "...names: string[]", + isVariadic: true, + }, + { + marker: "3", + text: "leading(allCaps: boolean, ...names: string[]): void", + overloadsCount: 1, + parameterCount: 2, + parameterName: "names", + parameterSpan: "...names: string[]", + isVariadic: true, + }, +);