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

Add infrastructure for refactors #14624

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3349,6 +3349,7 @@ namespace ts {
Warning,
Error,
Message,
Refactor
}

export enum ModuleResolutionKind {
Expand Down
94 changes: 94 additions & 0 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,10 @@ namespace FourSlash {
return diagnostics;
}

private getCodeFixDiagnostics(fileName: string): ts.Diagnostic[] {
return this.languageService.getCodeFixDiagnostics(fileName);
}

private getAllDiagnostics(): ts.Diagnostic[] {
const diagnostics: ts.Diagnostic[] = [];

Expand Down Expand Up @@ -2609,6 +2613,80 @@ namespace FourSlash {
}
}

public verifyCodeFixDiagnosticsAvailableAtMarker(negative: boolean, markerName: string, diagnosticCode?: number) {
const marker = this.getMarkerByName(markerName);
const markerPos = marker.position;
let foundDiagnostic = false;

const refactorDiagnostics = this.getCodeFixDiagnostics(this.activeFile.fileName);
for (const diag of refactorDiagnostics) {
if (diag.start <= markerPos && diag.start + diag.length >= markerPos) {
foundDiagnostic = diagnosticCode === undefined || diagnosticCode === diag.code;
}
}

if (negative && foundDiagnostic) {
this.raiseError(`verifyRefactorDiagnosticsAvailableAtMarker failed - expected no refactor diagnostic at marker ${markerName} but found some.`);
}
if (!negative && !foundDiagnostic) {
this.raiseError(`verifyRefactorDiagnosticsAvailableAtMarker failed - expected a refactor diagnostic at marker ${markerName} but found none.`);
}
}

public verifyApplicableRefactorAvailableAtMarker(negative: boolean, markerName: string) {
const marker = this.getMarkerByName(markerName);
const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, marker.position);
const isAvailable = applicableRefactors && applicableRefactors.length > 0;
if (negative && isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableAtMarker failed - expected no refactor at marker ${markerName} but found some.`);
}
if (!negative && !isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableAtMarker failed - expected a refactor at marker ${markerName} but found none.`);
}
}

public verifyApplicableRefactorAvailableForRange(negative: boolean) {
const ranges = this.getRanges();
if (!(ranges && ranges.length === 1)) {
throw new Error("Exactly one refactor range is allowed per test.");
}

const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, { pos: ranges[0].start, end: ranges[0].end });
const isAvailable = applicableRefactors && applicableRefactors.length > 0;
if (negative && isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some.`);
}
if (!negative && !isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`);
}
}

public verifyFileAfterApplyingRefactorAtMarker(
markerName: string,
expectedContent: string,
refactorNameToApply: string,
formattingOptions?: ts.FormatCodeSettings) {

formattingOptions = formattingOptions || this.formatCodeSettings;
const markerPos = this.getMarkerByName(markerName).position;

const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos);
const applicableRefactorToApply = ts.find(applicableRefactors, refactor => refactor.refactorName === refactorNameToApply);

if (!applicableRefactorToApply) {
this.raiseError(`The expected refactor: ${refactorNameToApply} is not available at the marker location.`);
}

const codeActions = this.languageService.getRefactorCodeActions(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply);

this.applyCodeAction(this.activeFile.fileName, codeActions);
const actualContent = this.getFileContent(this.activeFile.fileName);

if (this.normalizeNewlines(actualContent) !== this.normalizeNewlines(expectedContent)) {
this.raiseError(`verifyFileAfterApplyingRefactors failed: expected:\n${expectedContent}\nactual:\n${actualContent}`);
}
}

public printAvailableCodeFixes() {
const codeFixes = this.getCodeFixActions(this.activeFile.fileName);
Harness.IO.log(stringify(codeFixes));
Expand Down Expand Up @@ -3405,6 +3483,18 @@ namespace FourSlashInterface {
public codeFixAvailable() {
this.state.verifyCodeFixAvailable(this.negative);
}

public refactorDiagnosticsAvailableAtMarker(markerName: string, refactorCode?: number) {
this.state.verifyCodeFixDiagnosticsAvailableAtMarker(this.negative, markerName, refactorCode);
}

public applicableRefactorAvailableAtMarker(markerName: string) {
this.state.verifyApplicableRefactorAvailableAtMarker(this.negative, markerName);
}

public applicableRefactorAvailableForRange() {
this.state.verifyApplicableRefactorAvailableForRange(this.negative);
}
}

export class Verify extends VerifyNegatable {
Expand Down Expand Up @@ -3615,6 +3705,10 @@ namespace FourSlashInterface {
this.state.verifyRangeAfterCodeFix(expectedText, includeWhiteSpace, errorCode, index);
}

public fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, formattingOptions?: ts.FormatCodeSettings): void {
this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorNameToApply, formattingOptions);
}

