Skip to content

Commit f36bcd3

Browse files
author
Andy Hanson
committed
Code review
1 parent a40fab8 commit f36bcd3

17 files changed

+166
-154
lines changed

Diff for: ‎src/harness/fourslash.ts

+9-11
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,6 @@ namespace FourSlash {
365365

366366
function memoWrap(ls: ts.LanguageService, target: TestState): ts.LanguageService {
367367
const cacheableMembers: (keyof typeof ls)[] = [
368-
"getCompletionsAtPosition",
369368
"getCompletionEntryDetails",
370369
"getCompletionEntrySymbol",
371370
"getQuickInfoAtPosition",
@@ -1221,7 +1220,7 @@ Actual: ${stringify(fullActual)}`);
12211220
}
12221221

12231222
private getCompletionListAtCaret(options?: FourSlashInterface.CompletionsAtOptions): ts.CompletionInfo {
1224-
return this.languageService.getCompletionsAtPosition(this.activeFile.fileName, this.currentCaretPosition, options, options && options.settings);
1223+
return this.languageService.getCompletionsAtPosition(this.activeFile.fileName, this.currentCaretPosition, options);
12251224
}
12261225

12271226
private getCompletionEntryDetails(entryName: string, source?: string): ts.CompletionEntryDetails {
@@ -1719,7 +1718,7 @@ Actual: ${stringify(fullActual)}`);
17191718
Harness.IO.log(stringify(sigHelp));
17201719
}
17211720

1722-
public printCompletionListMembers(options: ts.GetCompletionsAtPositionOptions | undefined) {
1721+
public printCompletionListMembers(options: ts.Options | undefined) {
17231722
const completions = this.getCompletionListAtCaret(options);
17241723
this.printMembersOrCompletions(completions);
17251724
}
@@ -1818,7 +1817,7 @@ Actual: ${stringify(fullActual)}`);
18181817
}
18191818
else if (prevChar === " " && /A-Za-z_/.test(ch)) {
18201819
/* Completions */
1821-
this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, ts.defaultCompletionOptions, ts.defaultServicesSettings);
1820+
this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, ts.defaultOptions);
18221821
}
18231822

18241823
if (i % checkCadence === 0) {
@@ -2393,7 +2392,7 @@ Actual: ${stringify(fullActual)}`);
23932392
public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) {
23942393
this.goToMarker(markerName);
23952394

2396-
const actualCompletion = this.getCompletionListAtCaret({ ...ts.defaultCompletionOptions, includeExternalModuleExports: true }).entries.find(e =>
2395+
const actualCompletion = this.getCompletionListAtCaret({ ...ts.defaultOptions, includeExternalModuleExports: true }).entries.find(e =>
23972396
e.name === options.name && e.source === options.source);
23982397

23992398
if (!actualCompletion.hasAction) {
@@ -2445,7 +2444,7 @@ Actual: ${stringify(fullActual)}`);
24452444
const { fixId, newFileContent } = options;
24462445
const fixIds = ts.mapDefined(this.getCodeFixes(this.activeFile.fileName), a => a.fixId);
24472446
ts.Debug.assert(ts.contains(fixIds, fixId), "No available code fix has that group id.", () => `Expected '${fixId}'. Available action ids: ${fixIds}`);
2448-
const { changes, commands } = this.languageService.getCombinedCodeFix({ type: "file", fileName: this.activeFile.fileName }, fixId, this.formatCodeSettings);
2447+
const { changes, commands } = this.languageService.getCombinedCodeFix({ type: "file", fileName: this.activeFile.fileName }, fixId, this.formatCodeSettings, ts.defaultOptions);
24492448
assert.deepEqual(commands, options.commands);
24502449
assert(changes.every(c => c.fileName === this.activeFile.fileName), "TODO: support testing codefixes that touch multiple files");
24512450
this.applyChanges(changes);
@@ -2525,7 +2524,7 @@ Actual: ${stringify(fullActual)}`);
25252524
return;
25262525
}
25272526

2528-
return this.languageService.getCodeFixesAtPosition(fileName, diagnostic.start, diagnostic.start + diagnostic.length, [diagnostic.code], this.formatCodeSettings);
2527+
return this.languageService.getCodeFixesAtPosition(fileName, diagnostic.start, diagnostic.start + diagnostic.length, [diagnostic.code], this.formatCodeSettings, ts.defaultOptions);
25292528
});
25302529
}
25312530

