Skip to content

Add 'data' property to completion entry for better coordination between completions and completion details #42890

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

Merged
merged 9 commits into from
Mar 1, 2021
1 change: 1 addition & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,7 @@ namespace ts {
},
tryGetMemberInModuleExports: (name, symbol) => tryGetMemberInModuleExports(escapeLeadingUnderscores(name), symbol),
tryGetMemberInModuleExportsAndProperties: (name, symbol) => tryGetMemberInModuleExportsAndProperties(escapeLeadingUnderscores(name), symbol),
tryFindAmbientModule: moduleName => tryFindAmbientModule(moduleName, /*withAugmentations*/ true),
tryFindAmbientModuleWithoutAugmentations: moduleName => {
// we deliberately exclude augmentations
// since we are only interested in declarations of the module itself
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4166,6 +4166,7 @@ namespace ts {
/* @internal */ createSymbol(flags: SymbolFlags, name: __String): TransientSymbol;
/* @internal */ createIndexInfo(type: Type, isReadonly: boolean, declaration?: SignatureDeclaration): IndexInfo;
/* @internal */ isSymbolAccessible(symbol: Symbol, enclosingDeclaration: Node | undefined, meaning: SymbolFlags, shouldComputeAliasToMarkVisible: boolean): SymbolAccessibilityResult;
/* @internal */ tryFindAmbientModule(moduleName: string): Symbol | undefined;
/* @internal */ tryFindAmbientModuleWithoutAugmentations(moduleName: string): Symbol | undefined;

/* @internal */ getSymbolWalker(accept?: (symbol: Symbol) => boolean): SymbolWalker;
Expand Down
17 changes: 8 additions & 9 deletions src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ namespace ts.server {
isNewIdentifierLocation: false,
entries: response.body!.map<CompletionEntry>(entry => { // TODO: GH#18217
if (entry.replacementSpan !== undefined) {
const { name, kind, kindModifiers, sortText, replacementSpan, hasAction, source, isRecommended } = entry;
const { name, kind, kindModifiers, sortText, replacementSpan, hasAction, source, data, isRecommended } = entry;
// TODO: GH#241
const res: CompletionEntry = { name, kind, kindModifiers, sortText, replacementSpan: this.decodeSpan(replacementSpan, fileName), hasAction, source, isRecommended };
const res: CompletionEntry = { name, kind, kindModifiers, sortText, replacementSpan: this.decodeSpan(replacementSpan, fileName), hasAction, source, data: data as any, isRecommended };
return res;
}

Expand All @@ -216,14 +216,13 @@ namespace ts.server {
};
}

getCompletionEntryDetails(fileName: string, position: number, entryName: string, _options: FormatCodeOptions | FormatCodeSettings | undefined, source: string | undefined): CompletionEntryDetails {
const args: protocol.CompletionDetailsRequestArgs = { ...this.createFileLocationRequestArgs(fileName, position), entryNames: [{ name: entryName, source }] };
getCompletionEntryDetails(fileName: string, position: number, entryName: string, _options: FormatCodeOptions | FormatCodeSettings | undefined, source: string | undefined, _preferences: UserPreferences | undefined, data: unknown): CompletionEntryDetails {
const args: protocol.CompletionDetailsRequestArgs = { ...this.createFileLocationRequestArgs(fileName, position), entryNames: [{ name: entryName, source, data }] };

const request = this.processRequest<protocol.CompletionDetailsRequest>(CommandNames.CompletionDetails, args);
const response = this.processResponse<protocol.CompletionDetailsResponse>(request);
Debug.assert(response.body!.length === 1, "Unexpected length of completion details response body.");
const convertedCodeActions = map(response.body![0].codeActions, ({ description, changes }) => ({ description, changes: this.convertChanges(changes, fileName) }));
return { ...response.body![0], codeActions: convertedCodeActions };
const request = this.processRequest<protocol.CompletionDetailsRequest>(CommandNames.CompletionDetailsFull, args);
const response = this.processResponse<protocol.Response>(request);
Debug.assert(response.body.length === 1, "Unexpected length of completion details response body.");
return response.body[0];
}

getCompletionEntrySymbol(_fileName: string, _position: number, _entryName: string): Symbol {
Expand Down
28 changes: 19 additions & 9 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ namespace FourSlash {
}
const memo = Utils.memoize(
(_version: number, _active: string, _caret: number, _selectEnd: number, _marker: string, ...args: any[]) => (ls[key] as Function)(...args),
(...args) => args.join("|,|")
(...args) => args.map(a => a && typeof a === "object" ? JSON.stringify(a) : a).join("|,|")
);
proxy[key] = (...args: any[]) => memo(
target.languageServiceAdapterHost.getScriptInfo(target.activeFile.fileName)!.version,
Expand Down Expand Up @@ -867,7 +867,7 @@ namespace FourSlash {
nameToEntries.set(entry.name, [entry]);
}
else {
if (entries.some(e => e.source === entry.source)) {
if (entries.some(e => e.source === entry.source && this.deepEqual(e.data, entry.data))) {
this.raiseError(`Duplicate completions for ${entry.name}`);
}
entries.push(entry);
Expand All @@ -885,8 +885,8 @@ namespace FourSlash {
const name = typeof include === "string" ? include : include.name;
const found = nameToEntries.get(name);
if (!found) throw this.raiseError(`Includes: completion '${name}' not found.`);
assert(found.length === 1, `Must use 'exact' for multiple completions with same name: '${name}'`);
this.verifyCompletionEntry(ts.first(found), include);
if (!found.length) throw this.raiseError(`Includes: no completions with name '${name}' remain unmatched.`);
this.verifyCompletionEntry(found.shift()!, include);
}
}
if (options.excludes) {
Expand Down Expand Up @@ -933,7 +933,7 @@ namespace FourSlash {
assert.equal(actual.sortText, expected.sortText || ts.Completions.SortText.LocationPriority, this.messageAtLastKnownMarker(`Actual entry: ${JSON.stringify(actual)}`));

if (expected.text !== undefined) {
const actualDetails = ts.Debug.checkDefined(this.getCompletionEntryDetails(actual.name, actual.source), `No completion details available for name '${actual.name}' and source '${actual.source}'`);
const actualDetails = ts.Debug.checkDefined(this.getCompletionEntryDetails(actual.name, actual.source, actual.data), `No completion details available for name '${actual.name}' and source '${actual.source}'`);
assert.equal(ts.displayPartsToString(actualDetails.displayParts), expected.text, "Expected 'text' property to match 'displayParts' string");
assert.equal(ts.displayPartsToString(actualDetails.documentation), expected.documentation || "", "Expected 'documentation' property to match 'documentation' display parts string");
// TODO: GH#23587
Expand Down Expand Up @@ -1254,6 +1254,16 @@ namespace FourSlash {

}

private deepEqual(a: unknown, b: unknown) {
try {
this.assertObjectsEqual(a, b);
return true;
}
catch {
return false;
}
}

public verifyDisplayPartsOfReferencedSymbol(expected: ts.SymbolDisplayPart[]) {
const referencedSymbols = this.findReferencesAtCaret()!;

Expand Down Expand Up @@ -1281,11 +1291,11 @@ namespace FourSlash {
return this.languageService.getCompletionsAtPosition(this.activeFile.fileName, this.currentCaretPosition, options);
}

private getCompletionEntryDetails(entryName: string, source?: string, preferences?: ts.UserPreferences): ts.CompletionEntryDetails | undefined {
private getCompletionEntryDetails(entryName: string, source: string | undefined, data: ts.CompletionEntryData | undefined, preferences?: ts.UserPreferences): ts.CompletionEntryDetails | undefined {
if (preferences) {
this.configure(preferences);
}
return this.languageService.getCompletionEntryDetails(this.activeFile.fileName, this.currentCaretPosition, entryName, this.formatCodeSettings, source, preferences);
return this.languageService.getCompletionEntryDetails(this.activeFile.fileName, this.currentCaretPosition, entryName, this.formatCodeSettings, source, preferences, data);
}

private getReferencesAtCaret() {
Expand Down Expand Up @@ -2796,14 +2806,14 @@ namespace FourSlash {
public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) {
this.goToMarker(markerName);

const details = this.getCompletionEntryDetails(options.name, options.source, options.preferences);
const details = this.getCompletionEntryDetails(options.name, options.source, options.data, options.preferences);
if (!details) {
const completions = this.getCompletionListAtCaret(options.preferences)?.entries;
const matchingName = completions?.filter(e => e.name === options.name);
const detailMessage = matchingName?.length
? `\n Found ${matchingName.length} with name '${options.name}' from source(s) ${matchingName.map(e => `'${e.source}'`).join(", ")}.`
: ` (In fact, there were no completions with name '${options.name}' at all.)`;
return this.raiseError(`No completions were found for the given name, source, and preferences.` + detailMessage);
return this.raiseError(`No completions were found for the given name, source/data, and preferences.` + detailMessage);
}
const codeActions = details.codeActions;
if (codeActions?.length !== 1) {
Expand Down
1 change: 1 addition & 0 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1701,6 +1701,7 @@ namespace FourSlashInterface {
export interface VerifyCompletionActionOptions extends NewContentOptions {
name: string;
source?: string;
data?: ts.CompletionEntryData;
description: string;
preferences?: ts.UserPreferences;
}
Expand Down
4 changes: 2 additions & 2 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,8 @@ namespace Harness.LanguageService {
getCompletionsAtPosition(fileName: string, position: number, preferences: ts.UserPreferences | undefined): ts.CompletionInfo {
return unwrapJSONCallResult(this.shim.getCompletionsAtPosition(fileName, position, preferences));
}
getCompletionEntryDetails(fileName: string, position: number, entryName: string, formatOptions: ts.FormatCodeOptions | undefined, source: string | undefined, preferences: ts.UserPreferences | undefined): ts.CompletionEntryDetails {
return unwrapJSONCallResult(this.shim.getCompletionEntryDetails(fileName, position, entryName, JSON.stringify(formatOptions), source, preferences));
getCompletionEntryDetails(fileName: string, position: number, entryName: string, formatOptions: ts.FormatCodeOptions | undefined, source: string | undefined, preferences: ts.UserPreferences | undefined, data: ts.CompletionEntryData | undefined): ts.CompletionEntryDetails {
return unwrapJSONCallResult(this.shim.getCompletionEntryDetails(fileName, position, entryName, JSON.stringify(formatOptions), source, preferences, data));
}
getCompletionEntrySymbol(): ts.Symbol {
throw new Error("getCompletionEntrySymbol not implemented across the shim layer.");
Expand Down
7 changes: 7 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2159,6 +2159,7 @@ namespace ts.server.protocol {
export interface CompletionEntryIdentifier {
name: string;
source?: string;
data?: unknown;
}

/**
Expand Down Expand Up @@ -2245,6 +2246,12 @@ namespace ts.server.protocol {
* in the project package.json.
*/
isPackageJsonImport?: true;
/**
* A property to be sent back to TS Server in the CompletionDetailsRequest, along with `name`,
* that allows TS Server to look up the symbol represented by the completion item, disambiguating
* items with the same name.
*/
data?: unknown;
}

/**
Expand Down
20 changes: 14 additions & 6 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1808,14 +1808,14 @@ namespace ts.server {
if (kind === protocol.CommandTypes.CompletionsFull) return completions;

const prefix = args.prefix || "";
const entries = mapDefined<CompletionEntry, protocol.CompletionEntry>(completions.entries, entry => {
const entries = stableSort(mapDefined<CompletionEntry, protocol.CompletionEntry>(completions.entries, entry => {
if (completions.isMemberCompletion || startsWith(entry.name.toLowerCase(), prefix.toLowerCase())) {
const { name, kind, kindModifiers, sortText, insertText, replacementSpan, hasAction, source, isRecommended, isPackageJsonImport } = entry;
const { name, kind, kindModifiers, sortText, insertText, replacementSpan, hasAction, source, isRecommended, isPackageJsonImport, data } = entry;
const convertedSpan = replacementSpan ? toProtocolTextSpan(replacementSpan, scriptInfo) : undefined;
// Use `hasAction || undefined` to avoid serializing `false`.
return { name, kind, kindModifiers, sortText, insertText, replacementSpan: convertedSpan, hasAction: hasAction || undefined, source, isRecommended, isPackageJsonImport };
return { name, kind, kindModifiers, sortText, insertText, replacementSpan: convertedSpan, hasAction: hasAction || undefined, source, isRecommended, isPackageJsonImport, data };
}
}).sort((a, b) => compareStringsCaseSensitiveUI(a.name, b.name));
}), (a, b) => compareStringsCaseSensitiveUI(a.name, b.name));

if (kind === protocol.CommandTypes.Completions) {
if (completions.metadata) (entries as WithMetadata<readonly protocol.CompletionEntry[]>).metadata = completions.metadata;
Expand All @@ -1837,8 +1837,8 @@ namespace ts.server {
const formattingOptions = project.projectService.getFormatCodeOptions(file);

const result = mapDefined(args.entryNames, entryName => {
const { name, source } = typeof entryName === "string" ? { name: entryName, source: undefined } : entryName;
return project.getLanguageService().getCompletionEntryDetails(file, position, name, formattingOptions, source, this.getPreferences(file));
const { name, source, data } = typeof entryName === "string" ? { name: entryName, source: undefined, data: undefined } : entryName;
return project.getLanguageService().getCompletionEntryDetails(file, position, name, formattingOptions, source, this.getPreferences(file), data ? cast(data, isCompletionEntryData) : undefined);
});
return simplifiedResult
? result.map(details => ({ ...details, codeActions: map(details.codeActions, action => this.mapCodeAction(action)) }))
Expand Down Expand Up @@ -3118,4 +3118,12 @@ namespace ts.server {
isDefinition
};
}

function isCompletionEntryData(data: any): data is CompletionEntryData {
return data === undefined || data && typeof data === "object"
&& typeof data.exportName === "string"
&& (data.fileName === undefined || typeof data.fileName === "string")
&& (data.ambientModuleName === undefined || typeof data.ambientModuleName === "string"
&& (data.isPackageJsonImport === undefined || typeof data.isPackageJsonImport === "boolean"));
}
}
Loading