public importFixAtPosition(expectedTextArray: string[], errorCode?: number): void {
this.state.verifyImportFixAtPosition(expectedTextArray, errorCode);
}
Expand Down
9 changes: 9 additions & 0 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,15 @@ namespace Harness.LanguageService {
getCodeFixesAtPosition(): ts.CodeAction[] {
throw new Error("Not supported on the shim.");
}
getCodeFixDiagnostics(): ts.Diagnostic[] {
throw new Error("Not supported on the shim.");
}
getRefactorCodeActions(): ts.CodeAction[] {
throw new Error("Not supported on the shim.");
}
getApplicableRefactors(): ts.ApplicableRefactorInfo[] {
throw new Error("Not supported on the shim.");
}
getEmitOutput(fileName: string): ts.EmitOutput {
return unwrapJSONCallResult(this.shim.getEmitOutput(fileName));
}
Expand Down
22 changes: 17 additions & 5 deletions src/harness/unittests/tsserverProjectSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ namespace ts.projectSystem {
this.map[timeoutId] = cb.bind(/*this*/ undefined, ...args);
return timeoutId;
}

unregister(id: any) {
if (typeof id === "number") {
delete this.map[id];
Expand All @@ -338,10 +339,13 @@ namespace ts.projectSystem {
}

invoke() {
// Note: invoking a callback may result in new callbacks been queued,
// so do not clear the entire callback list regardless. Only remove the
// ones we have invoked.
for (const key in this.map) {
this.map[key]();
delete this.map[key];
}
this.map = [];
}
}

Expand Down Expand Up @@ -3589,7 +3593,7 @@ namespace ts.projectSystem {

// run first step
host.runQueuedTimeoutCallbacks();
assert.equal(host.getOutput().length, 1, "expect 1 messages");
assert.equal(host.getOutput().length, 1, "expect 1 message");
const e1 = <protocol.Event>getMessage(0);
assert.equal(e1.event, "syntaxDiag");
host.clearOutput();
Expand All @@ -3611,15 +3615,23 @@ namespace ts.projectSystem {

// run first step
host.runQueuedTimeoutCallbacks();
assert.equal(host.getOutput().length, 1, "expect 1 messages");
assert.equal(host.getOutput().length, 1, "expect 1 message");
const e1 = <protocol.Event>getMessage(0);
assert.equal(e1.event, "syntaxDiag");
host.clearOutput();

// the semanticDiag message
host.runQueuedImmediateCallbacks();
assert.equal(host.getOutput().length, 2, "expect 2 messages");
assert.equal(host.getOutput().length, 1, "expect 1 message");
const e2 = <protocol.Event>getMessage(0);
assert.equal(e2.event, "semanticDiag");
host.clearOutput();

// the refactor diagnostics check
host.runQueuedImmediateCallbacks();
assert.equal(host.getOutput().length, 2, "expect 2 messages");
const e3 = <protocol.Event>getMessage(0);
assert.equal(e3.event, "refactorDiag");
verifyRequestCompleted(getErrId, 1);

cancellationToken.resetToken();
Expand All @@ -3633,7 +3645,7 @@ namespace ts.projectSystem {
assert.equal(host.getOutput().length, 0, "expect 0 messages");
// run first step
host.runQueuedTimeoutCallbacks();
assert.equal(host.getOutput().length, 1, "expect 1 messages");
assert.equal(host.getOutput().length, 1, "expect 1 message");
const e1 = <protocol.Event>getMessage(0);
assert.equal(e1.event, "syntaxDiag");
host.clearOutput();
Expand Down
44 changes: 44 additions & 0 deletions src/server/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,50 @@ namespace ts.server {
return response.body.map(entry => this.convertCodeActions(entry, fileName));
}

getCodeFixDiagnostics(_fileName: string): Diagnostic[] {
return notImplemented();
}

private createFileLocationOrRangeRequestArgs(positionOrRange: number | TextRange, fileName: string): protocol.FileLocationOrRangeRequestArgs {
if (typeof positionOrRange === "number") {
const { line, offset } = this.positionToOneBasedLineOffset(fileName, positionOrRange);
return <protocol.FileLocationRequestArgs>{ file: fileName, line, offset };
}
const { line: startLine, offset: startOffset } = this.positionToOneBasedLineOffset(fileName, positionOrRange.pos);
const { line: endLine, offset: endOffset } = this.positionToOneBasedLineOffset(fileName, positionOrRange.end);
return <protocol.FileRangeRequestArgs>{
file: fileName,
startLine,
startOffset,
endLine,
endOffset
};
}

getApplicableRefactors(fileName: string, positionOrRange: number | TextRange): ApplicableRefactorInfo[] {
const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName);

const request = this.processRequest<protocol.GetApplicableRefactorsRequest>(CommandNames.GetApplicableRefactors, args);
const response = this.processResponse<protocol.GetApplicableRefactorsResponse>(request);

return response.body.refactors;
}

getRefactorCodeActions(
fileName: string,
_formatOptions: FormatCodeSettings,
positionOrRange: number | TextRange,
refactorName: string) {

const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetRefactorCodeActionsRequestArgs;
args.refactorName = refactorName;

const request = this.processRequest<protocol.GetRefactorCodeActionsRequest>(CommandNames.GetRefactorCodeActions, args);
const codeActions = this.processResponse<protocol.GetRefactorCodeActionsResponse>(request).body.actions;

return map(codeActions, codeAction => this.convertCodeActions(codeAction, fileName));
}

convertCodeActions(entry: protocol.CodeAction, fileName: string): CodeAction {
return {
description: entry.description,
Expand Down
50 changes: 46 additions & 4 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ namespace ts.server.protocol {
/* @internal */
export type GetCodeFixesFull = "getCodeFixes-full";
export type GetSupportedCodeFixes = "getSupportedCodeFixes";

export type GetApplicableRefactors = "getApplicableRefactors";
export type GetRefactorCodeActions = "getRefactorCodeActions";
}

/**
Expand Down Expand Up @@ -394,6 +397,43 @@ namespace ts.server.protocol {
position?: number;
}

export type FileLocationOrRangeRequestArgs = FileLocationRequestArgs | FileRangeRequestArgs;

export interface GetApplicableRefactorsRequest extends Request {
command: CommandTypes.GetApplicableRefactors;
arguments: GetApplicableRefactorsRequestArgs;
}

export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs;

export interface ApplicableRefactorInfo {
refactorName: string;
description: string;
}

export interface GetApplicableRefactorsResponse extends Response {
body?: { refactors: ApplicableRefactorInfo[] };
}

export interface GetRefactorCodeActionsRequest extends Request {
command: CommandTypes.GetRefactorCodeActions;
arguments: GetRefactorCodeActionsRequestArgs;
}

export type GetRefactorCodeActionsRequestArgs = FileLocationOrRangeRequestArgs & {
/* The kind of the applicable refactor */
refactorName: string;
};

export interface GetRefactorCodeActionsResponse extends Response {
body?: { actions: CodeAction[] };
}

export interface CodeFixDiagnosticEventBody {
file: string;
diagnostics: Diagnostic[];
}

/**
* Request for the available codefixes at a specific position.
*/
Expand All @@ -402,10 +442,7 @@ namespace ts.server.protocol {
arguments: CodeFixRequestArgs;
}

/**
* Instances of this interface specify errorcodes on a specific location in a sourcefile.
*/
export interface CodeFixRequestArgs extends FileRequestArgs {
export interface FileRangeRequestArgs extends FileRequestArgs {
/**
* The line number for the request (1-based).
*/
Expand Down Expand Up @@ -437,7 +474,12 @@ namespace ts.server.protocol {
*/
/* @internal */
endPosition?: number;
}

/**
* Instances of this interface specify errorcodes on a specific location in a sourcefile.
*/
export interface CodeFixRequestArgs extends FileRangeRequestArgs {
/**
* Errorcodes we want to get the fixes for.
*/
Expand Down
Loading