Skip to content

Commit 9651c42

Browse files
DanielRosenwasserDanielRosenwasser
committed
Account for trigger reasons, as implemented by Roslyn.
1 parent c046958 commit 9651c42

File tree

6 files changed

+173
-40
lines changed

6 files changed

+173
-40
lines changed

‎src/harness/fourslash.ts

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,17 +1426,22 @@ Actual: ${stringify(fullActual)}`);
14261426
}
14271427
}
14281428

1429-
public verifyNoSignatureHelp(triggerCharacter: ts.SignatureHelpTriggerCharacter | undefined, markers: ReadonlyArray<string>) {
1429+
public verifySignatureHelpPresence(expectPresent: boolean, triggerReason: ts.SignatureHelpTriggerReason | undefined, markers: ReadonlyArray<string>) {
14301430
if (markers.length) {
14311431
for (const marker of markers) {
14321432
this.goToMarker(marker);
1433-
this.verifyNoSignatureHelp(triggerCharacter, ts.emptyArray);
1433+
this.verifySignatureHelpPresence(expectPresent, triggerReason, ts.emptyArray);
14341434
}
14351435
return;
14361436
}
1437-
const actual = this.getSignatureHelp({ triggerCharacter });
1438-
if (actual) {
1439-
this.raiseError(`Expected no signature help, but got "${stringify(actual)}"`);
1437+
const actual = this.getSignatureHelp({ triggerReason });
1438+
if (expectPresent !== !!actual) {
1439+
if (actual) {
1440+
this.raiseError(`Expected no signature help, but got "${stringify(actual)}"`);
1441+
}
1442+
else {
1443+
this.raiseError("Expected signature help, but none was returned.")
1444+
}
14401445
}
14411446
}
14421447

@@ -1455,7 +1460,7 @@ Actual: ${stringify(fullActual)}`);
14551460
}
14561461

14571462
private verifySignatureHelpWorker(options: FourSlashInterface.VerifySignatureHelpOptions) {
1458-
const help = this.getSignatureHelp({ triggerCharacter: options.triggerCharacter })!;
1463+
const help = this.getSignatureHelp({ triggerReason: options.triggerReason })!;
14591464
const selectedItem = help.items[help.selectedItemIndex];
14601465
// Argument index may exceed number of parameters
14611466
const currentParameter = selectedItem.parameters[help.argumentIndex] as ts.SignatureHelpParameter | undefined;
@@ -1497,7 +1502,7 @@ Actual: ${stringify(fullActual)}`);
14971502

14981503
const allKeys: ReadonlyArray<keyof FourSlashInterface.VerifySignatureHelpOptions> = [
14991504
"marker",
1500-
"triggerCharacter",
1505+
"triggerReason",
15011506
"overloadsCount",
15021507
"docComment",
15031508
"text",
@@ -1769,9 +1774,9 @@ Actual: ${stringify(fullActual)}`);
17691774
Harness.IO.log(stringify(help.items[help.selectedItemIndex]));
17701775
}
17711776

