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

Support completions that require changing from dot to bracket access #20547

Merged
10 commits merged into from
Jan 9, 2018
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3898,5 +3898,9 @@
"Install '{0}'": {
"category": "Message",
"code": 95014
},
"Use bracket notation instead of dot notation": {
"category": "Message",
"code": 95015
}
}
34 changes: 25 additions & 9 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ namespace FourSlash {
}
}

private raiseError(message: string) {
private raiseError(message: string): never {
throw new Error(this.messageAtLastKnownMarker(message));
}

Expand Down Expand Up @@ -847,10 +847,10 @@ namespace FourSlash {
}
}

public verifyCompletionsAt(markerName: string, expected: string[], options?: FourSlashInterface.CompletionsAtOptions) {
public verifyCompletionsAt(markerName: string, expected: ReadonlyArray<FourSlashInterface.ExpectedCompletionEntry>, options?: FourSlashInterface.CompletionsAtOptions) {
this.goToMarker(markerName);

const actualCompletions = this.getCompletionListAtCaret();
const actualCompletions = this.getCompletionListAtCaret(options);
if (!actualCompletions) {
this.raiseError(`No completions at position '${this.currentCaretPosition}'.`);
}
Expand All @@ -866,9 +866,20 @@ namespace FourSlash {
}

ts.zipWith(actual, expected, (completion, expectedCompletion, index) => {
if (completion.name !== expectedCompletion) {
const { name, insertText, replacementSpan } = typeof expectedCompletion === "string" ? { name: expectedCompletion, insertText: undefined, replacementSpan: undefined } : expectedCompletion;
if (completion.name !== name) {
this.raiseError(`Expected completion at index ${index} to be ${expectedCompletion}, got ${completion.name}`);
}
if (completion.insertText !== insertText) {
this.raiseError(`Expected completion insert text at index ${index} to be ${insertText}, got ${completion.insertText}`);
}
const convertedReplacementSpan = replacementSpan && textSpanFromRange(replacementSpan);
try {
assert.deepEqual(completion.replacementSpan, convertedReplacementSpan);
}
catch {
this.raiseError(`Expected completion replacementSpan at index ${index} to be ${stringify(convertedReplacementSpan)}, got ${stringify(completion.replacementSpan)}`);
}
});
}

Expand Down Expand Up @@ -1807,7 +1818,7 @@ Actual: ${stringify(fullActual)}`);
}
else if (prevChar === " " && /A-Za-z_/.test(ch)) {
/* Completions */
this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, { includeExternalModuleExports: false });
this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
}

if (i % checkCadence === 0) {
Expand Down Expand Up @@ -2382,7 +2393,8 @@ Actual: ${stringify(fullActual)}`);
public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) {
this.goToMarker(markerName);

const actualCompletion = this.getCompletionListAtCaret({ includeExternalModuleExports: true }).entries.find(e => e.name === options.name && e.source === options.source);
const actualCompletion = this.getCompletionListAtCaret({ includeExternalModuleExports: true, includeInsertTextCompletions: false }).entries.find(e =>
e.name === options.name && e.source === options.source);

if (!actualCompletion.hasAction) {
this.raiseError(`Completion for ${options.name} does not have an associated action.`);
Expand Down Expand Up @@ -3182,8 +3194,7 @@ Actual: ${stringify(fullActual)}`);
private getTextSpanForRangeAtIndex(index: number): ts.TextSpan {
const ranges = this.getRanges();
if (ranges && ranges.length > index) {
const range = ranges[index];
return { start: range.start, length: range.end - range.start };
return textSpanFromRange(ranges[index]);
}
else {
this.raiseError("Supplied span index: " + index + " does not exist in range list of size: " + (ranges ? 0 : ranges.length));
Expand Down Expand Up @@ -3213,6 +3224,10 @@ Actual: ${stringify(fullActual)}`);
}
}

function textSpanFromRange(range: FourSlash.Range): ts.TextSpan {
return ts.createTextSpanFromBounds(range.start, range.end);
}

