Skip to content

Pvb/refactorapi #11913

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

Closed
wants to merge 9 commits into from
Prev Previous commit
Next Next commit
Refactoring to a simpler API
  • Loading branch information
Paul van Brenk committed Nov 9, 2016
commit 0a5c23d53ae2ec5d510794b157e79d2788eb62fe
11 changes: 3 additions & 8 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2076,22 +2076,17 @@ namespace FourSlash {
const start = markers[0].position;
const end = markers[1] ? markers[1].position : markers[0].position;

const actualRefactorings = this.languageService.getChangesForCodeRefactoringAtPosition(sourceFileName, start, end, refactoringId, /*options*/ undefined, this.languageService);
if (!actualRefactorings || actualRefactorings.length == 0) {
const textChanges = this.languageService.getChangesForCodeRefactoringAtPosition(sourceFileName, start, end, refactoringId, /*options*/ undefined, this.languageService);
if (!textChanges || textChanges.length == 0) {
this.raiseError("No code refactorings found.");
}

// for now only assume a single result for each refactoring
if (actualRefactorings.length > 1) {
this.raiseError("More than 1 refactoring returned.");
}

// for each file:
// * optionally apply the changes
// * check if the new contents match the expected contents
// * if we applied changes, but don't check the content raise an error
ts.forEach(this.testData.files, file => {
const refactorForFile = ts.find(actualRefactorings[0].changes, change => {
const refactorForFile = ts.find(textChanges, change => {
return change.fileName == file.fileName;
});

Expand Down
2 changes: 1 addition & 1 deletion src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ namespace Harness.LanguageService {
getAvailableCodeRefactoringsAtPosition(): ts.CodeRefactoring[] {
throw new Error("getAvailableCodeRefactoringsAtPosition not supported on the shim.");
}
getChangesForCodeRefactoringAtPosition(): ts.CodeAction[] {
getChangesForCodeRefactoringAtPosition(): ts.FileTextChanges[] {
throw new Error("getChangesForCodeRefactoringAtPosition not supported on the shim.");
}
getEmitOutput(fileName: string): ts.EmitOutput {
Expand Down
13 changes: 8 additions & 5 deletions src/server/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ namespace ts.server {
}

getSyntacticDiagnostics(fileName: string): Diagnostic[] {
const args: protocol.SyntacticDiagnosticsSyncRequestArgs = { file: fileName, includeLinePosition: true };
const args: protocol.SyntacticDiagnosticsSyncRequestArgs = { file: fileName, includeLinePosition: true };

const request = this.processRequest<protocol.SyntacticDiagnosticsSyncRequest>(CommandNames.SyntacticDiagnosticsSync, args);
const response = this.processResponse<protocol.SyntacticDiagnosticsSyncResponse>(request);
Expand Down Expand Up @@ -709,7 +709,7 @@ namespace ts.server {
return response.body;
}

getChangesForCodeRefactoringAtPosition(fileName: string, start: number, end: number, refactoringId: string, options: any, _serviceInstance: LanguageService): CodeAction[] {
getChangesForCodeRefactoringAtPosition(fileName: string, start: number, end: number, refactoringId: string, options: any, _serviceInstance: LanguageService): FileTextChanges[] {
const startLineOffset = this.positionToOneBasedLineOffset(fileName, start);
const endLineOffset = this.positionToOneBasedLineOffset(fileName, end);

Expand All @@ -726,20 +726,23 @@ namespace ts.server {
const request = this.processRequest<protocol.ApplyCodeRefactoringRequest>(CommandNames.ApplyCodeRefactoring, args);
const response = this.processResponse<protocol.ApplyCodeRefactoringResponse>(request);

return response.body.map(entry => this.convertCodeActions(entry, fileName));
return response.body.map(entry => ({
fileName: entry.fileName,
textChanges: entry.textChanges.map(codeEdit => this.convertCodeEditToTextChange(codeEdit, fileName))
}));
}

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))
textChanges: change.textChanges.map(textChange => this.convertCodeEditToTextChange(textChange, fileName))
}))
};
}

convertTextChangeToCodeEdit(change: protocol.CodeEdit, fileName: string): ts.TextChange {
convertCodeEditToTextChange(change: protocol.CodeEdit, fileName: string): ts.TextChange {
const start = this.lineOffsetToPosition(fileName, change.start);
const end = this.lineOffsetToPosition(fileName, change.end);

Expand Down
72 changes: 36 additions & 36 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,45 +401,45 @@ namespace ts.server.protocol {
}

export interface CodeChangeRequestArgs extends FileRequestArgs {
/**
* The line number for the request (1-based).
*/
startLine: number;
/**
* The line number for the request (1-based).
*/
startLine: number;

/**
* The character offset (on the line) for the request (1-based).
*/
startOffset: number;
/**
* The character offset (on the line) for the request (1-based).
*/
startOffset: number;

/**
* Position (can be specified instead of line/offset pair)
*/
/* @internal */
startPosition?: number;
/**
* Position (can be specified instead of line/offset pair)
*/
/* @internal */
startPosition?: number;

/**
* The line number for the request (1-based).
*/
endLine: number;
/**
* The line number for the request (1-based).
*/
endLine: number;

/**
* The character offset (on the line) for the request (1-based).
*/
endOffset: number;
/**
* The character offset (on the line) for the request (1-based).
*/
endOffset: number;

/**
* Position (can be specified instead of line/offset pair)
*/
/* @internal */
endPosition?: number;
/**
* Position (can be specified instead of line/offset pair)
*/
/* @internal */
endPosition?: number;
}

/**
* Request for the available code refactorings at a specific position.
*/
export interface AvailableCodeRefactoringsRequest extends Request {
command: CommandTypes.GetCodeRefactorings;
arguments: AvailableCodeRefactoringsRequestArgs;
command: CommandTypes.GetCodeRefactorings;
arguments: AvailableCodeRefactoringsRequestArgs;
}

/**
Expand Down Expand Up @@ -469,16 +469,16 @@ namespace ts.server.protocol {
input?: any;
}

export interface ApplyCodeRefactoringResponse extends CodeFixResponse {

export interface ApplyCodeRefactoringResponse extends Response {
body?: FileCodeEdits[];
}

/**
* Request for the available codefixes at a specific position.
*/
export interface CodeFixRequest extends Request {
command: CommandTypes.GetCodeFixes;
arguments: CodeFixRequestArgs;
command: CommandTypes.GetCodeFixes;
arguments: CodeFixRequestArgs;
}

export interface CodeFixResponse extends Response {
Expand All @@ -490,10 +490,10 @@ namespace ts.server.protocol {
* Instances of this interface specify errorcodes for a specific location in a sourcefile.
*/
export interface CodeFixRequestArgs extends CodeChangeRequestArgs {
/**
* Errorcodes we want to get the fixes for.
*/
errorCodes?: number[];
/**
* Errorcodes we want to get the fixes for.
*/
errorCodes?: number[];
}

/**
Expand Down
13 changes: 8 additions & 5 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1289,24 +1289,27 @@ namespace ts.server {
}
}

private getChangesForRefactoring(args: protocol.ApplyCodeRefactoringRequestArgs, simplifiedResult: boolean): protocol.CodeAction[] | CodeAction[] {
private getChangesForRefactoring(args: protocol.ApplyCodeRefactoringRequestArgs, simplifiedResult: boolean): protocol.FileCodeEdits[] | FileTextChanges[] {
const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args);

const scriptInfo = project.getScriptInfoForNormalizedPath(file);
const startPosition = getStartPosition();
const endPosition = getEndPosition();
const languageService = project.getLanguageService();

const codeActions = languageService.getChangesForCodeRefactoringAtPosition(file, startPosition, endPosition, args.refactoringId, args.input, languageService);
if (!codeActions) {
const fileTextChanges = languageService.getChangesForCodeRefactoringAtPosition(file, startPosition, endPosition, args.refactoringId, args.input, languageService);
if (!fileTextChanges) {
return undefined;
}

if (simplifiedResult) {
return codeActions.map(codeAction => this.mapCodeAction(codeAction, scriptInfo));
return fileTextChanges.map(fileTextChange => ({
fileName: fileTextChange.fileName,
textChanges: fileTextChange.textChanges.map(textChange => this.convertTextChangeToCodeEdit(textChange, scriptInfo))
}));
}
else {
return codeActions;
return fileTextChanges;
}

function getStartPosition() {
Expand Down
4 changes: 2 additions & 2 deletions src/services/coderefactorings/codeRefactoringProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ namespace ts {
export interface CodeRefactoringFactory {
refactoringId: string;
getAvailableRefactorings(context: CodeRefactoringContext): CodeRefactoring[];
getChangesForRefactoring(context: CodeRefactoringContext): CodeAction[];
getChangesForRefactoring(context: CodeRefactoringContext): FileTextChanges[];
}

export interface CodeRefactoringContext extends CodeChangeContext {
Expand Down Expand Up @@ -38,7 +38,7 @@ namespace ts {
return allRefactorings;
}

export function getTextChangesForRefactoring(context: CodeRefactoringContext): CodeAction[] {
export function getTextChangesForRefactoring(context: CodeRefactoringContext): FileTextChanges[] {
const factory = refactorings[context.refactoringId];
return factory.getChangesForRefactoring(context);
}
Expand Down
7 changes: 2 additions & 5 deletions src/services/coderefactorings/inlineTempRefactor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace ts.coderefactoring {
refactoringId: Diagnostics.Inline_temporary_variable.code.toString()
}];
},
getChangesForRefactoring: (context: CodeRefactoringContext): CodeAction[] => {
getChangesForRefactoring: (context: CodeRefactoringContext): FileTextChanges[] => {
const sourceFile = context.sourceFile;
const identifier = getTokenAtPosition(sourceFile, context.span.start);
let variableDeclaration: VariableDeclaration;
Expand Down Expand Up @@ -119,10 +119,7 @@ namespace ts.coderefactoring {
});
}

return [{
description: getLocaleSpecificMessage(Diagnostics.Inline_temporary_variable),
changes: fileTextChanges
}];
return fileTextChanges;
}
}
});
Expand Down
2 changes: 1 addition & 1 deletion src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1710,7 +1710,7 @@ namespace ts {
return coderefactoring.getAvailableCodeRefactorings(context);
}

function getChangesForCodeRefactoringAtPosition(fileName: string, start: number, end: number, refactoringId: string, options: any, serviceInstance: LanguageService): CodeAction[] {
function getChangesForCodeRefactoringAtPosition(fileName: string, start: number, end: number, refactoringId: string, options: any, serviceInstance: LanguageService): FileTextChanges[] {
synchronizeHostData();
const sourceFile = getValidSourceFile(fileName);
const span = { start, length: end - start };
Expand Down
2 changes: 1 addition & 1 deletion src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@

getAvailableCodeRefactoringsAtPosition(fileName: string, start: number, end: number, serviceInstance: LanguageService): CodeRefactoring[];

getChangesForCodeRefactoringAtPosition(fileName: string, start: number, end: number, refactoringId: string, options: any, serviceInstance: LanguageService): CodeAction[];
getChangesForCodeRefactoringAtPosition(fileName: string, start: number, end: number, refactoringId: string, options: any, serviceInstance: LanguageService): FileTextChanges[];

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

Expand Down