Skip to content
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

Trigger characters in signature help #24915

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 44 additions & 23 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1426,18 +1426,22 @@ Actual: ${stringify(fullActual)}`);
}
}

public verifyNoSignatureHelp(markers: ReadonlyArray<string>) {
public verifySignatureHelpPresence(expectPresent: boolean, triggerReason: ts.SignatureHelpTriggerReason | undefined, markers: ReadonlyArray<string>) {
if (markers.length) {
for (const marker of markers) {
this.goToMarker(marker);
this.verifyNoSignatureHelp(ts.emptyArray);
this.verifySignatureHelpPresence(expectPresent, triggerReason, ts.emptyArray);
}
return;
}

const actual = this.getSignatureHelp();
if (actual) {
this.raiseError(`Expected no signature help, but got "${stringify(actual)}"`);
const actual = this.getSignatureHelp({ triggerReason });
if (expectPresent !== !!actual) {
if (actual) {
this.raiseError(`Expected no signature help, but got "${stringify(actual)}"`);
}
else {
this.raiseError("Expected signature help, but none was returned.")
}
}
}

Expand All @@ -1456,7 +1460,7 @@ Actual: ${stringify(fullActual)}`);
}

private verifySignatureHelpWorker(options: FourSlashInterface.VerifySignatureHelpOptions) {
const help = this.getSignatureHelp()!;
const help = this.getSignatureHelp({ triggerReason: options.triggerReason })!;
const selectedItem = help.items[help.selectedItemIndex];
// Argument index may exceed number of parameters
const currentParameter = selectedItem.parameters[help.argumentIndex] as ts.SignatureHelpParameter | undefined;
Expand Down Expand Up @@ -1498,6 +1502,7 @@ Actual: ${stringify(fullActual)}`);

const allKeys: ReadonlyArray<keyof FourSlashInterface.VerifySignatureHelpOptions> = [
"marker",
"triggerReason",
"overloadsCount",
"docComment",
"text",
Expand Down Expand Up @@ -1724,7 +1729,7 @@ Actual: ${stringify(fullActual)}`);
}

public printCurrentParameterHelp() {
const help = this.languageService.getSignatureHelpItems(this.activeFile.fileName, this.currentCaretPosition);
const help = this.languageService.getSignatureHelpItems(this.activeFile.fileName, this.currentCaretPosition, /*options*/ undefined);
Harness.IO.log(stringify(help));
}

Expand Down Expand Up @@ -1765,12 +1770,14 @@ Actual: ${stringify(fullActual)}`);
}

public printCurrentSignatureHelp() {
const help = this.getSignatureHelp()!;
const help = this.getSignatureHelp(ts.emptyOptions)!;
Harness.IO.log(stringify(help.items[help.selectedItemIndex]));
}

private getSignatureHelp(): ts.SignatureHelpItems | undefined {
return this.languageService.getSignatureHelpItems(this.activeFile.fileName, this.currentCaretPosition);
private getSignatureHelp({ triggerReason }: FourSlashInterface.VerifySignatureHelpOptions): ts.SignatureHelpItems | undefined {
return this.languageService.getSignatureHelpItems(this.activeFile.fileName, this.currentCaretPosition, {
triggerReason
});
}

public printCompletionListMembers(preferences: ts.UserPreferences | undefined) {
Expand Down Expand Up @@ -1866,13 +1873,18 @@ Actual: ${stringify(fullActual)}`);
offset++;

if (highFidelity) {
if (ch === "(" || ch === ",") {
if (ch === "(" || ch === "," || ch === "<") {
/* Signature help*/
this.languageService.getSignatureHelpItems(this.activeFile.fileName, offset);
this.languageService.getSignatureHelpItems(this.activeFile.fileName, offset, {
triggerReason: {
kind: "characterTyped",
triggerCharacter: ch
}
});
}
else if (prevChar === " " && /A-Za-z_/.test(ch)) {
/* Completions */
this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, ts.defaultPreferences);
this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, ts.emptyOptions);
}