@@ -4419,7 +4418,7 @@ namespace FourSlashInterface {
44194418
this.state.printCurrentSignatureHelp();
44204419
}
44214420

4422-
public printCompletionListMembers(options: ts.GetCompletionsAtPositionOptions | undefined) {
4421+
public printCompletionListMembers(options: ts.Options | undefined) {
44234422
this.state.printCompletionListMembers(options);
44244423
}
44254424

@@ -4616,12 +4615,11 @@ namespace FourSlashInterface {
46164615
}
46174616

46184617
export type ExpectedCompletionEntry = string | { name: string, insertText?: string, replacementSpan?: FourSlash.Range };
4619-
export interface CompletionsAtOptions extends ts.GetCompletionsAtPositionOptions {
4618+
export interface CompletionsAtOptions extends Partial<ts.Options> {
46204619
isNewIdentifierLocation?: boolean;
4621-
settings?: ts.ServicesSettings;
46224620
}
46234621

4624-
export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions {
4622+
export interface VerifyCompletionListContainsOptions extends ts.Options {
46254623
sourceDisplay: string;
46264624
isRecommended?: true;
46274625
insertText?: string;

Diff for: ‎src/harness/harnessLanguageService.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -417,8 +417,8 @@ namespace Harness.LanguageService {
417417
getEncodedSemanticClassifications(fileName: string, span: ts.TextSpan): ts.Classifications {
418418
return unwrapJSONCallResult(this.shim.getEncodedSemanticClassifications(fileName, span.start, span.length));
419419
}
420-
getCompletionsAtPosition(fileName: string, position: number, options: ts.GetCompletionsAtPositionOptions | undefined, settings: ts.ServicesSettings): ts.CompletionInfo {
421-
return unwrapJSONCallResult(this.shim.getCompletionsAtPosition(fileName, position, options, settings));
420+
getCompletionsAtPosition(fileName: string, position: number, options: ts.Options | undefined): ts.CompletionInfo {
421+
return unwrapJSONCallResult(this.shim.getCompletionsAtPosition(fileName, position, options));
422422
}
423423
getCompletionEntryDetails(fileName: string, position: number, entryName: string, options: ts.FormatCodeOptions | undefined, source: string | undefined): ts.CompletionEntryDetails {
424424
return unwrapJSONCallResult(this.shim.getCompletionEntryDetails(fileName, position, entryName, JSON.stringify(options), source));

Diff for: ‎src/harness/unittests/tsserverProjectSystem.ts

+6-6
Original file line numberDiff line numberDiff line change
@@ -1321,13 +1321,13 @@ namespace ts.projectSystem {
13211321
service.checkNumberOfProjects({ externalProjects: 1 });
13221322
checkProjectActualFiles(service.externalProjects[0], [f1.path, f2.path, libFile.path]);
13231323

1324-
const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, defaultCompletionOptions, defaultServicesSettings);
1324+
const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, defaultOptions);
13251325
// should contain completions for string
13261326
assert.isTrue(completions1.entries.some(e => e.name === "charAt"), "should contain 'charAt'");
13271327
assert.isFalse(completions1.entries.some(e => e.name === "toExponential"), "should not contain 'toExponential'");
13281328

13291329
service.closeClientFile(f2.path);
1330-
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, defaultCompletionOptions, defaultServicesSettings);
1330+
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, defaultOptions);
13311331
// should contain completions for string
13321332
assert.isFalse(completions2.entries.some(e => e.name === "charAt"), "should not contain 'charAt'");
13331333
assert.isTrue(completions2.entries.some(e => e.name === "toExponential"), "should contain 'toExponential'");
@@ -1353,11 +1353,11 @@ namespace ts.projectSystem {
13531353
service.checkNumberOfProjects({ externalProjects: 1 });
13541354
checkProjectActualFiles(service.externalProjects[0], [f1.path, f2.path, libFile.path]);
13551355

1356-
const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, defaultCompletionOptions, defaultServicesSettings);
1356+
const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, defaultOptions);
13571357
assert.isTrue(completions1.entries.some(e => e.name === "somelongname"), "should contain 'somelongname'");
13581358

13591359
service.closeClientFile(f2.path);
1360-
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, defaultCompletionOptions, defaultServicesSettings);
1360+
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, defaultOptions);
13611361
assert.isFalse(completions2.entries.some(e => e.name === "somelongname"), "should not contain 'somelongname'");
13621362
const sf2 = service.externalProjects[0].getLanguageService().getProgram().getSourceFile(f2.path);
13631363
assert.equal(sf2.text, "");
@@ -1962,7 +1962,7 @@ namespace ts.projectSystem {
19621962

19631963
// Check identifiers defined in HTML content are available in .ts file
19641964
const project = configuredProjectAt(projectService, 0);
1965-
let completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 1, defaultCompletionOptions, defaultServicesSettings);
1965+
let completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 1, defaultOptions);
19661966
assert(completions && completions.entries[0].name === "hello", `expected entry hello to be in completion list`);
19671967

