Skip to content

Commit 6007eb7

Browse files
RyanCavanaughRyanCavanaugh
authored
Merge pull request #16307 from RyanCavanaugh/refactor_refactor
Refactor refactor
2 parents b217c39 + f725d7d commit 6007eb7

16 files changed

+263
-91
lines changed

‎scripts/buildProtocol.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class DeclarationsWalker {
113113
}
114114
}
115115

116-
function generateProtocolFile(protocolTs: string, typeScriptServicesDts: string): string {
116+
function writeProtocolFile(outputFile: string, protocolTs: string, typeScriptServicesDts: string) {
117117
const options = { target: ts.ScriptTarget.ES5, declaration: true, noResolve: true, types: <string[]>[], stripInternal: true };
118118

119119
/**
@@ -163,14 +163,17 @@ function generateProtocolFile(protocolTs: string, typeScriptServicesDts: string)
163163
protocolDts += "\nimport protocol = ts.server.protocol;";
164164
protocolDts += "\nexport = protocol;";
165165
protocolDts += "\nexport as namespace protocol;";
166+
166167
// do sanity check and try to compile generated text as standalone program
167168
const sanityCheckProgram = getProgramWithProtocolText(protocolDts, /*includeTypeScriptServices*/ false);
168169
const diagnostics = [...sanityCheckProgram.getSyntacticDiagnostics(), ...sanityCheckProgram.getSemanticDiagnostics(), ...sanityCheckProgram.getGlobalDiagnostics()];
170+
171+
ts.sys.writeFile(outputFile, protocolDts);
172+
169173
if (diagnostics.length) {
170174
const flattenedDiagnostics = diagnostics.map(d => `${ts.flattenDiagnosticMessageText(d.messageText, "\n")} at ${d.file.fileName} line ${d.start}`).join("\n");
171175
throw new Error(`Unexpected errors during sanity check: ${flattenedDiagnostics}`);
172176
}
173-
return protocolDts;
174177
}
175178

176179
if (process.argv.length < 5) {
@@ -181,5 +184,4 @@ if (process.argv.length < 5) {
181184
const protocolTs = process.argv[2];
182185
const typeScriptServicesDts = process.argv[3];
183186
const outputFile = process.argv[4];
184-
const generatedProtocolDts = generateProtocolFile(protocolTs, typeScriptServicesDts);
185-
ts.sys.writeFile(outputFile, generatedProtocolDts);
187+
writeProtocolFile(outputFile, protocolTs, typeScriptServicesDts);

‎src/harness/fourslash.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2741,6 +2741,7 @@ namespace FourSlash {
27412741
markerName: string,
27422742
expectedContent: string,
27432743
refactorNameToApply: string,
2744+
actionName: string,
27442745
formattingOptions?: ts.FormatCodeSettings) {
27452746

27462747
formattingOptions = formattingOptions || this.formatCodeSettings;
@@ -2753,9 +2754,11 @@ namespace FourSlash {
27532754
this.raiseError(`The expected refactor: ${refactorNameToApply} is not available at the marker location.`);
27542755
}
27552756

2756-
const codeActions = this.languageService.getRefactorCodeActions(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply);
2757+
const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply, actionName);
27572758

2758-
this.applyCodeActions(codeActions);
2759+
for (const edit of editInfo.edits) {
2760+
this.applyEdits(edit.fileName, edit.textChanges);
2761+
}
27592762
const actualContent = this.getFileContent(this.activeFile.fileName);
27602763

27612764
if (this.normalizeNewlines(actualContent) !== this.normalizeNewlines(expectedContent)) {
@@ -3798,8 +3801,8 @@ namespace FourSlashInterface {
37983801
this.state.verifyRangeAfterCodeFix(expectedText, includeWhiteSpace, errorCode, index);
37993802
}
38003803

3801-
public fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, formattingOptions?: ts.FormatCodeSettings): void {
3802-
this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorNameToApply, formattingOptions);
3804+
public fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, actionName: string, formattingOptions?: ts.FormatCodeSettings): void {
3805+
this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorNameToApply, actionName, formattingOptions);
38033806
}
38043807

