Skip to content

Commit

Permalink
More review
Browse files Browse the repository at this point in the history
  • Loading branch information
Andy Hanson committed Mar 6, 2018
1 parent 590faf0 commit 4d592ee
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 48 deletions.
16 changes: 8 additions & 8 deletions src/harness/fourslash.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2924,7 +2924,7 @@ Actual: ${stringify(fullActual)}`);

public verifyApplicableRefactorAvailableAtMarker(negative: boolean, markerName: string) {
const marker = this.getMarkerByName(markerName);
const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, marker.position);
const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, marker.position, ts.defaultOptions);
const isAvailable = applicableRefactors && applicableRefactors.length > 0;
if (negative && isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableAtMarker failed - expected no refactor at marker ${markerName} but found some.`);
Expand All @@ -2944,7 +2944,7 @@ Actual: ${stringify(fullActual)}`);
public verifyRefactorAvailable(negative: boolean, name: string, actionName?: string) {
const selection = this.getSelection();

let refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, selection) || [];
let refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, selection, ts.defaultOptions) || [];
refactors = refactors.filter(r => r.name === name && (actionName === undefined || r.actions.some(a => a.name === actionName)));
const isAvailable = refactors.length > 0;

Expand All @@ -2966,7 +2966,7 @@ Actual: ${stringify(fullActual)}`);
public verifyRefactor({ name, actionName, refactors }: FourSlashInterface.VerifyRefactorOptions) {
const selection = this.getSelection();

const actualRefactors = (this.languageService.getApplicableRefactors(this.activeFile.fileName, selection) || ts.emptyArray)
const actualRefactors = (this.languageService.getApplicableRefactors(this.activeFile.fileName, selection, ts.defaultOptions) || ts.emptyArray)
.filter(r => r.name === name && r.actions.some(a => a.name === actionName));
this.assertObjectsEqual(actualRefactors, refactors);
}
Expand All @@ -2977,7 +2977,7 @@ Actual: ${stringify(fullActual)}`);
throw new Error("Exactly one refactor range is allowed per test.");
}

const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, { pos: ranges[0].pos, end: ranges[0].end });
const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, ts.first(ranges), ts.defaultOptions);
const isAvailable = applicableRefactors && applicableRefactors.length > 0;
if (negative && isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some.`);
Expand All @@ -2989,7 +2989,7 @@ Actual: ${stringify(fullActual)}`);

public applyRefactor({ refactorName, actionName, actionDescription, newContent: newContentWithRenameMarker }: FourSlashInterface.ApplyRefactorOptions) {
const range = this.getSelection();
const refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, range);
const refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, range, ts.defaultOptions);
const refactorsWithName = refactors.filter(r => r.name === refactorName);
if (refactorsWithName.length === 0) {
this.raiseError(`The expected refactor: ${refactorName} is not available at the marker location.\nAvailable refactors: ${refactors.map(r => r.name)}`);
Expand All @@ -3003,7 +3003,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);
const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactorName, actionName, ts.defaultOptions);
for (const edit of editInfo.edits) {
this.applyEdits(edit.fileName, edit.textChanges, /*isFormattingEdit*/ false);
}
Expand Down Expand Up @@ -3048,14 +3048,14 @@ Actual: ${stringify(fullActual)}`);
formattingOptions = formattingOptions || this.formatCodeSettings;
const markerPos = this.getMarkerByName(markerName).position;

const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos);
const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos, ts.defaultOptions);
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);
const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply, actionName, ts.defaultOptions);

