Merge pull request #16307 from RyanCavanaugh/refactor_refactor

Refactor refactor
This commit is contained in:
Ryan Cavanaugh 2017-06-06 22:28:52 -07:00 committed by GitHub
commit 6007eb7dfb
16 changed files with 267 additions and 95 deletions

View File

@ -113,7 +113,7 @@ class DeclarationsWalker {
}
}
function generateProtocolFile(protocolTs: string, typeScriptServicesDts: string): string {
function writeProtocolFile(outputFile: string, protocolTs: string, typeScriptServicesDts: string) {
const options = { target: ts.ScriptTarget.ES5, declaration: true, noResolve: true, types: <string[]>[], stripInternal: true };
/**
@ -163,14 +163,17 @@ function generateProtocolFile(protocolTs: string, typeScriptServicesDts: string)
protocolDts += "\nimport protocol = ts.server.protocol;";
protocolDts += "\nexport = protocol;";
protocolDts += "\nexport as namespace protocol;";
// do sanity check and try to compile generated text as standalone program
const sanityCheckProgram = getProgramWithProtocolText(protocolDts, /*includeTypeScriptServices*/ false);
const diagnostics = [...sanityCheckProgram.getSyntacticDiagnostics(), ...sanityCheckProgram.getSemanticDiagnostics(), ...sanityCheckProgram.getGlobalDiagnostics()];
ts.sys.writeFile(outputFile, protocolDts);
if (diagnostics.length) {
const flattenedDiagnostics = diagnostics.map(d => `${ts.flattenDiagnosticMessageText(d.messageText, "\n")} at ${d.file.fileName} line ${d.start}`).join("\n");
throw new Error(`Unexpected errors during sanity check: ${flattenedDiagnostics}`);
}
return protocolDts;
}
if (process.argv.length < 5) {
@ -181,5 +184,4 @@ if (process.argv.length < 5) {
const protocolTs = process.argv[2];
const typeScriptServicesDts = process.argv[3];
const outputFile = process.argv[4];
const generatedProtocolDts = generateProtocolFile(protocolTs, typeScriptServicesDts);
ts.sys.writeFile(outputFile, generatedProtocolDts);
writeProtocolFile(outputFile, protocolTs, typeScriptServicesDts);

View File

@ -2741,6 +2741,7 @@ namespace FourSlash {
markerName: string,
expectedContent: string,
refactorNameToApply: string,
actionName: string,
formattingOptions?: ts.FormatCodeSettings) {
formattingOptions = formattingOptions || this.formatCodeSettings;
@ -2753,9 +2754,11 @@ namespace FourSlash {
this.raiseError(`The expected refactor: ${refactorNameToApply} is not available at the marker location.`);
}
const codeActions = this.languageService.getRefactorCodeActions(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply);
const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply, actionName);
this.applyCodeActions(codeActions);
for (const edit of editInfo.edits) {
this.applyEdits(edit.fileName, edit.textChanges);
}
const actualContent = this.getFileContent(this.activeFile.fileName);
if (this.normalizeNewlines(actualContent) !== this.normalizeNewlines(expectedContent)) {
@ -3798,8 +3801,8 @@ 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 fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, actionName: string, formattingOptions?: ts.FormatCodeSettings): void {
this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorNameToApply, actionName, formattingOptions);
}
public rangeIs(expectedText: string, includeWhiteSpace?: boolean): void {

View File

@ -492,7 +492,7 @@ namespace Harness.LanguageService {
getCodeFixDiagnostics(): ts.Diagnostic[] {
throw new Error("Not supported on the shim.");
}
getRefactorCodeActions(): ts.CodeAction[] {
getEditsForRefactor(): ts.RefactorEditInfo {
throw new Error("Not supported on the shim.");
}
getApplicableRefactors(): ts.ApplicableRefactorInfo[] {

View File

@ -240,8 +240,8 @@ namespace ts.server {
CommandNames.GetCodeFixesFull,
CommandNames.GetSupportedCodeFixes,
CommandNames.GetApplicableRefactors,
CommandNames.GetRefactorCodeActions,
CommandNames.GetRefactorCodeActionsFull,
CommandNames.GetEditsForRefactor,
CommandNames.GetEditsForRefactorFull,
];
it("should not throw when commands are executed with invalid arguments", () => {

View File

@ -719,20 +719,49 @@ namespace ts.server {
return response.body;
}
getRefactorCodeActions(
getEditsForRefactor(
fileName: string,
_formatOptions: FormatCodeSettings,
positionOrRange: number | TextRange,
refactorName: string) {
refactorName: string,
actionName: string): RefactorEditInfo {
const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetRefactorCodeActionsRequestArgs;
args.refactorName = refactorName;
const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetEditsForRefactorRequestArgs;
args.refactor = refactorName;
args.action = actionName;
const request = this.processRequest<protocol.GetRefactorCodeActionsRequest>(CommandNames.GetRefactorCodeActions, args);
const response = this.processResponse<protocol.GetRefactorCodeActionsResponse>(request);
const codeActions = response.body.actions;
const request = this.processRequest<protocol.GetEditsForRefactorRequest>(CommandNames.GetEditsForRefactor, args);
const response = this.processResponse<protocol.GetEditsForRefactorResponse>(request);
return map(codeActions, codeAction => this.convertCodeActions(codeAction, fileName));
if (!response.body) {
return {
edits: []
};
}
const edits: FileTextChanges[] = this.convertCodeEditsToTextChanges(response.body.edits);
const renameFilename: string | undefined = response.body.renameFilename;
let renameLocation: number | undefined = undefined;
if (renameFilename !== undefined) {
renameLocation = this.lineOffsetToPosition(renameFilename, response.body.renameLocation);
}
return {
edits,
renameFilename,
renameLocation
};
}
private convertCodeEditsToTextChanges(edits: ts.server.protocol.FileCodeEdits[]): FileTextChanges[] {
return edits.map(edit => {
const fileName = edit.fileName;
return {
fileName,
textChanges: edit.textChanges.map(t => this.convertTextChangeToCodeEdit(t, fileName))
};
});
}
convertCodeActions(entry: protocol.CodeAction, fileName: string): CodeAction {

View File

@ -98,8 +98,11 @@ namespace ts.server.protocol {
GetSupportedCodeFixes = "getSupportedCodeFixes",
GetApplicableRefactors = "getApplicableRefactors",
GetRefactorCodeActions = "getRefactorCodeActions",
GetRefactorCodeActionsFull = "getRefactorCodeActions-full",
GetEditsForRefactor = "getEditsForRefactor",
/* @internal */
GetEditsForRefactorFull = "getEditsForRefactor-full",
// NOTE: If updating this, be sure to also update `allCommandNames` in `harness/unittests/session.ts`.
}
/**
@ -401,52 +404,98 @@ namespace ts.server.protocol {
export type FileLocationOrRangeRequestArgs = FileLocationRequestArgs | FileRangeRequestArgs;
/**
* Request refactorings at a given position or selection area.
*/
export interface GetApplicableRefactorsRequest extends Request {
command: CommandTypes.GetApplicableRefactors;
arguments: GetApplicableRefactorsRequestArgs;
}
export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs;
export interface ApplicableRefactorInfo {
name: string;
description: string;
}
/**
* Response is a list of available refactorings.
* Each refactoring exposes one or more "Actions"; a user selects one action to invoke a refactoring
*/
export interface GetApplicableRefactorsResponse extends Response {
body?: ApplicableRefactorInfo[];
}
export interface GetRefactorCodeActionsRequest extends Request {
command: CommandTypes.GetRefactorCodeActions;
arguments: GetRefactorCodeActionsRequestArgs;
/**
* A set of one or more available refactoring actions, grouped under a parent refactoring.
*/
export interface ApplicableRefactorInfo {
/**
* The programmatic name of the refactoring
*/
name: string;
/**
* A description of this refactoring category to show to the user.
* If the refactoring gets inlined (see below), this text will not be visible.
*/
description: string;
/**
* Inlineable refactorings can have their actions hoisted out to the top level
* of a context menu. Non-inlineanable refactorings should always be shown inside
* their parent grouping.
*
* If not specified, this value is assumed to be 'true'
*/
inlineable?: boolean;
actions: RefactorActionInfo[];
}
export type GetRefactorCodeActionsRequestArgs = FileLocationOrRangeRequestArgs & {
/* The kind of the applicable refactor */
refactorName: string;
/**
* Represents a single refactoring action - for example, the "Extract Method..." refactor might
* offer several actions, each corresponding to a surround class or closure to extract into.
*/
export type RefactorActionInfo = {
/**
* The programmatic name of the refactoring action
*/
name: string;
/**
* A description of this refactoring action to show to the user.
* If the parent refactoring is inlined away, this will be the only text shown,
* so this description should make sense by itself if the parent is inlineable=true
*/
description: string;
};
export type RefactorCodeActions = {
actions: protocol.CodeAction[];
renameLocation?: number
};
/* @internal */
export type RefactorCodeActionsFull = {
actions: ts.CodeAction[];
renameLocation?: number
};
export interface GetRefactorCodeActionsResponse extends Response {
body: RefactorCodeActions;
export interface GetEditsForRefactorRequest extends Request {
command: CommandTypes.GetEditsForRefactor;
arguments: GetEditsForRefactorRequestArgs;
}
/* @internal */
export interface GetRefactorCodeActionsFullResponse extends Response {
body: RefactorCodeActionsFull;
/**
* Request the edits that a particular refactoring action produces.
* Callers must specify the name of the refactor and the name of the action.
*/
export type GetEditsForRefactorRequestArgs = FileLocationOrRangeRequestArgs & {
/* The 'name' property from the refactoring that offered this action */
refactor: string;
/* The 'name' property from the refactoring action */
action: string;
};
export interface GetEditsForRefactorResponse extends Response {
body?: RefactorEditInfo;
}
export type RefactorEditInfo = {
edits: FileCodeEdits[];
/**
* An optional location where the editor should start a rename operation once
* the refactoring edits have been applied
*/
renameLocation?: Location;
renameFilename?: string;
};
/**
* Request for the available codefixes at a specific position.
*/

View File

@ -1425,29 +1425,40 @@ namespace ts.server {
return project.getLanguageService().getApplicableRefactors(file, position || textRange);
}
private getRefactorCodeActions(args: protocol.GetRefactorCodeActionsRequestArgs, simplifiedResult: boolean): protocol.RefactorCodeActions | protocol.RefactorCodeActionsFull {
private getEditsForRefactor(args: protocol.GetEditsForRefactorRequestArgs, simplifiedResult: boolean): ts.RefactorEditInfo | protocol.RefactorEditInfo {
const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args);
const scriptInfo = project.getScriptInfoForNormalizedPath(file);
const { position, textRange } = this.extractPositionAndRange(args, scriptInfo);
const result: ts.CodeAction[] = project.getLanguageService().getRefactorCodeActions(
const result = project.getLanguageService().getEditsForRefactor(
file,
this.projectService.getFormatCodeOptions(),
position || textRange,
args.refactorName
args.refactor,
args.action
);
if (simplifiedResult) {
// Not full
if (result === undefined) {
return {
actions: result.map(action => this.mapCodeAction(action, scriptInfo))
edits: []
};
}
if (simplifiedResult) {
const file = result.renameFilename;
let location: ILineInfo | undefined = undefined;
if (file !== undefined && result.renameLocation !== undefined) {
const renameScriptInfo = project.getScriptInfoForNormalizedPath(toNormalizedPath(file));
location = renameScriptInfo.positionToLineOffset(result.renameLocation);
}
return {
renameLocation: location,
renameFilename: file,
edits: result.edits.map(change => this.mapTextChangesToCodeEdits(project, change))
};
}
else {
// Full
return {
actions: result
};
return result;
}
}
@ -1505,6 +1516,14 @@ namespace ts.server {
};
}
private mapTextChangesToCodeEdits(project: Project, textChanges: FileTextChanges): protocol.FileCodeEdits {
const scriptInfo = project.getScriptInfoForNormalizedPath(toNormalizedPath(textChanges.fileName));
return {
fileName: textChanges.fileName,
textChanges: textChanges.textChanges.map(textChange => this.convertTextChangeToCodeEdit(textChange, scriptInfo))
};
}
private convertTextChangeToCodeEdit(change: ts.TextChange, scriptInfo: ScriptInfo): protocol.CodeEdit {
return {
start: scriptInfo.positionToLineOffset(change.span.start),
@ -1833,11 +1852,11 @@ namespace ts.server {
[CommandNames.GetApplicableRefactors]: (request: protocol.GetApplicableRefactorsRequest) => {
return this.requiredResponse(this.getApplicableRefactors(request.arguments));
},
[CommandNames.GetRefactorCodeActions]: (request: protocol.GetRefactorCodeActionsRequest) => {
return this.requiredResponse(this.getRefactorCodeActions(request.arguments, /*simplifiedResult*/ true));
[CommandNames.GetEditsForRefactor]: (request: protocol.GetEditsForRefactorRequest) => {
return this.requiredResponse(this.getEditsForRefactor(request.arguments, /*simplifiedResult*/ true));
},
[CommandNames.GetRefactorCodeActionsFull]: (request: protocol.GetRefactorCodeActionsRequest) => {
return this.requiredResponse(this.getRefactorCodeActions(request.arguments, /*simplifiedResult*/ false));
[CommandNames.GetEditsForRefactorFull]: (request: protocol.GetEditsForRefactorRequest) => {
return this.requiredResponse(this.getEditsForRefactor(request.arguments, /*simplifiedResult*/ false));
}
});

View File

@ -8,10 +8,10 @@ namespace ts {
description: string;
/** Compute the associated code actions */
getCodeActions(context: RefactorContext): CodeAction[];
getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined;
/** A fast syntactic check to see if the refactor is applicable at given position. */
isApplicable(context: RefactorContext): boolean;
/** Compute (quickly) which actions are available here */
getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined;
}
export interface RefactorContext {
@ -34,7 +34,6 @@ namespace ts {
}
export function getApplicableRefactors(context: RefactorContext): ApplicableRefactorInfo[] | undefined {
let results: ApplicableRefactorInfo[];
const refactorList: Refactor[] = [];
refactors.forEach(refactor => {
@ -44,16 +43,17 @@ namespace ts {
if (context.cancellationToken && context.cancellationToken.isCancellationRequested()) {
return results;
}
if (refactor.isApplicable(context)) {
(results || (results = [])).push({ name: refactor.name, description: refactor.description });
const infos = refactor.getAvailableActions(context);
if (infos && infos.length) {
(results || (results = [])).push(...infos);
}
}
return results;
}
export function getRefactorCodeActions(context: RefactorContext, refactorName: string): CodeAction[] | undefined {
export function getEditsForRefactor(context: RefactorContext, refactorName: string, actionName: string): RefactorEditInfo | undefined {
const refactor = refactors.get(refactorName);
return refactor && refactor.getCodeActions(context);
return refactor && refactor.getEditsForAction(context, actionName);
}
}
}

View File

@ -1,16 +1,18 @@
/* @internal */
namespace ts.refactor {
const actionName = "convert";
const convertFunctionToES6Class: Refactor = {
name: "Convert to ES2015 class",
description: Diagnostics.Convert_function_to_an_ES2015_class.message,
getCodeActions,
isApplicable
getEditsForAction,
getAvailableActions
};
registerRefactor(convertFunctionToES6Class);
function isApplicable(context: RefactorContext): boolean {
function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] {
const start = context.startPosition;
const node = getTokenAtPosition(context.file, start, /*includeJsDocComment*/ false);
const checker = context.program.getTypeChecker();
@ -20,10 +22,28 @@ namespace ts.refactor {
symbol = (symbol.valueDeclaration as VariableDeclaration).initializer.symbol;
}
return symbol && symbol.flags & SymbolFlags.Function && symbol.members && symbol.members.size > 0;
if (symbol && (symbol.flags & SymbolFlags.Function) && symbol.members && (symbol.members.size > 0)) {
return [
{
name: convertFunctionToES6Class.name,
description: convertFunctionToES6Class.description,
actions: [
{
description: convertFunctionToES6Class.description,
name: actionName
}
]
}
];
}
}
function getCodeActions(context: RefactorContext): CodeAction[] | undefined {
function getEditsForAction(context: RefactorContext, action: string): RefactorEditInfo | undefined {
// Somehow wrong action got invoked?
if (actionName !== action) {
return undefined;
}
const start = context.startPosition;
const sourceFile = context.file;
const checker = context.program.getTypeChecker();
@ -35,7 +55,7 @@ namespace ts.refactor {
const deletes: (() => any)[] = [];
if (!(ctorSymbol.flags & (SymbolFlags.Function | SymbolFlags.Variable))) {
return [];
return undefined;
}
const ctorDeclaration = ctorSymbol.valueDeclaration;
@ -63,7 +83,7 @@ namespace ts.refactor {
}
if (!newClassDeclaration) {
return [];
return undefined;
}
// Because the preceding node could be touched, we need to insert nodes before delete nodes.
@ -72,10 +92,9 @@ namespace ts.refactor {
deleteCallback();
}
return [{
description: formatStringFromArgs(Diagnostics.Convert_function_0_to_class.message, [ctorSymbol.name]),
changes: changeTracker.getChanges()
}];
return {
edits: changeTracker.getChanges()
};
function deleteNode(node: Node, inList = false) {
if (deletedNodes.some(n => isNodeDescendantOf(node, n))) {

View File

@ -1999,15 +1999,16 @@ namespace ts {
return refactor.getApplicableRefactors(getRefactorContext(file, positionOrRange));
}
function getRefactorCodeActions(
function getEditsForRefactor(
fileName: string,
formatOptions: FormatCodeSettings,
positionOrRange: number | TextRange,
refactorName: string): CodeAction[] | undefined {
refactorName: string,
actionName: string): RefactorEditInfo {
synchronizeHostData();
const file = getValidSourceFile(fileName);
return refactor.getRefactorCodeActions(getRefactorContext(file, positionOrRange, formatOptions), refactorName);
return refactor.getEditsForRefactor(getRefactorContext(file, positionOrRange, formatOptions), refactorName, actionName);
}
return {
@ -2015,8 +2016,6 @@ namespace ts {
cleanupSemanticCache,
getSyntacticDiagnostics,
getSemanticDiagnostics,
getApplicableRefactors,
getRefactorCodeActions,
getCompilerOptionsDiagnostics,
getSyntacticClassifications,
getSemanticClassifications,
@ -2054,7 +2053,9 @@ namespace ts {
getEmitOutput,
getNonBoundSourceFile,
getSourceFile,
getProgram
getProgram,
getApplicableRefactors,
getEditsForRefactor,
};
}

View File

@ -265,8 +265,9 @@ namespace ts {
isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean;
getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: FormatCodeSettings): CodeAction[];
getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[];
getRefactorCodeActions(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string): CodeAction[] | undefined;
getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string): RefactorEditInfo | undefined;
getEmitOutput(fileName: string, emitOnlyDtsFiles?: boolean): EmitOutput;
@ -357,11 +358,60 @@ namespace ts {
changes: FileTextChanges[];
}
/**
* A set of one or more available refactoring actions, grouped under a parent refactoring.
*/
export interface ApplicableRefactorInfo {
/**
* The programmatic name of the refactoring
*/
name: string;
/**
* A description of this refactoring category to show to the user.
* If the refactoring gets inlined (see below), this text will not be visible.
*/
description: string;
/**
* Inlineable refactorings can have their actions hoisted out to the top level
* of a context menu. Non-inlineanable refactorings should always be shown inside
* their parent grouping.
*
* If not specified, this value is assumed to be 'true'
*/
inlineable?: boolean;
actions: RefactorActionInfo[];
}
/**
* Represents a single refactoring action - for example, the "Extract Method..." refactor might
* offer several actions, each corresponding to a surround class or closure to extract into.
*/
export type RefactorActionInfo = {
/**
* The programmatic name of the refactoring action
*/
name: string;
/**
* A description of this refactoring action to show to the user.
* If the parent refactoring is inlined away, this will be the only text shown,
* so this description should make sense by itself if the parent is inlineable=true
*/
description: string;
};
/**
* A set of edits to make in response to a refactor action, plus an optional
* location where renaming should be invoked from
*/
export type RefactorEditInfo = {
edits: FileTextChanges[];
renameFilename?: string;
renameLocation?: number;
};
export interface TextInsertion {
newText: string;
/** The position in newText the caret should point to after the insertion. */

View File

@ -23,4 +23,4 @@ verify.fileAfterApplyingRefactorAtMarker('1',
foo.prototype.instanceProp1 = "hello";
foo.prototype.instanceProp2 = undefined;
foo.staticProp = "world";
`, 'Convert to ES2015 class');
`, 'Convert to ES2015 class', 'convert');

View File

@ -24,4 +24,4 @@ verify.fileAfterApplyingRefactorAtMarker('4',
foo.instanceProp1 = "hello";
foo.instanceProp2 = undefined;
foo.staticProp = "world";
`, 'Convert to ES2015 class');
`, 'Convert to ES2015 class', 'convert');

View File

@ -25,4 +25,4 @@ class foo {
foo.prototype.instanceProp1 = "hello";
foo.prototype.instanceProp2 = undefined;
foo.staticProp = "world";
`, 'Convert to ES2015 class');
`, 'Convert to ES2015 class', 'convert');

View File

@ -236,8 +236,8 @@ declare namespace FourSlashInterface {
noMatchingBracePositionInCurrentFile(bracePosition: number): void;
DocCommentTemplate(expectedText: string, expectedOffset: number, empty?: boolean): void;
noDocCommentTemplate(): void;
rangeAfterCodeFix(expectedText: string, includeWhiteSpace?: boolean, errorCode?: number, index?: number): void
getAndApplyCodeFix(errorCode?: number, index?: number): void;
rangeAfterCodeFix(expectedText: string, includeWhiteSpace?: boolean, errorCode?: number, index?: number): void;
fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, actionName: string, formattingOptions?: FormatCodeOptions): void;
rangeIs(expectedText: string, includeWhiteSpace?: boolean): void;
fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, formattingOptions?: FormatCodeOptions): void;
importFixAtPosition(expectedTextArray: string[], errorCode?: number): void;

View File

@ -22,4 +22,4 @@ class fn {
console.log('hello world');
}
}
`, 'Convert to ES2015 class');
`, 'Convert to ES2015 class', 'convert');