38053808
public rangeIs(expectedText: string, includeWhiteSpace?: boolean): void {

‎src/harness/harnessLanguageService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ namespace Harness.LanguageService {
492492
getCodeFixDiagnostics(): ts.Diagnostic[] {
493493
throw new Error("Not supported on the shim.");
494494
}
495-
getRefactorCodeActions(): ts.CodeAction[] {
495+
getEditsForRefactor(): ts.RefactorEditInfo {
496496
throw new Error("Not supported on the shim.");
497497
}
498498
getApplicableRefactors(): ts.ApplicableRefactorInfo[] {

‎src/harness/unittests/session.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,8 @@ namespace ts.server {
240240
CommandNames.GetCodeFixesFull,
241241
CommandNames.GetSupportedCodeFixes,
242242
CommandNames.GetApplicableRefactors,
243-
CommandNames.GetRefactorCodeActions,
244-
CommandNames.GetRefactorCodeActionsFull,
243+
CommandNames.GetEditsForRefactor,
244+
CommandNames.GetEditsForRefactorFull,
245245
];
246246

247247
it("should not throw when commands are executed with invalid arguments", () => {

‎src/server/client.ts

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -719,20 +719,49 @@ namespace ts.server {
719719
return response.body;
720720
}
721721

722-
getRefactorCodeActions(
722+
getEditsForRefactor(
723723
fileName: string,
724724
_formatOptions: FormatCodeSettings,
725725
positionOrRange: number | TextRange,
726-
refactorName: string) {
726+
refactorName: string,
727+
actionName: string): RefactorEditInfo {
727728

728-
const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetRefactorCodeActionsRequestArgs;
729-
args.refactorName = refactorName;
729+
const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetEditsForRefactorRequestArgs;
730+
args.refactor = refactorName;
731+
args.action = actionName;
730732

731-
const request = this.processRequest<protocol.GetRefactorCodeActionsRequest>(CommandNames.GetRefactorCodeActions, args);
732-
const response = this.processResponse<protocol.GetRefactorCodeActionsResponse>(request);
733-
const codeActions = response.body.actions;
733+
const request = this.processRequest<protocol.GetEditsForRefactorRequest>(CommandNames.GetEditsForRefactor, args);
734+
const response = this.processResponse<protocol.GetEditsForRefactorResponse>(request);
734735

735-
return map(codeActions, codeAction => this.convertCodeActions(codeAction, fileName));
736+
if (!response.body) {
737+
return {
738+
edits: []
739+
};
740+
}
741+
742+
const edits: FileTextChanges[] = this.convertCodeEditsToTextChanges(response.body.edits);
743+
744+
const renameFilename: string | undefined = response.body.renameFilename;
745+
let renameLocation: number | undefined = undefined;
746+
if (renameFilename !== undefined) {
747+
renameLocation = this.lineOffsetToPosition(renameFilename, response.body.renameLocation);
748+
}
749+
750+
return {
751+
edits,
752+
renameFilename,
753+
renameLocation
754+
};
755+
}
756+
757+
private convertCodeEditsToTextChanges(edits: ts.server.protocol.FileCodeEdits[]): FileTextChanges[] {
758+
return edits.map(edit => {
759+
const fileName = edit.fileName;
760+
return {
761+
fileName,
762+
textChanges: edit.textChanges.map(t => this.convertTextChangeToCodeEdit(t, fileName))
763+
};
764+
});
736765
}
737766

738767
convertCodeActions(entry: protocol.CodeAction, fileName: string): CodeAction {

‎src/server/protocol.ts

Lines changed: 76 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,11 @@ namespace ts.server.protocol {
9898
GetSupportedCodeFixes = "getSupportedCodeFixes",
9999

100100
GetApplicableRefactors = "getApplicableRefactors",
101-
GetRefactorCodeActions = "getRefactorCodeActions",
102-
GetRefactorCodeActionsFull = "getRefactorCodeActions-full",
101+
GetEditsForRefactor = "getEditsForRefactor",
102+
/* @internal */
103+
GetEditsForRefactorFull = "getEditsForRefactor-full",
104+
105+
// NOTE: If updating this, be sure to also update `allCommandNames` in `harness/unittests/session.ts`.
103106
}
104107

105108
/**
@@ -401,52 +404,98 @@ namespace ts.server.protocol {
401404

402405
export type FileLocationOrRangeRequestArgs = FileLocationRequestArgs | FileRangeRequestArgs;
403406

407+
/**
408+
* Request refactorings at a given position or selection area.
409+
*/
404410
export interface GetApplicableRefactorsRequest extends Request {
405411
command: CommandTypes.GetApplicableRefactors;
406412
arguments: GetApplicableRefactorsRequestArgs;
407413
}
408-
409414
export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs;
410415

416+
/**
417+
* Response is a list of available refactorings.
418+
* Each refactoring exposes one or more "Actions"; a user selects one action to invoke a refactoring
419+
*/
420+
export interface GetApplicableRefactorsResponse extends Response {
421+
body?: ApplicableRefactorInfo[];
422+
}
423+
424+
/**
425+
* A set of one or more available refactoring actions, grouped under a parent refactoring.
426+
*/
411427
export interface ApplicableRefactorInfo {
428+
/**
429+
* The programmatic name of the refactoring
430+
*/
412431
name: string;
432+
/**
433+
* A description of this refactoring category to show to the user.
434+
* If the refactoring gets inlined (see below), this text will not be visible.
435+
*/
413436
description: string;
414-
}
437+
/**
438+
* Inlineable refactorings can have their actions hoisted out to the top level
439+
* of a context menu. Non-inlineanable refactorings should always be shown inside
440+
* their parent grouping.
441+
*
442+
* If not specified, this value is assumed to be 'true'
443+
*/
444+
inlineable?: boolean;
415445

416-
export interface GetApplicableRefactorsResponse extends Response {
417-
body?: ApplicableRefactorInfo[];
446+
actions: RefactorActionInfo[];
418447
}
419448

420-
export interface GetRefactorCodeActionsRequest extends Request {
421-
command: CommandTypes.GetRefactorCodeActions;
422-
arguments: GetRefactorCodeActionsRequestArgs;
423-
}
449+
/**
450+
* Represents a single refactoring action - for example, the "Extract Method..." refactor might
451+
* offer several actions, each corresponding to a surround class or closure to extract into.
452+
*/
453+
export type RefactorActionInfo = {
454+
/**
455+
* The programmatic name of the refactoring action
456+
*/
457+
name: string;
424458

425-
export type GetRefactorCodeActionsRequestArgs = FileLocationOrRangeRequestArgs & {
426-
/* The kind of the applicable refactor */
427-
refactorName: string;
459+
/**
460+
* A description of this refactoring action to show to the user.
461+
* If the parent refactoring is inlined away, this will be the only text shown,
462+
* so this description should make sense by itself if the parent is inlineable=true
463+
*/
464+
description: string;
428465
};
429466

430-
export type RefactorCodeActions = {
431-
actions: protocol.CodeAction[];
432-
renameLocation?: number
433-
};
467+
export interface GetEditsForRefactorRequest extends Request {
468+
command: CommandTypes.GetEditsForRefactor;
469+
arguments: GetEditsForRefactorRequestArgs;
470+
}
434471

435-
/* @internal */
436-
export type RefactorCodeActionsFull = {
437-
actions: ts.CodeAction[];
438-
renameLocation?: number
472+
/**
473+
* Request the edits that a particular refactoring action produces.
474+
* Callers must specify the name of the refactor and the name of the action.
475+
*/
476+
export type GetEditsForRefactorRequestArgs = FileLocationOrRangeRequestArgs & {
477+
/* The 'name' property from the refactoring that offered this action */
478+
refactor: string;
479+
/* The 'name' property from the refactoring action */
480+
action: string;
439481
};
440482

441-
export interface GetRefactorCodeActionsResponse extends Response {
442-
body: RefactorCodeActions;
443-
}
444483

445-
/* @internal */
446-
export interface GetRefactorCodeActionsFullResponse extends Response {
447-
body: RefactorCodeActionsFull;
484+
export interface GetEditsForRefactorResponse extends Response {
485+
body?: RefactorEditInfo;
448486
}
449487

488+
export type RefactorEditInfo = {
489+
edits: FileCodeEdits[];
490+
491+
/**
492+
* An optional location where the editor should start a rename operation once
493+
* the refactoring edits have been applied
494+
*/
495+
renameLocation?: Location;
496+
renameFilename?: string;
497+
};
498+
450499
/**
451500
* Request for the available codefixes at a specific position.
452501
*/

‎src/server/session.ts

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1425,30 +1425,41 @@ namespace ts.server {
14251425
return project.getLanguageService().getApplicableRefactors(file, position || textRange);
14261426
}
14271427

1428-
private getRefactorCodeActions(args: protocol.GetRefactorCodeActionsRequestArgs, simplifiedResult: boolean): protocol.RefactorCodeActions | protocol.RefactorCodeActionsFull {
1428+
private getEditsForRefactor(args: protocol.GetEditsForRefactorRequestArgs, simplifiedResult: boolean): ts.RefactorEditInfo | protocol.RefactorEditInfo {
14291429
const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args);
14301430
const scriptInfo = project.getScriptInfoForNormalizedPath(file);
14311431
const { position, textRange } = this.extractPositionAndRange(args, scriptInfo);
14321432

1433-
const result: ts.CodeAction[] = project.getLanguageService().getRefactorCodeActions(
1433+
const result = project.getLanguageService().getEditsForRefactor(
14341434
file,
14351435
this.projectService.getFormatCodeOptions(),
14361436
position || textRange,
1437-
args.refactorName
1437+
args.refactor,
1438+
args.action
14381439
);
14391440

1440-
if (simplifiedResult) {
1441-
// Not full
1441+
if (result === undefined) {
14421442
return {
1443-
actions: result.map(action => this.mapCodeAction(action, scriptInfo))
1443+
edits: []
14441444
};
14451445
}
1446-
else {
1447-
// Full
1446+
1447+
if (simplifiedResult) {
1448+
const file = result.renameFilename;
1449+
let location: ILineInfo | undefined = undefined;
1450+
if (file !== undefined && result.renameLocation !== undefined) {
1451+
const renameScriptInfo = project.getScriptInfoForNormalizedPath(toNormalizedPath(file));
1452+
location = renameScriptInfo.positionToLineOffset(result.renameLocation);
1453+
}
14481454
return {
1449-
actions: result
1455+
renameLocation: location,
1456+
renameFilename: file,
1457+
edits: result.edits.map(change => this.mapTextChangesToCodeEdits(project, change))
14501458
};
14511459
}
1460+
else {
1461+
return result;
1462+
}
14521463
}
14531464

14541465
private getCodeFixes(args: protocol.CodeFixRequestArgs, simplifiedResult: boolean): protocol.CodeAction[] | CodeAction[] {
@@ -1505,6 +1516,14 @@ namespace ts.server {
15051516
};
15061517
}
15071518

1519+
private mapTextChangesToCodeEdits(project: Project, textChanges: FileTextChanges): protocol.FileCodeEdits {
1520+
const scriptInfo = project.getScriptInfoForNormalizedPath(toNormalizedPath(textChanges.fileName));
1521+
return {
1522+
fileName: textChanges.fileName,
1523+
textChanges: textChanges.textChanges.map(textChange => this.convertTextChangeToCodeEdit(textChange, scriptInfo))
1524+
};
1525+
}
1526+
15081527
private convertTextChangeToCodeEdit(change: ts.TextChange, scriptInfo: ScriptInfo): protocol.CodeEdit {
15091528
return {
15101529
start: scriptInfo.positionToLineOffset(change.span.start),
@@ -1833,11 +1852,11 @@ namespace ts.server {
18331852
[CommandNames.GetApplicableRefactors]: (request: protocol.GetApplicableRefactorsRequest) => {
18341853
return this.requiredResponse(this.getApplicableRefactors(request.arguments));
18351854
},
1836-
[CommandNames.GetRefactorCodeActions]: (request: protocol.GetRefactorCodeActionsRequest) => {
1837-
return this.requiredResponse(this.getRefactorCodeActions(request.arguments, /*simplifiedResult*/ true));
1855+
[CommandNames.GetEditsForRefactor]: (request: protocol.GetEditsForRefactorRequest) => {
1856+
return this.requiredResponse(this.getEditsForRefactor(request.arguments, /*simplifiedResult*/ true));
18381857
},
1839-
[CommandNames.GetRefactorCodeActionsFull]: (request: protocol.GetRefactorCodeActionsRequest) => {
1840-
return this.requiredResponse(this.getRefactorCodeActions(request.arguments, /*simplifiedResult*/ false));
1858+
[CommandNames.GetEditsForRefactorFull]: (request: protocol.GetEditsForRefactorRequest) => {
1859+
return this.requiredResponse(this.getEditsForRefactor(request.arguments, /*simplifiedResult*/ false));
18411860
}
18421861
});
18431862

0 commit comments

Comments
 (0)