for (const edit of editInfo.edits) {
this.applyEdits(edit.fileName, edit.textChanges, /*isFormattingEdit*/ false);
Expand Down
2 changes: 2 additions & 0 deletions src/harness/unittests/extractTestHelpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ namespace ts {
endPosition: selectionRange.end,
host: notImplementedHost,
formatContext: formatting.getFormatContext(testFormatOptions),
options: defaultOptions,
};
const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromRange(selectionRange));
assert.equal(rangeToExtract.errors, undefined, rangeToExtract.errors && "Range error: " + rangeToExtract.errors[0].messageText);
Expand Down Expand Up @@ -190,6 +191,7 @@ namespace ts {
endPosition: selectionRange.end,
host: notImplementedHost,
formatContext: formatting.getFormatContext(testFormatOptions),
options: defaultOptions,
};
const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromRange(selectionRange));
assert.isUndefined(rangeToExtract.errors, rangeToExtract.errors && "Range error: " + rangeToExtract.errors[0].messageText);
Expand Down
4 changes: 2 additions & 2 deletions src/harness/unittests/organizeImports.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ export const Other = 1;
content: "function F() { }",
};
const languageService = makeLanguageService(testFile);
const changes = languageService.organizeImports({ type: "file", fileName: testFile.path }, testFormatOptions);
const changes = languageService.organizeImports({ type: "file", fileName: testFile.path }, testFormatOptions, defaultOptions);
assert.isEmpty(changes);
});