1772-
private getSignatureHelp({ triggerCharacter }: FourSlashInterface.VerifySignatureHelpOptions): ts.SignatureHelpItems | undefined {
1777+
private getSignatureHelp({ triggerReason }: FourSlashInterface.VerifySignatureHelpOptions): ts.SignatureHelpItems | undefined {
17731778
return this.languageService.getSignatureHelpItems(this.activeFile.fileName, this.currentCaretPosition, {
1774-
triggerCharacter
1779+
triggerReason
17751780
});
17761781
}
17771782

@@ -1870,7 +1875,12 @@ Actual: ${stringify(fullActual)}`);
18701875
if (highFidelity) {
18711876
if (ch === "(" || ch === "," || ch === "<") {
18721877
/* Signature help*/
1873-
this.languageService.getSignatureHelpItems(this.activeFile.fileName, offset, { triggerCharacter: ch });
1878+
this.languageService.getSignatureHelpItems(this.activeFile.fileName, offset, {
1879+
triggerReason: {
1880+
kind: "characterTyped",
1881+
triggerCharacter: ch
1882+
}
1883+
});
18741884
}
18751885
else if (prevChar === " " && /A-Za-z_/.test(ch)) {
18761886
/* Completions */
@@ -4081,11 +4091,15 @@ namespace FourSlashInterface {
40814091
}
40824092

40834093
public noSignatureHelp(...markers: string[]): void {
4084-
this.state.verifyNoSignatureHelp(/*triggerCharacter*/ undefined, markers);
4094+
this.state.verifySignatureHelpPresence(/*expectPresent*/ false, /*triggerReason*/ undefined, markers);
4095+
}
4096+
4097+
public noSignatureHelpForTriggerReason(reason: ts.SignatureHelpTriggerReason, ...markers: string[]): void {
4098+
this.state.verifySignatureHelpPresence(/*expectPresent*/ false, reason, markers);
40854099
}
40864100

4087-
public noSignatureHelpForTriggerCharacter(triggerCharacter: ts.SignatureHelpTriggerCharacter, ...markers: string[]): void {
4088-
this.state.verifyNoSignatureHelp(triggerCharacter, markers);
4101+
public signatureHelpPresentForTriggerReason(reason: ts.SignatureHelpTriggerReason, ...markers: string[]): void {
4102+
this.state.verifySignatureHelpPresence(/*expectPresent*/ true, reason, markers);
40894103
}
40904104

40914105
public signatureHelp(...options: VerifySignatureHelpOptions[]): void {
@@ -4816,7 +4830,7 @@ namespace FourSlashInterface {
48164830
readonly isVariadic?: boolean;
48174831
/** @default ts.emptyArray */
48184832
readonly tags?: ReadonlyArray<ts.JSDocTagInfo>;
4819-
readonly triggerCharacter?: ts.SignatureHelpTriggerCharacter;
4833+
readonly triggerReason?: ts.SignatureHelpTriggerReason;
48204834
}
48214835

48224836
export type ArrayOrSingle<T> = T | ReadonlyArray<T>;

‎src/server/protocol.ts

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2067,16 +2067,57 @@ namespace ts.server.protocol {
20672067
}
20682068

20692069
export type SignatureHelpTriggerCharacter = "," | "(" | "<";
2070+
export type SignatureHelpRetriggerCharacter = SignatureHelpTriggerCharacter | ")";
2071+
2072+
/**
2073+
* Arguments of a signature help request.
2074+
*/
2075+
export interface SignatureHelpRequestArgs extends FileLocationRequestArgs {
2076+
/**
2077+
* Reason why signature help was invoked.
2078+
* See each individual possible
2079+
*/
2080+
triggerReason?: SignatureHelpTriggerReason;
2081+
}
2082+
2083+
export type SignatureHelpTriggerReason =
2084+
| SignatureHelpInvokedReason
2085+
| SignatureHelpCharacterTypedReason
2086+
| SignatureHelpRetriggeredReason;
20702087

20712088
/**
2072-
* Arguments of a signature help request.
2089+
* Signals that the user manually requested signature help.
2090+
* The language service will unconditionally attempt to provide a result.
20732091
*/
2074-
export interface SignatureHelpRequestArgs extends FileLocationRequestArgs {
2092+
export interface SignatureHelpInvokedReason {
2093+
kind: "invoked",
2094+
triggerCharacter?: undefined,
2095+
}
2096+
2097+
/**
2098+
* Signals that the signature help request came from a user typing a character.
2099+
* Depending on the character and the syntactic context, the request may or may not be served a result.
2100+
*/
2101+
export interface SignatureHelpCharacterTypedReason {
2102+
kind: "characterTyped",
2103+
/**
2104+
* Character that was responsible for triggering signature help.
2105+
*/
2106+
triggerCharacter: SignatureHelpTriggerCharacter,
2107+
}
2108+
2109+
/**
2110+
* Signals that this signature help request came from typing a character or moving the cursor.
2111+
* This should only occur if a signature help session was already active and the editor needs to see if it should adjust.
2112+
* The language service will unconditionally attempt to provide a result.
2113+
* `triggerCharacter` can be `undefined` for a retrigger caused by a cursor move.
2114+
*/
2115+
export interface SignatureHelpRetriggeredReason {
2116+
kind: "retrigger",
20752117
/**
20762118
* Character that was responsible for triggering signature help.
2077-
* Should be `undefined` if a user manually requested completion.
20782119
*/
2079-
triggerCharacter?: SignatureHelpTriggerCharacter;
2120+
triggerCharacter?: SignatureHelpRetriggerCharacter,
20802121
}
20812122

20822123
/**

‎src/services/services.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1756,12 +1756,12 @@ namespace ts {
17561756
/**
17571757
* This is a semantic operation.
17581758
*/
1759-
function getSignatureHelpItems(fileName: string, position: number, { triggerCharacter }: SignatureHelpItemsOptions = emptyOptions): SignatureHelpItems | undefined {
1759+
function getSignatureHelpItems(fileName: string, position: number, { triggerReason }: SignatureHelpItemsOptions = emptyOptions): SignatureHelpItems | undefined {
17601760
synchronizeHostData();
17611761

17621762
const sourceFile = getValidSourceFile(fileName);
17631763

1764-
return SignatureHelp.getSignatureHelpItems(program, sourceFile, position, triggerCharacter, cancellationToken);
1764+
return SignatureHelp.getSignatureHelpItems(program, sourceFile, position, triggerReason, cancellationToken);
17651765
}
17661766

17671767
/// Syntactic features

‎src/services/signatureHelp.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace ts.SignatureHelp {
1919
argumentCount: number;
2020
}
2121

22-
export function getSignatureHelpItems(program: Program, sourceFile: SourceFile, position: number, triggerCharacter: SignatureHelpTriggerCharacter | undefined, cancellationToken: CancellationToken): SignatureHelpItems | undefined {
22+
export function getSignatureHelpItems(program: Program, sourceFile: SourceFile, position: number, triggerReason: SignatureHelpTriggerReason | undefined, cancellationToken: CancellationToken): SignatureHelpItems | undefined {
2323
const typeChecker = program.getTypeChecker();
2424

2525
// Decide whether to show signature help
@@ -29,9 +29,11 @@ namespace ts.SignatureHelp {
2929
return undefined;
3030
}
3131

32-
// In the middle of a string, don't provide signature help unless the user explicitly requested it.
33-
if (triggerCharacter !== undefined && isInString(sourceFile, position, startingToken)) {
34-
return undefined;
32+
if (shouldCarefullyCheckContext(triggerReason)) {
33+
// In the middle of a string, don't provide signature help unless the user explicitly requested it.
34+
if (isInString(sourceFile, position, startingToken)) {
35+
return undefined;
36+
}
3537
}
3638

3739
const argumentInfo = getContainingArgumentInfo(startingToken, position, sourceFile);
@@ -55,6 +57,11 @@ namespace ts.SignatureHelp {
5557
return typeChecker.runWithCancellationToken(cancellationToken, typeChecker => createSignatureHelpItems(candidateInfo.candidates, candidateInfo.resolvedSignature, argumentInfo, sourceFile, typeChecker));
5658
}
5759

60+
function shouldCarefullyCheckContext(reason: SignatureHelpTriggerReason | undefined) {
61+
// Only need to be careful if the user typed a character and signature help wasn't showing.
62+
return !!reason && reason.kind === "characterTyped";
63+
}
64+
5865
function getCandidateInfo(argumentInfo: ArgumentListInfo, checker: TypeChecker): { readonly candidates: ReadonlyArray<Signature>, readonly resolvedSignature: Signature } | undefined {
5966
const { invocation } = argumentInfo;
6067
if (invocation.kind === InvocationKind.Call) {

‎src/services/types.ts

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,13 +383,50 @@ namespace ts {
383383
}
384384

385385
export type SignatureHelpTriggerCharacter = "," | "(" | "<";
386+
export type SignatureHelpRetriggerCharacter = SignatureHelpTriggerCharacter | ")";
386387

387388
export interface SignatureHelpItemsOptions {
389+
triggerReason?: SignatureHelpTriggerReason;
390+
}
391+
392+
export type SignatureHelpTriggerReason =
393+
| SignatureHelpInvokedReason
394+
| SignatureHelpCharacterTypedReason
395+
| SignatureHelpRetriggeredReason;
396+
397+
/**
398+
* Signals that the user manually requested signature help.
399+
* The language service will unconditionally attempt to provide a result.
400+
*/
401+
export interface SignatureHelpInvokedReason {
402+
kind: "invoked",
403+
triggerCharacter?: undefined,
404+
}
405+
406+
/**
407+
* Signals that the signature help request came from a user typing a character.
408+
* Depending on the character and the syntactic context, the request may or may not be served a result.
409+
*/
410+
export interface SignatureHelpCharacterTypedReason {
411+
kind: "characterTyped",
388412
/**
389-
* If the editor is asking for signature help because a certain character was typed
390-
* (as opposed to when the user explicitly requested them) this should be set.
413+
* Character that was responsible for triggering signature help.
414+
*/
415+
triggerCharacter: SignatureHelpTriggerCharacter,
416+
}
417+
418+
/**
419+
* Signals that this signature help request came from typing a character or moving the cursor.
420+
* This should only occur if a signature help session was already active and the editor needs to see if it should adjust.
421+
* The language service will unconditionally attempt to provide a result.
422+
* `triggerCharacter` can be `undefined` for a retrigger caused by a cursor move.
423+
*/
424+
export interface SignatureHelpRetriggeredReason {
425+
kind: "retrigger",
426+
/**
427+
* Character that was responsible for triggering signature help.
391428
*/
392-
triggerCharacter?: SignatureHelpTriggerCharacter;
429+
triggerCharacter?: SignatureHelpRetriggerCharacter,
393430
}
394431

395432
export interface ApplyCodeActionCommandResult {

‎tests/cases/fourslash/fourslash.ts

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ declare namespace FourSlashInterface {
139139
file(name: string, content?: string, scriptKindName?: string): any;
140140
select(startMarker: string, endMarker: string): void;
141141
selectRange(range: Range): void;
142-
selectAllInFile(fileName: string): void;
143142
}
144143
class verifyNegatable {
145144
private negative;
@@ -179,7 +178,7 @@ declare namespace FourSlashInterface {
179178
isInCommentAtPosition(onlyMultiLineDiverges?: boolean): void;
180179
codeFix(options: {
181180
description: string,
182-
newFileContent?: NewFileContent,
181+
newFileContent?: string | { readonly [fileName: string]: string },
183182
newRangeContent?: string,
184183
errorCode?: number,
185184
index?: number,
@@ -191,7 +190,6 @@ declare namespace FourSlashInterface {
191190
applicableRefactorAvailableForRange(): void;
192191

193192
refactorAvailable(name: string, actionName?: string): void;
194-
refactorsAvailable(names: ReadonlyArray<string>): void;
195193
refactor(options: {
196194
name: string;
197195
actionName: string;
@@ -257,14 +255,15 @@ declare namespace FourSlashInterface {
257255
* For each of starts, asserts the ranges that are referenced from there.
258256
* This uses the 'findReferences' command instead of 'getReferencesAtPosition', so references are grouped by their definition.
259257
*/
260-
referenceGroups(starts: ArrayOrSingle<string> | ArrayOrSingle<Range>, parts: Array<FourSlashInterface.ReferenceGroup>): void;
258+
referenceGroups(starts: ArrayOrSingle<string> | ArrayOrSingle<Range>, parts: Array<{ definition: ReferencesDefinition, ranges: Range[] }>): void;
261259
singleReferenceGroup(definition: ReferencesDefinition, ranges?: Range[]): void;
262260
rangesAreOccurrences(isWriteAccess?: boolean): void;
263261
rangesWithSameTextAreRenameLocations(): void;
264262
rangesAreRenameLocations(options?: Range[] | { findInStrings?: boolean, findInComments?: boolean, ranges?: Range[] });
265263
findReferencesDefinitionDisplayPartsAtCaretAre(expected: ts.SymbolDisplayPart[]): void;
266264
noSignatureHelp(...markers: string[]): void;
267-
noSignatureHelpForTriggerCharacter(triggerCharacter: string, ...markers: string[]): void
265+
noSignatureHelpForTriggerReason(triggerReason: SignatureHelpTriggerReason, ...markers: string[]): void
266+
signatureHelpPresentForTriggerReason(triggerReason: SignatureHelpTriggerReason, ...markers: string[]): void
268267
signatureHelp(...options: VerifySignatureHelpOptions[], ): void;
269268
// Checks that there are no compile errors.
270269
noErrors(): void;
@@ -338,7 +337,7 @@ declare namespace FourSlashInterface {
338337
getEditsForFileRename(options: {
339338
oldPath: string;
340339
newPath: string;
341-
newFileContents: { readonly [fileName: string]: string };
340+
newFileContents: { [fileName: string]: string };
342341
}): void;
343342
moveToNewFile(options: {
344343
readonly newFileContents: { readonly [fileName: string]: string };
@@ -359,7 +358,7 @@ declare namespace FourSlashInterface {
359358
enableFormatting(): void;
360359
disableFormatting(): void;
361360

362-
applyRefactor(options: { refactorName: string, actionName: string, actionDescription: string, newContent: NewFileContent }): void;
361+
applyRefactor(options: { refactorName: string, actionName: string, actionDescription: string, newContent: string }): void;
363362
}
364363
class debug {
365364
printCurrentParameterHelp(): void;
@@ -514,10 +513,6 @@ declare namespace FourSlashInterface {
514513
text: string;
515514
range: Range;
516515
}
517-
interface ReferenceGroup {
518-
readonly definition: ReferencesDefinition;
519-
readonly ranges: ReadonlyArray<Range>;
520-
}
521516
interface Diagnostic {
522517
message: string;
523518
/** @default `test.ranges()[0]` */
@@ -567,7 +562,47 @@ declare namespace FourSlashInterface {
567562
argumentCount?: number;
568563
isVariadic?: boolean;
569564
tags?: ReadonlyArray<JSDocTagInfo>;
570-
triggerCharacter?: string;
565+
triggerReason?: SignatureHelpTriggerReason;
566+
}
567+
568+
export type SignatureHelpTriggerReason =
569+
| SignatureHelpInvokedReason
570+
| SignatureHelpCharacterTypedReason
571+
| SignatureHelpRetriggeredReason;
572+
573+
/**
574+
* Signals that the user manually requested signature help.
575+
* The language service will unconditionally attempt to provide a result.
576+
*/
577+
export interface SignatureHelpInvokedReason {
578+
kind: "invoked",
579+
triggerCharacter?: undefined,
580+
}
581+
582+
/**
583+
* Signals that the signature help request came from a user typing a character.
584+
* Depending on the character and the syntactic context, the request may or may not be served a result.
585+
*/
586+
export interface SignatureHelpCharacterTypedReason {
587+
kind: "characterTyped",
588+
/**
589+
* Character that was responsible for triggering signature help.
590+
*/
591+
triggerCharacter: string,
592+
}
593+
594+
/**
595+
* Signals that this signature help request came from typing a character or moving the cursor.
596+
* This should only occur if a signature help session was already active and the editor needs to see if it should adjust.
597+
* The language service will unconditionally attempt to provide a result.
598+
* `triggerCharacter` can be `undefined` for a retrigger caused by a cursor move.
599+
*/
600+
export interface SignatureHelpRetriggeredReason {
601+
kind: "retrigger",
602+
/**
603+
* Character that was responsible for triggering signature help.
604+
*/
605+
triggerCharacter?: string,
571606
}
572607

573608
interface JSDocTagInfo {
@@ -576,7 +611,6 @@ declare namespace FourSlashInterface {
576611
}
577612

578613
type ArrayOrSingle<T> = T | ReadonlyArray<T>;
579-
type NewFileContent = string | { readonly [fileName: string]: string };
580614
}
581615
declare function verifyOperationIsCancelled(f: any): void;
582616
declare var test: FourSlashInterface.test_;

0 commit comments

Comments
 (0)