if (i % checkCadence === 0) {
Expand Down Expand Up @@ -2505,7 +2517,7 @@ Actual: ${stringify(fullActual)}`);
`Expected '${fixId}'. Available action ids: ${ts.mapDefined(this.getCodeFixes(this.activeFile.fileName), a => a.fixId)}`);
ts.Debug.assertEqual(fixWithId!.fixAllDescription, fixAllDescription);

const { changes, commands } = this.languageService.getCombinedCodeFix({ type: "file", fileName: this.activeFile.fileName }, fixId, this.formatCodeSettings, ts.defaultPreferences);
const { changes, commands } = this.languageService.getCombinedCodeFix({ type: "file", fileName: this.activeFile.fileName }, fixId, this.formatCodeSettings, ts.emptyOptions);
assert.deepEqual<ReadonlyArray<{}> | undefined>(commands, expectedCommands);
assert(changes.every(c => c.fileName === this.activeFile.fileName), "TODO: support testing codefixes that touch multiple files");
this.applyChanges(changes);
Expand Down Expand Up @@ -2585,7 +2597,7 @@ Actual: ${stringify(fullActual)}`);
* Rerieves a codefix satisfying the parameters, or undefined if no such codefix is found.
* @param fileName Path to file where error should be retrieved from.
*/
private getCodeFixes(fileName: string, errorCode?: number, preferences: ts.UserPreferences = ts.defaultPreferences): ts.CodeFixAction[] {
private getCodeFixes(fileName: string, errorCode?: number, preferences: ts.UserPreferences = ts.emptyOptions): ts.CodeFixAction[] {
const diagnosticsForCodeFix = this.getDiagnostics(fileName, /*includeSuggestions*/ true).map(diagnostic => ({
start: diagnostic.start,
length: diagnostic.length,
Expand Down Expand Up @@ -3067,7 +3079,7 @@ Actual: ${stringify(fullActual)}`);
this.raiseError(`Expected action description to be ${JSON.stringify(actionDescription)}, got: ${JSON.stringify(action.description)}`);
}

const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactorName, actionName, ts.defaultPreferences)!;
const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactorName, actionName, ts.emptyOptions)!;
for (const edit of editInfo.edits) {
this.applyEdits(edit.fileName, edit.textChanges, /*isFormattingEdit*/ false);
}
Expand Down Expand Up @@ -3131,7 +3143,7 @@ Actual: ${stringify(fullActual)}`);
const action = ts.first(refactor.actions);
assert(action.name === "Move to a new file" && action.description === "Move to a new file");

const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactor.name, action.name, options.preferences || ts.defaultPreferences)!;
const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactor.name, action.name, options.preferences || ts.emptyOptions)!;
this.testNewFileContents(editInfo.edits, options.newFileContents, "move to new file");
}

Expand Down Expand Up @@ -3173,14 +3185,14 @@ Actual: ${stringify(fullActual)}`);
formattingOptions = formattingOptions || this.formatCodeSettings;
const markerPos = this.getMarkerByName(markerName).position;

const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos, ts.defaultPreferences);
const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos, ts.emptyOptions);
const applicableRefactorToApply = ts.find(applicableRefactors, refactor => refactor.name === refactorNameToApply);

if (!applicableRefactorToApply) {
this.raiseError(`The expected refactor: ${refactorNameToApply} is not available at the marker location.`);
}

const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply, actionName, ts.defaultPreferences)!;
const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply, actionName, ts.emptyOptions)!;

for (const edit of editInfo.edits) {
this.applyEdits(edit.fileName, edit.textChanges, /*isFormattingEdit*/ false);
Expand Down Expand Up @@ -3377,7 +3389,7 @@ Actual: ${stringify(fullActual)}`);

