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 a "getCombinedCodeFix" service #20338

Merged
15 commits merged into from
Dec 7, 2017
17 changes: 15 additions & 2 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,27 @@ namespace ts {

export function zipWith<T, U, V>(arrayA: ReadonlyArray<T>, arrayB: ReadonlyArray<U>, callback: (a: T, b: U, index: number) => V): V[] {
const result: V[] = [];
Debug.assert(arrayA.length === arrayB.length);
Debug.assertEqual(arrayA.length, arrayB.length);
for (let i = 0; i < arrayA.length; i++) {
result.push(callback(arrayA[i], arrayB[i], i));
}
return result;
}

export function zipToIterator<T, U>(arrayA: ReadonlyArray<T>, arrayB: ReadonlyArray<U>): Iterator<[T, U]> {
Debug.assertEqual(arrayA.length, arrayB.length);
let i = 0;
return {
next() {
if (i === arrayA.length) {
return { value: undefined as never, done: true };
}
i++;
return { value: [arrayA[i - 1], arrayB[i - 1]], done: false };
}
};
}

export function zipToMap<T>(keys: ReadonlyArray<string>, values: ReadonlyArray<T>): Map<T> {
Debug.assert(keys.length === values.length);
const map = createMap<T>();
Expand Down Expand Up @@ -1385,7 +1399,6 @@ namespace ts {
this.set(key, values = [value]);
}
return values;

}
function multiMapRemove<T>(this: MultiMap<T>, key: string, value: T) {
const values = this.get(key);
Expand Down
12 changes: 6 additions & 6 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -949,22 +949,22 @@ namespace ts {

export interface ConstructorDeclaration extends FunctionLikeDeclarationBase, ClassElement, JSDocContainer {
kind: SyntaxKind.Constructor;
parent?: ClassDeclaration | ClassExpression;
parent?: ClassLikeDeclaration;
body?: FunctionBody;
/* @internal */ returnFlowNode?: FlowNode;
}

/** For when we encounter a semicolon in a class declaration. ES6 allows these as class elements. */
export interface SemicolonClassElement extends ClassElement {
kind: SyntaxKind.SemicolonClassElement;
parent?: ClassDeclaration | ClassExpression;
parent?: ClassLikeDeclaration;
}

// See the comment on MethodDeclaration for the intuition behind GetAccessorDeclaration being a
// ClassElement and an ObjectLiteralElement.
export interface GetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, ObjectLiteralElement, JSDocContainer {
kind: SyntaxKind.GetAccessor;
parent?: ClassDeclaration | ClassExpression | ObjectLiteralExpression;
parent?: ClassLikeDeclaration | ObjectLiteralExpression;
name: PropertyName;
body?: FunctionBody;
}
Expand All @@ -973,7 +973,7 @@ namespace ts {
// ClassElement and an ObjectLiteralElement.
export interface SetAccessorDeclaration extends FunctionLikeDeclarationBase, ClassElement, ObjectLiteralElement, JSDocContainer {
kind: SyntaxKind.SetAccessor;
parent?: ClassDeclaration | ClassExpression | ObjectLiteralExpression;
parent?: ClassLikeDeclaration | ObjectLiteralExpression;
name: PropertyName;
body?: FunctionBody;
}
Expand All @@ -982,7 +982,7 @@ namespace ts {

export interface IndexSignatureDeclaration extends SignatureDeclarationBase, ClassElement, TypeElement {
kind: SyntaxKind.IndexSignature;
parent?: ClassDeclaration | ClassExpression | InterfaceDeclaration | TypeLiteralNode;
parent?: ClassLikeDeclaration | InterfaceDeclaration | TypeLiteralNode;
}

export interface TypeNode extends Node {
Expand Down Expand Up @@ -1986,7 +1986,7 @@ namespace ts {

export interface HeritageClause extends Node {
kind: SyntaxKind.HeritageClause;
parent?: InterfaceDeclaration | ClassDeclaration | ClassExpression;
parent?: InterfaceDeclaration | ClassLikeDeclaration;
token: SyntaxKind.ExtendsKeyword | SyntaxKind.ImplementsKeyword;
types: NodeArray<ExpressionWithTypeArguments>;
}
Expand Down
41 changes: 32 additions & 9 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2376,7 +2376,7 @@ Actual: ${stringify(fullActual)}`);
*/
public getAndApplyCodeActions(errorCode?: number, index?: number) {
const fileName = this.activeFile.fileName;
this.applyCodeActions(this.getCodeFixActions(fileName, errorCode), index);
this.applyCodeActions(this.getCodeFixes(fileName, errorCode), index);
}

public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) {
Expand Down Expand Up @@ -2429,6 +2429,17 @@ Actual: ${stringify(fullActual)}`);
this.verifyRangeIs(expectedText, includeWhiteSpace);
}

public verifyCodeFixAll(options: FourSlashInterface.VerifyCodeFixAllOptions): void {
const { fixId, newFileContent } = options;
const fixIds = ts.mapDefined(this.getCodeFixes(this.activeFile.fileName), a => a.fixId);
ts.Debug.assert(ts.contains(fixIds, fixId), "No available code fix has that group id.", () => `Expected '${fixId}'. Available action ids: ${fixIds}`);
const { changes, commands } = this.languageService.getCombinedCodeFix({ type: "file", fileName: this.activeFile.fileName }, fixId, this.formatCodeSettings);
assert.deepEqual(commands, options.commands);
assert(changes.every(c => c.fileName === this.activeFile.fileName), "TODO: support testing codefixes that touch multiple files");
this.applyChanges(changes);
this.verifyCurrentFileContent(newFileContent);
}

/**
* Applies fixes for the errors in fileName and compares the results to
* expectedContents after all fixes have been applied.
Expand All @@ -2441,7 +2452,7 @@ Actual: ${stringify(fullActual)}`);
public verifyFileAfterCodeFix(expectedContents: string, fileName?: string) {
fileName = fileName ? fileName : this.activeFile.fileName;

this.applyCodeActions(this.getCodeFixActions(fileName));
this.applyCodeActions(this.getCodeFixes(fileName));

const actualContents: string = this.getFileContent(fileName);
if (this.removeWhitespace(actualContents) !== this.removeWhitespace(expectedContents)) {
Expand All @@ -2451,7 +2462,7 @@ Actual: ${stringify(fullActual)}`);

public verifyCodeFix(options: FourSlashInterface.VerifyCodeFixOptions) {
const fileName = this.activeFile.fileName;
const actions = this.getCodeFixActions(fileName, options.errorCode);
const actions = this.getCodeFixes(fileName, options.errorCode);
let index = options.index;
if (index === undefined) {
if (!(actions && actions.length === 1)) {
Expand Down Expand Up @@ -2490,7 +2501,7 @@ Actual: ${stringify(fullActual)}`);
* Rerieves a codefix satisfying the parameters, or undefined if no such codefix is found.
* @param fileName Path to file where error should be retrieved from.
*/
private getCodeFixActions(fileName: string, errorCode?: number): ts.CodeAction[] {
private getCodeFixes(fileName: string, errorCode?: number): ts.CodeFixAction[] {
const diagnosticsForCodeFix = this.getDiagnostics(fileName).map(diagnostic => ({
start: diagnostic.start,
length: diagnostic.length,
Expand All @@ -2506,7 +2517,7 @@ Actual: ${stringify(fullActual)}`);
});
}

private applyCodeActions(actions: ts.CodeAction[], index?: number): void {
private applyCodeActions(actions: ReadonlyArray<ts.CodeAction>, index?: number): void {
if (index === undefined) {
if (!(actions && actions.length === 1)) {
this.raiseError(`Should find exactly one codefix, but ${actions ? actions.length : "none"} found. ${actions ? actions.map(a => `${Harness.IO.newLine()} "${a.description}"`) : ""}`);
Expand All @@ -2519,8 +2530,10 @@ Actual: ${stringify(fullActual)}`);
}
}

const changes = actions[index].changes;
this.applyChanges(actions[index].changes);
}

private applyChanges(changes: ReadonlyArray<ts.FileTextChanges>): void {
for (const change of changes) {
this.applyEdits(change.fileName, change.textChanges, /*isFormattingEdit*/ false);
}
Expand All @@ -2532,7 +2545,7 @@ Actual: ${stringify(fullActual)}`);
this.raiseError("Exactly one range should be specified in the testfile.");
}

const codeFixes = this.getCodeFixActions(this.activeFile.fileName, errorCode);
const codeFixes = this.getCodeFixes(this.activeFile.fileName, errorCode);

if (codeFixes.length === 0) {
if (expectedTextArray.length !== 0) {
Expand Down Expand Up @@ -2871,7 +2884,7 @@ Actual: ${stringify(fullActual)}`);
}

public verifyCodeFixAvailable(negative: boolean, info: FourSlashInterface.VerifyCodeFixAvailableOptions[] | undefined) {
const codeFixes = this.getCodeFixActions(this.activeFile.fileName);
const codeFixes = this.getCodeFixes(this.activeFile.fileName);

if (negative) {
if (codeFixes.length) {
Expand Down Expand Up @@ -3038,7 +3051,7 @@ Actual: ${stringify(fullActual)}`);
}

public printAvailableCodeFixes() {
const codeFixes = this.getCodeFixActions(this.activeFile.fileName);
const codeFixes = this.getCodeFixes(this.activeFile.fileName);
Harness.IO.log(stringify(codeFixes));
}

Expand Down Expand Up @@ -4149,6 +4162,10 @@ namespace FourSlashInterface {
this.state.verifyRangeAfterCodeFix(expectedText, includeWhiteSpace, errorCode, index);
}

public codeFixAll(options: VerifyCodeFixAllOptions): void {
this.state.verifyCodeFixAll(options);
}

public fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, actionName: string, formattingOptions?: ts.FormatCodeSettings): void {
this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorNameToApply, actionName, formattingOptions);
}
Expand Down Expand Up @@ -4584,6 +4601,12 @@ namespace FourSlashInterface {
commands?: ts.CodeActionCommand[];
}

export interface VerifyCodeFixAllOptions {
fixId: string;
newFileContent: string;
commands: ReadonlyArray<{}>;
}

export interface VerifyRefactorOptions {
name: string;
actionName: string;
Expand Down
4 changes: 2 additions & 2 deletions src/harness/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ namespace assert {
export function isFalse(expr: boolean, msg = "Expected value to be false."): void {
assert(!expr, msg);
}
export function equal<T>(a: T, b: T, msg = "Expected values to be equal."): void {
assert(a === b, msg);
export function equal<T>(a: T, b: T, msg?: string): void {
assert(a === b, msg || (() => `Expected to be equal:\nExpected:\n${JSON.stringify(a)}\nActual:\n${JSON.stringify(b)}`));
}
export function notEqual<T>(a: T, b: T, msg = "Expected values to not be equal."): void {
assert(a !== b, msg);
Expand Down
3 changes: 2 additions & 1 deletion src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -521,9 +521,10 @@ namespace Harness.LanguageService {
getSpanOfEnclosingComment(fileName: string, position: number, onlyMultiLine: boolean): ts.TextSpan {
return unwrapJSONCallResult(this.shim.getSpanOfEnclosingComment(fileName, position, onlyMultiLine));
}
getCodeFixesAtPosition(): ts.CodeAction[] {
getCodeFixesAtPosition(): never {
throw new Error("Not supported on the shim.");
}
getCombinedCodeFix = ts.notImplemented;
applyCodeActionCommand = ts.notImplemented;
getCodeFixDiagnostics(): ts.Diagnostic[] {
throw new Error("Not supported on the shim.");
Expand Down
22 changes: 11 additions & 11 deletions src/server/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ namespace ts.server {
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, codeAction => this.convertCodeActions(codeAction, fileName));
const convertedCodeActions = map(response.body[0].codeActions, ({ description, changes }) => ({ description, changes: this.convertChanges(changes, fileName) }));
return { ...response.body[0], codeActions: convertedCodeActions };
}

Expand Down Expand Up @@ -553,15 +553,18 @@ namespace ts.server {
return notImplemented();
}

getCodeFixesAtPosition(file: string, start: number, end: number, errorCodes: number[]): CodeAction[] {
getCodeFixesAtPosition(file: string, start: number, end: number, errorCodes: ReadonlyArray<number>): ReadonlyArray<CodeFixAction> {
const args: protocol.CodeFixRequestArgs = { ...this.createFileRangeRequestArgs(file, start, end), errorCodes };

const request = this.processRequest<protocol.CodeFixRequest>(CommandNames.GetCodeFixes, args);
const response = this.processResponse<protocol.CodeFixResponse>(request);

return response.body.map(entry => this.convertCodeActions(entry, file));
// TODO: GH#20538 shouldn't need cast
return (response.body as ReadonlyArray<protocol.CodeFixAction>).map(({ description, changes, fixId }) => ({ description, changes: this.convertChanges(changes, file), fixId }));
}

getCombinedCodeFix = notImplemented;

applyCodeActionCommand = notImplemented;

private createFileLocationOrRangeRequestArgs(positionOrRange: number | TextRange, fileName: string): protocol.FileLocationOrRangeRequestArgs {
Expand Down Expand Up @@ -638,14 +641,11 @@ namespace ts.server {
});
}

convertCodeActions(entry: protocol.CodeAction, fileName: string): CodeAction {
return {
description: entry.description,
changes: entry.changes.map(change => ({
fileName: change.fileName,
textChanges: change.textChanges.map(textChange => this.convertTextChangeToCodeEdit(textChange, fileName))
}))
};
private convertChanges(changes: protocol.FileCodeEdits[], fileName: string): FileTextChanges[] {
return changes.map(change => ({
fileName: change.fileName,
textChanges: change.textChanges.map(textChange => this.convertTextChangeToCodeEdit(textChange, fileName))
}));
}

convertTextChangeToCodeEdit(change: protocol.CodeEdit, fileName: string): ts.TextChange {
Expand Down
55 changes: 52 additions & 3 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,14 @@ namespace ts.server.protocol {
BreakpointStatement = "breakpointStatement",
CompilerOptionsForInferredProjects = "compilerOptionsForInferredProjects",
GetCodeFixes = "getCodeFixes",
ApplyCodeActionCommand = "applyCodeActionCommand",
/* @internal */
GetCodeFixesFull = "getCodeFixes-full",
// TODO: GH#20538
/* @internal */
GetCombinedCodeFix = "getCombinedCodeFix",
/* @internal */
GetCombinedCodeFixFull = "getCombinedCodeFix-full",
ApplyCodeActionCommand = "applyCodeActionCommand",
GetSupportedCodeFixes = "getSupportedCodeFixes",

GetApplicableRefactors = "getApplicableRefactors",
Expand Down Expand Up @@ -552,6 +557,19 @@ namespace ts.server.protocol {
arguments: CodeFixRequestArgs;
}

// TODO: GH#20538
/* @internal */
export interface GetCombinedCodeFixRequest extends Request {
command: CommandTypes.GetCombinedCodeFix;
arguments: GetCombinedCodeFixRequestArgs;
}

// TODO: GH#20538
/* @internal */
export interface GetCombinedCodeFixResponse extends Response {
body: CombinedCodeActions;
}

export interface ApplyCodeActionCommandRequest extends Request {
command: CommandTypes.ApplyCodeActionCommand;
arguments: ApplyCodeActionCommandRequestArgs;
Expand Down Expand Up @@ -601,7 +619,21 @@ namespace ts.server.protocol {
/**
* Errorcodes we want to get the fixes for.
*/
errorCodes?: number[];
errorCodes?: ReadonlyArray<number>;
}

// TODO: GH#20538
/* @internal */
export interface GetCombinedCodeFixRequestArgs {
scope: GetCombinedCodeFixScope;
fixId: {};
}

// TODO: GH#20538
/* @internal */
export interface GetCombinedCodeFixScope {
type: "file";
args: FileRequestArgs;
}

export interface ApplyCodeActionCommandRequestArgs {
Expand Down Expand Up @@ -1587,7 +1619,7 @@ namespace ts.server.protocol {

export interface CodeFixResponse extends Response {
/** The code actions that are available */
body?: CodeAction[];
body?: CodeAction[]; // TODO: GH#20538 CodeFixAction[]
}

export interface CodeAction {
Expand All @@ -1599,6 +1631,23 @@ namespace ts.server.protocol {
commands?: {}[];
}

// TODO: GH#20538
/* @internal */
export interface CombinedCodeActions {
changes: ReadonlyArray<FileCodeEdits>;
commands?: ReadonlyArray<{}>;
}

// TODO: GH#20538
/* @internal */
export interface CodeFixAction extends CodeAction {
/**
* If present, one may call 'getCombinedCodeFix' with this fixId.
* This may be omitted to indicate that the code fix can't be applied in a group.
*/
fixId?: {};
}

/**
* Format and format on key response message.
*/
Expand Down
Loading