Expand Down Expand Up @@ -403,7 +403,7 @@ import { React, Other } from "react";
function runBaseline(baselinePath: string, testFile: TestFSWithWatch.FileOrFolder, ...otherFiles: TestFSWithWatch.FileOrFolder[]) {
const { path: testPath, content: testContent } = testFile;
const languageService = makeLanguageService(testFile, ...otherFiles);
const changes = languageService.organizeImports({ type: "file", fileName: testPath }, testFormatOptions);
const changes = languageService.organizeImports({ type: "file", fileName: testPath }, testFormatOptions, defaultOptions);
assert.equal(changes.length, 1);
assert.equal(changes[0].fileName, testPath);

Expand Down
6 changes: 3 additions & 3 deletions src/server/protocol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2589,17 +2589,17 @@ namespace ts.server.protocol {
}

export interface Options {
quote: "double" | "single";
quote?: "double" | "single";
/**
* If enabled, TypeScript will search through all external modules' exports and add them to the completions list.
* This affects lone identifier completions but not completions on the right hand side of `obj.`.
*/
includeExternalModuleExports: boolean;
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;
includeInsertTextCompletions?: boolean;
}

export interface CompilerOptions {
Expand Down
16 changes: 8 additions & 8 deletions src/server/session.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1239,7 +1239,7 @@ namespace ts.server {
const position = this.getPosition(args, scriptInfo);

const completions = project.getLanguageService().getCompletionsAtPosition(file, position, {
...this.getServicesOptions(file),
...this.getOptions(file),
includeExternalModuleExports: args.includeExternalModuleExports,
includeInsertTextCompletions: args.includeInsertTextCompletions
});
Expand Down Expand Up @@ -1564,7 +1564,7 @@ namespace ts.server {
const { file, project } = this.getFileAndProject(args);
const scriptInfo = project.getScriptInfoForNormalizedPath(file);
const { position, textRange } = this.extractPositionAndRange(args, scriptInfo);
return project.getLanguageService().getApplicableRefactors(file, position || textRange);
return project.getLanguageService().getApplicableRefactors(file, position || textRange, this.getOptions(file));
}

private getEditsForRefactor(args: protocol.GetEditsForRefactorRequestArgs, simplifiedResult: boolean): RefactorEditInfo | protocol.RefactorEditInfo {
Expand All @@ -1577,7 +1577,8 @@ namespace ts.server {
this.getFormatOptions(file),
position || textRange,
args.refactor,
args.action
args.action,
this.getOptions(file),
);

if (result === undefined) {
Expand All @@ -1603,8 +1604,7 @@ namespace ts.server {
private organizeImports({ scope }: protocol.OrganizeImportsRequestArgs, simplifiedResult: boolean): ReadonlyArray<protocol.FileCodeEdits> | ReadonlyArray<FileTextChanges> {
Debug.assert(scope.type === "file");
const { file, project } = this.getFileAndProject(scope.args);
const formatOptions = this.getFormatOptions(file);
const changes = project.getLanguageService().organizeImports({ type: "file", fileName: file }, formatOptions);
const changes = project.getLanguageService().organizeImports({ type: "file", fileName: file }, this.getFormatOptions(file), this.getOptions(file));
if (simplifiedResult) {
return this.mapTextChangesToCodeEdits(project, changes);
}
Expand All @@ -1622,7 +1622,7 @@ namespace ts.server {
const scriptInfo = project.getScriptInfoForNormalizedPath(file);
const { startPosition, endPosition } = this.getStartAndEndPosition(args, scriptInfo);

const codeActions = project.getLanguageService().getCodeFixesAtPosition(file, startPosition, endPosition, args.errorCodes, this.getFormatOptions(file), this.getServicesOptions(file));
const codeActions = project.getLanguageService().getCodeFixesAtPosition(file, startPosition, endPosition, args.errorCodes, this.getFormatOptions(file), this.getOptions(file));
if (!codeActions) {
return undefined;
}
Expand All @@ -1637,7 +1637,7 @@ namespace ts.server {
private getCombinedCodeFix({ scope, fixId }: protocol.GetCombinedCodeFixRequestArgs, simplifiedResult: boolean): protocol.CombinedCodeActions | CombinedCodeActions {
Debug.assert(scope.type === "file");
const { file, project } = this.getFileAndProject(scope.args);
const res = project.getLanguageService().getCombinedCodeFix({ type: "file", fileName: file }, fixId, this.getFormatOptions(file), this.getServicesOptions(file));
const res = project.getLanguageService().getCombinedCodeFix({ type: "file", fileName: file }, fixId, this.getFormatOptions(file), this.getOptions(file));
if (simplifiedResult) {
return { changes: this.mapTextChangesToCodeEdits(project, res.changes), commands: res.commands };
}
Expand Down Expand Up @@ -2157,7 +2157,7 @@ namespace ts.server {
return this.projectService.getFormatCodeOptions(file);
}

private getServicesOptions(file: NormalizedPath): Options {
private getOptions(file: NormalizedPath): Options {
return this.projectService.getOptions(file);
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/services/organizeImports.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ namespace ts.OrganizeImports {
sourceFile: SourceFile,
formatContext: formatting.FormatContext,
host: LanguageServiceHost,
program: Program) {
program: Program,
_options: Options,
) {

const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext });

Expand Down
1 change: 1 addition & 0 deletions src/services/refactorProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace ts {
endPosition?: number;
program: Program;
cancellationToken?: CancellationToken;
options: Options;
}

export namespace refactor {
Expand Down
17 changes: 10 additions & 7 deletions src/services/services.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1835,13 +1835,13 @@ namespace ts {
return codefix.getAllFixes({ fixId, sourceFile, program, host, cancellationToken, formatContext, options });
}

function organizeImports(scope: OrganizeImportsScope, formatOptions: FormatCodeSettings): ReadonlyArray<FileTextChanges> {
function organizeImports(scope: OrganizeImportsScope, formatOptions: FormatCodeSettings, options: Options): ReadonlyArray<FileTextChanges> {
synchronizeHostData();
Debug.assert(scope.type === "file");
const sourceFile = getValidSourceFile(scope.fileName);
const formatContext = formatting.getFormatContext(formatOptions);

return OrganizeImports.organizeImports(sourceFile, formatContext, host, program);
return OrganizeImports.organizeImports(sourceFile, formatContext, host, program, options);
}

function applyCodeActionCommand(action: CodeActionCommand): Promise<ApplyCodeActionCommandResult>;
Expand Down Expand Up @@ -2065,7 +2065,7 @@ namespace ts {
return Rename.getRenameInfo(program.getTypeChecker(), defaultLibFileName, getCanonicalFileName, getValidSourceFile(fileName), position);
}

function getRefactorContext(file: SourceFile, positionOrRange: number | TextRange, formatOptions?: FormatCodeSettings): RefactorContext {
function getRefactorContext(file: SourceFile, positionOrRange: number | TextRange, options: Options, formatOptions?: FormatCodeSettings): RefactorContext {
const [startPosition, endPosition] = typeof positionOrRange === "number" ? [positionOrRange, undefined] : [positionOrRange.pos, positionOrRange.end];
return {
file,
Expand All @@ -2075,25 +2075,28 @@ namespace ts {
host,
formatContext: formatting.getFormatContext(formatOptions),
cancellationToken,
options,
};
}

function getApplicableRefactors(fileName: string, positionOrRange: number | TextRange): ApplicableRefactorInfo[] {
function getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, options: Options): ApplicableRefactorInfo[] {
synchronizeHostData();
const file = getValidSourceFile(fileName);
return refactor.getApplicableRefactors(getRefactorContext(file, positionOrRange));
return refactor.getApplicableRefactors(getRefactorContext(file, positionOrRange, options));
}

function getEditsForRefactor(
fileName: string,
formatOptions: FormatCodeSettings,
positionOrRange: number | TextRange,
refactorName: string,
actionName: string): RefactorEditInfo {
actionName: string,
options: Options,
): RefactorEditInfo {

synchronizeHostData();
const file = getValidSourceFile(fileName);
return refactor.getEditsForRefactor(getRefactorContext(file, positionOrRange, formatOptions), refactorName, actionName);
return refactor.getEditsForRefactor(getRefactorContext(file, positionOrRange, options, formatOptions), refactorName, actionName);
}

return {
Expand Down
10 changes: 5 additions & 5 deletions src/services/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ namespace ts {

getSpanOfEnclosingComment(fileName: string, position: number, onlyMultiLine: boolean): TextSpan;

getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: ReadonlyArray<number>, formatOptions: FormatCodeSettings, servicesOptions: Options): ReadonlyArray<CodeFixAction>;
getCombinedCodeFix(scope: CombinedCodeFixScope, fixId: {}, formatOptions: FormatCodeSettings, servicesOptions: Options): CombinedCodeActions;
getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: ReadonlyArray<number>, formatOptions: FormatCodeSettings, options: Options): ReadonlyArray<CodeFixAction>;
getCombinedCodeFix(scope: CombinedCodeFixScope, fixId: {}, formatOptions: FormatCodeSettings, options: Options): CombinedCodeActions;
applyCodeActionCommand(action: CodeActionCommand): Promise<ApplyCodeActionCommandResult>;
applyCodeActionCommand(action: CodeActionCommand[]): Promise<ApplyCodeActionCommandResult[]>;
applyCodeActionCommand(action: CodeActionCommand | CodeActionCommand[]): Promise<ApplyCodeActionCommandResult | ApplyCodeActionCommandResult[]>;
Expand All @@ -314,9 +314,9 @@ namespace ts {
applyCodeActionCommand(fileName: string, action: CodeActionCommand[]): Promise<ApplyCodeActionCommandResult[]>;
/** @deprecated `fileName` will be ignored */
applyCodeActionCommand(fileName: string, action: CodeActionCommand | CodeActionCommand[]): Promise<ApplyCodeActionCommandResult | ApplyCodeActionCommandResult[]>;
getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[];
getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string): RefactorEditInfo | undefined;
organizeImports(scope: OrganizeImportsScope, formatOptions: FormatCodeSettings): ReadonlyArray<FileTextChanges>;
getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange, options: Options): ApplicableRefactorInfo[];
getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, options: Options): RefactorEditInfo | undefined;
organizeImports(scope: OrganizeImportsScope, formatOptions: FormatCodeSettings, options: Options): ReadonlyArray<FileTextChanges>;

getEmitOutput(fileName: string, emitOnlyDtsFiles?: boolean): EmitOutput;

Expand Down
Loading

0 comments on commit 4d592ee

Please sign in to comment.