public getEditsForFileRename({ oldPath, newPath, newFileContents }: FourSlashInterface.GetEditsForFileRenameOptions): void {
const test = (fileContents: { readonly [fileName: string]: string }, description: string): void => {
const changes = this.languageService.getEditsForFileRename(oldPath, newPath, this.formatCodeSettings, ts.defaultPreferences);
const changes = this.languageService.getEditsForFileRename(oldPath, newPath, this.formatCodeSettings, ts.emptyOptions);
this.testNewFileContents(changes, fileContents, description);
};

Expand All @@ -3391,7 +3403,7 @@ Actual: ${stringify(fullActual)}`);
test(renameKeys(newFileContents, key => pathUpdater(key) || key), "with file moved");
}

private getApplicableRefactors(positionOrRange: number | ts.TextRange, preferences = ts.defaultPreferences): ReadonlyArray<ts.ApplicableRefactorInfo> {
private getApplicableRefactors(positionOrRange: number | ts.TextRange, preferences = ts.emptyOptions): ReadonlyArray<ts.ApplicableRefactorInfo> {
return this.languageService.getApplicableRefactors(this.activeFile.fileName, positionOrRange, preferences) || ts.emptyArray;
}
}
Expand Down Expand Up @@ -4079,7 +4091,15 @@ namespace FourSlashInterface {
}

public noSignatureHelp(...markers: string[]): void {
this.state.verifyNoSignatureHelp(markers);
this.state.verifySignatureHelpPresence(/*expectPresent*/ false, /*triggerReason*/ undefined, markers);
}

public noSignatureHelpForTriggerReason(reason: ts.SignatureHelpTriggerReason, ...markers: string[]): void {
this.state.verifySignatureHelpPresence(/*expectPresent*/ false, reason, markers);
}

public signatureHelpPresentForTriggerReason(reason: ts.SignatureHelpTriggerReason, ...markers: string[]): void {
this.state.verifySignatureHelpPresence(/*expectPresent*/ true, reason, markers);
}

public signatureHelp(...options: VerifySignatureHelpOptions[]): void {
Expand Down Expand Up @@ -4810,6 +4830,7 @@ namespace FourSlashInterface {
readonly isVariadic?: boolean;
/** @default ts.emptyArray */
readonly tags?: ReadonlyArray<ts.JSDocTagInfo>;
readonly triggerReason?: ts.SignatureHelpTriggerReason;
}

export type ArrayOrSingle<T> = T | ReadonlyArray<T>;
Expand Down
4 changes: 2 additions & 2 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,8 @@ namespace Harness.LanguageService {
getBreakpointStatementAtPosition(fileName: string, position: number): ts.TextSpan {
return unwrapJSONCallResult(this.shim.getBreakpointStatementAtPosition(fileName, position));
}
getSignatureHelpItems(fileName: string, position: number): ts.SignatureHelpItems {
return unwrapJSONCallResult(this.shim.getSignatureHelpItems(fileName, position));
getSignatureHelpItems(fileName: string, position: number, options: ts.SignatureHelpItemsOptions | undefined): ts.SignatureHelpItems {
return unwrapJSONCallResult(this.shim.getSignatureHelpItems(fileName, position, options));
}
getRenameInfo(fileName: string, position: number): ts.RenameInfo {
return unwrapJSONCallResult(this.shim.getRenameInfo(fileName, position));
Expand Down
2 changes: 1 addition & 1 deletion src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ namespace ts.server {

this.hostConfiguration = {
formatCodeOptions: getDefaultFormatCodeSettings(this.host),
preferences: defaultPreferences,
preferences: emptyOptions,
hostInfo: "Unknown host",
extraFileExtensions: []
};
Expand Down
56 changes: 54 additions & 2 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1789,6 +1789,10 @@ namespace ts.server.protocol {
* Optional prefix to apply to possible completions.
*/
prefix?: string;
/**
* Character that was responsible for triggering completion.
* Should be `undefined` if a user manually requested completion.
*/
triggerCharacter?: CompletionsTriggerCharacter;
/**
* @deprecated Use UserPreferences.includeCompletionsForModuleExports
Expand Down Expand Up @@ -2062,10 +2066,58 @@ namespace ts.server.protocol {
argumentCount: number;
}

export type SignatureHelpTriggerCharacter = "," | "(" | "<";
export type SignatureHelpRetriggerCharacter = SignatureHelpTriggerCharacter | ")";

/**
* Arguments of a signature help request.
*/
export interface SignatureHelpRequestArgs extends FileLocationRequestArgs {
/**
* Reason why signature help was invoked.
* See each individual possible
*/
triggerReason?: SignatureHelpTriggerReason;
}

export type SignatureHelpTriggerReason =
| SignatureHelpInvokedReason
| SignatureHelpCharacterTypedReason
| SignatureHelpRetriggeredReason;

/**
* Arguments of a signature help request.
* Signals that the user manually requested signature help.
* The language service will unconditionally attempt to provide a result.
*/
export interface SignatureHelpRequestArgs extends FileLocationRequestArgs {
export interface SignatureHelpInvokedReason {
kind: "invoked",
triggerCharacter?: undefined,
}

/**
* Signals that the signature help request came from a user typing a character.
* Depending on the character and the syntactic context, the request may or may not be served a result.
*/
export interface SignatureHelpCharacterTypedReason {
kind: "characterTyped",
/**
* Character that was responsible for triggering signature help.
*/
triggerCharacter: SignatureHelpTriggerCharacter,
}

/**
* Signals that this signature help request came from typing a character or moving the cursor.
* This should only occur if a signature help session was already active and the editor needs to see if it should adjust.
* The language service will unconditionally attempt to provide a result.
* `triggerCharacter` can be `undefined` for a retrigger caused by a cursor move.
*/
export interface SignatureHelpRetriggeredReason {
kind: "retrigger",
/**
* Character that was responsible for triggering signature help.
*/
triggerCharacter?: SignatureHelpRetriggerCharacter,
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/server/scriptInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ namespace ts.server {

if (preferences) {
if (!this.preferences) {
this.preferences = defaultPreferences;
this.preferences = emptyOptions;
}
this.preferences = { ...this.preferences, ...preferences };
}
Expand Down
2 changes: 1 addition & 1 deletion src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1434,7 +1434,7 @@ namespace ts.server {
const { file, project } = this.getFileAndProject(args);
const scriptInfo = this.projectService.getScriptInfoForNormalizedPath(file)!;
const position = this.getPosition(args, scriptInfo);
const helpItems = project.getLanguageService().getSignatureHelpItems(file, position);
const helpItems = project.getLanguageService().getSignatureHelpItems(file, position, args);
if (!helpItems) {
return undefined;
}
Expand Down
Loading