19681968
// Close HTML file
@@ -1976,7 +1976,7 @@ namespace ts.projectSystem {
19761976
checkProjectActualFiles(configuredProjectAt(projectService, 0), [file1.path, file2.path, config.path]);
19771977

19781978
// Check identifiers defined in HTML content are not available in .ts file
1979-
completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 5, defaultCompletionOptions, defaultServicesSettings);
1979+
completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 5, defaultOptions);
19801980
assert(completions && completions.entries[0].name !== "hello", `unexpected hello entry in completion list`);
19811981
});
19821982

Diff for: ‎src/server/client.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,9 @@ namespace ts.server {
169169
};
170170
}
171171

172-
getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions | undefined): CompletionInfo {
173-
const args: protocol.CompletionsRequestArgs = { ...this.createFileLocationRequestArgs(fileName, position), ...options };
172+
getCompletionsAtPosition(fileName: string, position: number, _settings: Options | undefined): CompletionInfo {
173+
// Not passing along 'settings' because server should already have those from the 'configure' command
174+
const args: protocol.CompletionsRequestArgs = this.createFileLocationRequestArgs(fileName, position);
174175

175176
const request = this.processRequest<protocol.CompletionsRequest>(CommandNames.Completions, args);
176177
const response = this.processResponse<protocol.CompletionsResponse>(request);

Diff for: ‎src/server/editorServices.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ namespace ts.server {
200200

201201
export interface HostConfiguration {
202202
formatCodeOptions: FormatCodeSettings;
203-
servicesOptions: ServicesSettings;
203+
options: Options;
204204
hostInfo: string;
205205
extraFileExtensions?: JsFileExtensionInfo[];
206206
}
@@ -443,7 +443,7 @@ namespace ts.server {
443443

444444
this.hostConfiguration = {
445445
formatCodeOptions: getDefaultFormatCodeSettings(this.host),
446-
servicesOptions: defaultServicesSettings,
446+
options: defaultOptions,
447447
hostInfo: "Unknown host",
448448
extraFileExtensions: []
449449
};
@@ -710,9 +710,9 @@ namespace ts.server {
710710
return info && info.getFormatCodeSettings() || this.hostConfiguration.formatCodeOptions;
711711
}
712712

713-
getServicesSettings(file: NormalizedPath) {
713+
getOptions(file: NormalizedPath): Options {
714714
const info = this.getScriptInfoForNormalizedPath(file);
715-
return info && info.getServicesSettings() || this.hostConfiguration.servicesOptions;
715+
return info && info.getOptions() || this.hostConfiguration.options;
716716
}
717717

718718
private onSourceFileChanged(fileName: NormalizedPath, eventKind: FileWatcherEventKind) {
@@ -1830,7 +1830,7 @@ namespace ts.server {
18301830
if (args.file) {
18311831
const info = this.getScriptInfoForNormalizedPath(toNormalizedPath(args.file));
18321832
if (info) {
1833-
info.setSettings(convertFormatOptions(args.formatOptions), args.servicesOptions);
1833+
info.setOptions(convertFormatOptions(args.formatOptions), args.options);
18341834
this.logger.info(`Host configuration update for file ${args.file}`);
18351835
}
18361836
}
@@ -1843,8 +1843,8 @@ namespace ts.server {
18431843
mergeMapLikes(this.hostConfiguration.formatCodeOptions, convertFormatOptions(args.formatOptions));
18441844
this.logger.info("Format host information updated");
18451845
}
1846-
if (args.servicesOptions) {
1847-
mergeMapLikes(this.hostConfiguration.servicesOptions, args.servicesOptions);
1846+
if (args.options) {
1847+
mergeMapLikes(this.hostConfiguration.options, args.options);
18481848
}
18491849
if (args.extraFileExtensions) {
18501850
this.hostConfiguration.extraFileExtensions = args.extraFileExtensions;

Diff for: ‎src/server/protocol.ts

+17-9
Original file line numberDiff line numberDiff line change
@@ -1232,7 +1232,7 @@ namespace ts.server.protocol {
12321232
*/
12331233
formatOptions?: FormatCodeSettings;
12341234

1235-
servicesOptions?: ServicesSettings;
1235+
options?: Options;
12361236

12371237
/**
12381238
* The host's additional supported .js file extensions
@@ -1709,15 +1709,13 @@ namespace ts.server.protocol {
17091709
*/
17101710
prefix?: string;
17111711
/**
1712-
* If enabled, TypeScript will search through all external modules' exports and add them to the completions list.
1713-
* This affects lone identifier completions but not completions on the right hand side of `obj.`.
1712+
* @deprecated Use Options
17141713
*/
1715-
includeExternalModuleExports: boolean;
1714+
includeExternalModuleExports?: boolean;
17161715
/**
1717-
* If enabled, the completion list will include completions with invalid identifier names.
1718-
* For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`.
1716+
* @deprecated Use Options
17191717
*/
1720-
includeInsertTextCompletions: boolean;
1718+
includeInsertTextCompletions?: boolean;
17211719
}
17221720

17231721
/**
@@ -2590,8 +2588,18 @@ namespace ts.server.protocol {
25902588
insertSpaceBeforeTypeAnnotation?: boolean;
25912589
}
25922590

2593-
export interface ServicesSettings {
2594-
quote: '"' | "'";
2591+
export interface Options {
2592+
quote: "double" | "single";
2593+
/**
2594+
* If enabled, TypeScript will search through all external modules' exports and add them to the completions list.
2595+
* This affects lone identifier completions but not completions on the right hand side of `obj.`.
2596+
*/
2597+
includeExternalModuleExports: boolean;
2598+
/**
2599+
* If enabled, the completion list will include completions with invalid identifier names.
2600+
* For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`.
2601+
*/
2602+
includeInsertTextCompletions: boolean;
25952603
}
25962604

25972605
export interface CompilerOptions {

Diff for: ‎src/server/scriptInfo.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ namespace ts.server {
205205
*/
206206
readonly containingProjects: Project[] = [];
207207
private formatSettings: FormatCodeSettings | undefined;
208-
private servicesSettings: ServicesSettings | undefined;
208+
private options: Options | undefined;
209209

210210
/* @internal */
211211
fileWatcher: FileWatcher;
@@ -295,7 +295,7 @@ namespace ts.server {
295295
}
296296

297297
getFormatCodeSettings(): FormatCodeSettings { return this.formatSettings; }
298-
getServicesSettings(): ServicesSettings { return this.servicesSettings; }
298+
getOptions(): Options { return this.options; }
299299

300300
attachToProject(project: Project): boolean {
301301
const isNew = !this.isAttached(project);
@@ -388,19 +388,19 @@ namespace ts.server {
388388
}
389389
}
390390

391-
setSettings(formatSettings: FormatCodeSettings, servicesSettings: ServicesSettings): void {
391+
setOptions(formatSettings: FormatCodeSettings, options: Options): void {
392392
if (formatSettings) {
393393
if (!this.formatSettings) {
394394
this.formatSettings = getDefaultFormatCodeSettings(this.host);
395395
}
396396
mergeMapLikes(this.formatSettings, formatSettings);
397397
}
398398

399-
if (servicesSettings) {
400-
if (!this.servicesSettings) {
401-
this.servicesSettings = clone(defaultServicesSettings);
399+
if (options) {
400+
if (!this.options) {
401+
this.options = clone(defaultOptions);
402402
}
403-
mergeMapLikes(this.servicesSettings, servicesSettings);
403+
mergeMapLikes(this.options, options);
404404
}
405405
}
406406

0 commit comments

Comments
 (0)