Skip to content

Commit

Permalink
Support completions that require changing from dot to bracket access (#…
Browse files Browse the repository at this point in the history 
…20547)

* Support completions that require changing from dot to bracket access

* Use insertText and replacementSpan

* Rename includeBracketCompletions to includeInsertTextCompletions

* Don't add completions that start with space
  • Loading branch information
Andy authored Jan 9, 2018
1 parent 73e3e8d commit 89ceb4b
Show file tree
Hide file tree
Showing 19 changed files with 343 additions and 290 deletions.
34 changes: 25 additions & 9 deletions src/harness/fourslash.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ namespace FourSlash {
}
}

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

Expand Down Expand Up @@ -848,10 +848,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 @@ -867,9 +867,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 @@ -1808,7 +1819,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 @@ -2383,7 +2394,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 @@ -3195,8 +3207,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 @@ -3226,6 +3237,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 @@ -3967,7 +3982,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 @@ -4591,6 +4606,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
View file
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
View file
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
View file
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

0 comments on commit 89ceb4b

Please sign in to comment.