export function runFourSlashTest(basePath: string, testType: FourSlashTestType, fileName: string) {
const content = Harness.IO.readFile(fileName);
runFourSlashTestContent(basePath, testType, content, fileName);
Expand Down Expand Up @@ -3954,7 +3969,7 @@ namespace FourSlashInterface {
super(state);
}

public completionsAt(markerName: string, completions: string[], options?: CompletionsAtOptions) {
public completionsAt(markerName: string, completions: ReadonlyArray<ExpectedCompletionEntry>, options?: CompletionsAtOptions) {
this.state.verifyCompletionsAt(markerName, completions, options);
}

Expand Down Expand Up @@ -4578,6 +4593,7 @@ namespace FourSlashInterface {
newContent: string;
}

export type ExpectedCompletionEntry = string | { name: string, insertText?: string, replacementSpan?: FourSlash.Range };
export interface CompletionsAtOptions extends ts.GetCompletionsAtPositionOptions {
isNewIdentifierLocation?: boolean;
}
Expand Down
12 changes: 6 additions & 6 deletions src/harness/unittests/tsserverProjectSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1302,13 +1302,13 @@ namespace ts.projectSystem {
service.checkNumberOfProjects({ externalProjects: 1 });
checkProjectActualFiles(service.externalProjects[0], [f1.path, f2.path, libFile.path]);

const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false });
const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
// should contain completions for string
assert.isTrue(completions1.entries.some(e => e.name === "charAt"), "should contain 'charAt'");
assert.isFalse(completions1.entries.some(e => e.name === "toExponential"), "should not contain 'toExponential'");

service.closeClientFile(f2.path);
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false });
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
// should contain completions for string
assert.isFalse(completions2.entries.some(e => e.name === "charAt"), "should not contain 'charAt'");
assert.isTrue(completions2.entries.some(e => e.name === "toExponential"), "should contain 'toExponential'");
Expand All @@ -1334,11 +1334,11 @@ namespace ts.projectSystem {
service.checkNumberOfProjects({ externalProjects: 1 });
checkProjectActualFiles(service.externalProjects[0], [f1.path, f2.path, libFile.path]);

const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false });
const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
assert.isTrue(completions1.entries.some(e => e.name === "somelongname"), "should contain 'somelongname'");

service.closeClientFile(f2.path);
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false });
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
assert.isFalse(completions2.entries.some(e => e.name === "somelongname"), "should not contain 'somelongname'");
const sf2 = service.externalProjects[0].getLanguageService().getProgram().getSourceFile(f2.path);
assert.equal(sf2.text, "");
Expand Down Expand Up @@ -1943,7 +1943,7 @@ namespace ts.projectSystem {

// Check identifiers defined in HTML content are available in .ts file
const project = configuredProjectAt(projectService, 0);
let completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 1, { includeExternalModuleExports: false });
let completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 1, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
assert(completions && completions.entries[0].name === "hello", `expected entry hello to be in completion list`);

// Close HTML file
Expand All @@ -1957,7 +1957,7 @@ namespace ts.projectSystem {
checkProjectActualFiles(configuredProjectAt(projectService, 0), [file1.path, file2.path, config.path]);

// Check identifiers defined in HTML content are not available in .ts file
completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 5, { includeExternalModuleExports: false });
completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 5, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
assert(completions && completions.entries[0].name !== "hello", `unexpected hello entry in completion list`);
});

Expand Down
11 changes: 11 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1693,6 +1693,11 @@ namespace ts.server.protocol {
* This affects lone identifier completions but not completions on the right hand side of `obj.`.
*/
includeExternalModuleExports: boolean;
/**
* If enabled, the completion list will include completions with invalid identifier names.
* For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`.
*/
includeInsertTextCompletions: boolean;
}

/**
Expand Down Expand Up @@ -1768,6 +1773,12 @@ namespace ts.server.protocol {
* is often the same as the name but may be different in certain circumstances.
*/
sortText: string;
/**
* Text to insert instead of `name`.
* This is used to support bracketed completions; If `name` might be "a-b" but `insertText` would be `["a-b"]`,
* coupled with `replacementSpan` to replace a dotted access with a bracket access.
*/
insertText?: string;
/**
* An optional span that indicates the text to be replaced by this completion item.
* If present, this span should be used instead of the default one.
Expand Down
4 changes: 2 additions & 2 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1207,10 +1207,10 @@ namespace ts.server {
if (simplifiedResult) {
return mapDefined<CompletionEntry, protocol.CompletionEntry>(completions && completions.entries, entry => {
if (completions.isMemberCompletion || startsWith(entry.name.toLowerCase(), prefix.toLowerCase())) {
const { name, kind, kindModifiers, sortText, replacementSpan, hasAction, source, isRecommended } = entry;
const { name, kind, kindModifiers, sortText, insertText, replacementSpan, hasAction, source, isRecommended } = entry;
const convertedSpan = replacementSpan ? this.toLocationTextSpan(replacementSpan, scriptInfo) : undefined;
// Use `hasAction || undefined` to avoid serializing `false`.
return { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan, hasAction: hasAction || undefined, source, isRecommended };
return { name, kind, kindModifiers, sortText, insertText, replacementSpan: convertedSpan, hasAction: hasAction || undefined, source, isRecommended };
}
}).sort((a, b) => compareStringsCaseSensitiveUI(a.name, b.name));
}
Expand Down
Loading