Skip to content

Commit b357841

Browse files
committed
Redesign APIs
1 parent fa5a483 commit b357841

File tree

10 files changed

+322
-238
lines changed

10 files changed

+322
-238
lines changed

src/compiler/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3301,6 +3301,7 @@ namespace ts {
33013301
Warning,
33023302
Error,
33033303
Message,
3304+
Refactor
33043305
}
33053306

33063307
export enum ModuleResolutionKind {

src/harness/fourslash.ts

Lines changed: 87 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -489,8 +489,8 @@ namespace FourSlash {
489489
return diagnostics;
490490
}
491491

492-
private getRefactorDiagnostics(fileName: string, range?: ts.TextRange): ts.RefactorDiagnostic[] {
493-
return this.languageService.getRefactorDiagnostics(fileName, range);
492+
private getRefactorDiagnostics(fileName: string): ts.Diagnostic[] {
493+
return this.languageService.getRefactorDiagnostics(fileName);
494494
}
495495

496496
private getAllDiagnostics(): ts.Diagnostic[] {
@@ -2200,20 +2200,15 @@ namespace FourSlash {
22002200
return actions;
22012201
}
22022202

2203-
private getRefactorActions(fileName: string, range?: ts.TextRange, formattingOptions?: ts.FormatCodeSettings): ts.CodeAction[] {
2204-
const diagnostics = this.getRefactorDiagnostics(fileName, range);
2205-
const actions: ts.CodeAction[] = [];
2206-
formattingOptions = formattingOptions || this.formatCodeSettings;
2203+
private applyAllCodeActions(fileName: string, codeActions: ts.CodeAction[]): void {
2204+
if (!codeActions) {
2205+
return;
2206+
}
22072207

2208-
for (const diagnostic of diagnostics) {
2209-
const diagnosticRange: ts.TextRange = {
2210-
pos: diagnostic.start,
2211-
end: diagnostic.end
2212-
};
2213-
const newActions = this.languageService.getCodeActionsForRefactorAtPosition(fileName, diagnosticRange, diagnostic.code, formattingOptions);
2214-
actions.push.apply(actions, newActions);
2208+
for (const codeAction of codeActions) {
2209+
const fileChanges = ts.find(codeAction.changes, change => change.fileName === fileName);
2210+
this.applyEdits(fileChanges.fileName, fileChanges.textChanges, /*isFormattingEdit*/ false);
22152211
}
2216-
return actions;
22172212
}
22182213

22192214
private applyCodeAction(fileName: string, actions: ts.CodeAction[], index?: number): void {
@@ -2574,42 +2569,79 @@ namespace FourSlash {
25742569
}
25752570
}
25762571

2577-
public verifyRefactorAvailable(negative: boolean) {
2578-
// The ranges are used only when the refactors require a range as input information. For example the "extractMethod" refactor
2579-
// onlye one range is allowed per test
2580-
const ranges = this.getRanges();
2581-
if (ranges.length > 1) {
2582-
throw new Error("only one refactor range is allowed per test");
2572+
public verifyRefactorDiagnosticsAvailableAtMarker(negative: boolean, markerName: string, diagnosticCode?: number) {
2573+
const marker = this.getMarkerByName(markerName);
2574+
const markerPos = marker.position;
2575+
let foundDiagnostic = false;
2576+
2577+
const refactorDiagnostics = this.getRefactorDiagnostics(this.activeFile.fileName);
2578+
for (const diag of refactorDiagnostics) {
2579+
if (diag.start <= markerPos && diag.start + diag.length >= markerPos) {
2580+
foundDiagnostic = diagnosticCode === undefined || diagnosticCode === diag.code;
2581+
}
2582+
}
2583+
2584+
if (negative && foundDiagnostic) {
2585+
this.raiseError(`verifyRefactorDiagnosticsAvailableAtMarker failed - expected no refactor diagnostic at marker ${markerName} but found some.`);
25832586
}
2587+
if (!negative && !foundDiagnostic) {
2588+
this.raiseError(`verifyRefactorDiagnosticsAvailableAtMarker failed - expected a refactor diagnostic at marker ${markerName} but found none.`);
2589+
}
2590+
}
25842591

2585-
const range = ranges[0] ? { pos: ranges[0].start, end: ranges[0].end } : undefined;
2586-
const refactorDiagnostics = this.getRefactorDiagnostics(this.activeFile.fileName, range);
2587-
if (negative && refactorDiagnostics.length > 0) {
2588-
this.raiseError(`verifyRefactorAvailable failed - expected no refactors but found some.`);
2592+
public verifyApplicableRefactorAvailableAtMarker(negative: boolean, markerName: string) {
2593+
const marker = this.getMarkerByName(markerName);
2594+
const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, marker.position);
2595+
const isAvailable = applicableRefactors && applicableRefactors.length > 0;
2596+
if (negative && isAvailable) {
2597+
this.raiseError(`verifyApplicableRefactorAvailableAtMarker failed - expected no refactor at marker ${markerName} but found some.`);
25892598
}
2590-
if (!negative && refactorDiagnostics.length === 0) {
2591-
this.raiseError(`verifyRefactorAvailable failed: expected refactor diagnostics but none found.`);
2599+
if (!negative && !isAvailable) {
2600+
this.raiseError(`verifyApplicableRefactorAvailableAtMarker failed - expected a refactor at marker ${markerName} but found none.`);
25922601
}
25932602
}
25942603

2595-
public verifyFileAfterApplyingRefactors(expectedContent: string, formattingOptions?: ts.FormatCodeSettings) {
2604+
public verifyApplicableRefactorAvailableForRange(negative: boolean) {
25962605
const ranges = this.getRanges();
2597-
if (ranges.length > 1) {
2598-
throw new Error("only one refactor range is allowed per test");
2599-
}
2600-
2601-
const range = ranges[0] ? { pos: ranges[0].start, end: ranges[0].end } : undefined;
2602-
const actions = this.getRefactorActions(this.activeFile.fileName, range, formattingOptions);
2603-
2604-
// Each refactor diagnostic will return one code action, but multiple refactor diagnostics can point to the same
2605-
// code action. For example in the case of "convert function to es6 class":
2606-
//
2607-
// function foo() { }
2608-
// ^^^
2609-
// foo.prototype.getName = function () { return "name"; };
2610-
// ^^^
2611-
// These two diagnostics result in the same code action, so we only apply the first one.
2612-
this.applyCodeAction(this.activeFile.fileName, actions, /*index*/ 0);
2606+
if (!(ranges && ranges.length === 1)) {
2607+
throw new Error("Exactly one refactor range is allowed per test.");
2608+
}
2609+
2610+
const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, { pos: ranges[0].start, end: ranges[0].end });
2611+
const isAvailable = applicableRefactors && applicableRefactors.length > 0;
2612+
if (negative && isAvailable) {
2613+
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some.`);
2614+
}
2615+
if (!negative && !isAvailable) {
2616+
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`);
2617+
}
2618+
}
2619+
2620+
public verifyFileAfterApplyingRefactorAtMarker(
2621+
markerName: string,
2622+
expectedContent: string,
2623+
refactorKindToApply?: ts.RefactorKind,
2624+
formattingOptions?: ts.FormatCodeSettings) {
2625+
2626+
formattingOptions = formattingOptions || this.formatCodeSettings;
2627+
const markerPos = this.getMarkerByName(markerName).position;
2628+
const diagnostics = this.getRefactorDiagnostics(this.activeFile.fileName);
2629+
2630+
const diagnosticCodesAtMarker: number[] = [];
2631+
for (const diagnostic of diagnostics) {
2632+
if (diagnostic.start <= markerPos && diagnostic.start + diagnostic.length >= markerPos) {
2633+
diagnosticCodesAtMarker.push(diagnostic.code);
2634+
}
2635+
}
2636+
2637+
const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos);
2638+
const applicableRefactorKinds =
2639+
ts.map(ts.filter(applicableRefactors, ar => refactorKindToApply === undefined || refactorKindToApply === ar.refactorKind),
2640+
refactorInfo => refactorInfo.refactorKind);
2641+
const codeActions = this.languageService.getRefactorCodeActions(
2642+
this.activeFile.fileName, formattingOptions, markerPos, applicableRefactorKinds, diagnosticCodesAtMarker);
2643+
2644+
this.applyAllCodeActions(this.activeFile.fileName, codeActions);
26132645
const actualContent = this.getFileContent(this.activeFile.fileName);
26142646

26152647
if (this.normalizeNewlines(actualContent) !== this.normalizeNewlines(expectedContent)) {
@@ -3409,8 +3441,16 @@ namespace FourSlashInterface {
34093441
this.state.verifyCodeFixAvailable(this.negative);
34103442
}
34113443

3412-
public refactorAvailable() {
3413-
this.state.verifyRefactorAvailable(this.negative);
3444+
public refactorDiagnosticsAvailableAtMarker(markerName: string, refactorCode?: number) {
3445+
this.state.verifyRefactorDiagnosticsAvailableAtMarker(this.negative, markerName, refactorCode);
3446+
}
3447+
3448+
public applicableRefactorAvailableAtMarker(markerName: string) {
3449+
this.state.verifyApplicableRefactorAvailableAtMarker(this.negative, markerName);
3450+
}
3451+
3452+
public applicableRefactorAvailableForRange(){
3453+
this.state.verifyApplicableRefactorAvailableForRange(this.negative);
34143454
}
34153455
}
34163456

@@ -3618,8 +3658,8 @@ namespace FourSlashInterface {
36183658
this.state.verifyRangeAfterCodeFix(expectedText, includeWhiteSpace, errorCode, index);
36193659
}
36203660

3621-
public fileAfterApplyingRefactors(expectedContent: string, formattingOptions: ts.FormatCodeSettings): void {
3622-
this.state.verifyFileAfterApplyingRefactors(expectedContent, formattingOptions);
3661+
public fileAfterApplyingRefactorsAtMarker(markerName: string, expectedContent: string, refactorKindToApply?: ts.RefactorKind, formattingOptions?: ts.FormatCodeSettings): void {
3662+
this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorKindToApply, formattingOptions);
36233663
}
36243664

36253665
public importFixAtPosition(expectedTextArray: string[], errorCode?: number): void {

src/harness/harnessLanguageService.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -489,10 +489,13 @@ namespace Harness.LanguageService {
489489
getCodeFixesAtPosition(): ts.CodeAction[] {
490490
throw new Error("Not supported on the shim.");
491491
}
492-
getRefactorDiagnostics(): ts.RefactorDiagnostic[] {
492+
getRefactorDiagnostics(): ts.Diagnostic[] {
493493
throw new Error("Not supported on the shim.");
494494
}
495-
getCodeActionsForRefactorAtPosition(): ts.CodeAction[] {
495+
getRefactorCodeActions(): ts.CodeAction[] {
496+
throw new Error("Not supported on the shim.");
497+
}
498+
getApplicableRefactors(): ts.ApplicableRefactorInfo[] {
496499
throw new Error("Not supported on the shim.");
497500
}
498501
getEmitOutput(fileName: string): ts.EmitOutput {

src/server/client.ts

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -692,48 +692,53 @@ namespace ts.server {
692692
return response.body.map(entry => this.convertCodeActions(entry, fileName));
693693
}
694694

695-
getRefactorDiagnostics(fileName: string, range: TextRange): RefactorDiagnostic[] {
696-
const startLineOffset = this.positionToOneBasedLineOffset(fileName, range.pos);
697-
const endLineOffset = this.positionToOneBasedLineOffset(fileName, range.end);
695+
getRefactorDiagnostics(_fileName: string): Diagnostic[] {
696+
return notImplemented();
697+
}
698698

699-
const args: protocol.GetRefactorsForRangeRequestArgs = {
699+
private positionOrRangeToLocationOrSpan(positionOrRange: number | TextRange, fileName: string) {
700+
let locationOrSpan: protocol.LocationOrSpanWithPosition;
701+
if (typeof positionOrRange === "number") {
702+
locationOrSpan = this.positionToOneBasedLineOffset(fileName, positionOrRange);
703+
}
704+
else {
705+
locationOrSpan = positionOrRange
706+
? { start: this.positionToOneBasedLineOffset(fileName, positionOrRange.pos), end: this.positionToOneBasedLineOffset(fileName, positionOrRange.end) }
707+
: undefined;
708+
}
709+
return locationOrSpan;
710+
}
711+
712+
getApplicableRefactors(fileName: string, positionOrRange: number | TextRange): ApplicableRefactorInfo[] {
713+
const args: protocol.Refactor.GetApplicableRefactorsRequestArgs = {
700714
file: fileName,
701-
startLine: startLineOffset.line,
702-
startOffset: startLineOffset.offset,
703-
endLine: endLineOffset.line,
704-
endOffset: endLineOffset.offset,
715+
locationOrSpan: this.positionOrRangeToLocationOrSpan(positionOrRange, fileName)
705716
};
706717

707-
const request = this.processRequest<protocol.GetRefactorsForRangeRequest>(CommandNames.GetRefactorsForRange, args);
708-
const response = this.processResponse<protocol.GetRefactorsForRangeResponse>(request);
718+
const request = this.processRequest<protocol.Refactor.GetApplicableRefactorsRequest>(CommandNames.GetApplicableRefactors, args);
719+
const response = this.processResponse<protocol.Refactor.GetApplicableRefactorsResponse>(request);
709720

710-
return response.body.map(entry => {
711-
return <RefactorDiagnostic>{
712-
code: entry.code,
713-
end: this.lineOffsetToPosition(fileName, entry.end),
714-
start: this.lineOffsetToPosition(fileName, entry.start),
715-
text: entry.text
716-
};
717-
});
721+
return response.body;
718722
}
719723

720-
getCodeActionsForRefactorAtPosition(fileName: string, range: TextRange, refactorCode: number): CodeAction[] {
721-
const startLineOffset = this.positionToOneBasedLineOffset(fileName, range.pos);
722-
const endLineOffset = this.positionToOneBasedLineOffset(fileName, range.end);
724+
getRefactorCodeActions(
725+
fileName: string,
726+
_formatOptions: FormatCodeSettings,
727+
positionOrRange: number | TextRange,
728+
refactorKinds?: RefactorKind[],
729+
diagnosticCodes?: number[]) {
723730

724-
const args: protocol.GetCodeActionsForRefactorRequestArgs = {
731+
const args: protocol.Refactor.GetRefactorCodeActionsRequestArgs = {
725732
file: fileName,
726-
startLine: startLineOffset.line,
727-
startOffset: startLineOffset.offset,
728-
endLine: endLineOffset.line,
729-
endOffset: endLineOffset.offset,
730-
refactorCode
733+
locationOrSpan: this.positionOrRangeToLocationOrSpan(positionOrRange, fileName),
734+
refactorKinds,
735+
diagnosticCodes
731736
};
732737

733-
const request = this.processRequest<protocol.GetCodeActionsForRefactorRequest>(CommandNames.GetCodeActionsForRefactor, args);
734-
const response = this.processResponse<protocol.GetCodeActionsForRefactorResponse>(request);
738+
const request = this.processRequest<protocol.Refactor.GetRefactorCodeActionsRequest>(CommandNames.GetRefactorCodeActions, args);
739+
const codeActions = this.processResponse<protocol.Refactor.GetRefactorCodeActionsResponse>(request).body;
735740

736-
return response.body.map(entry => this.convertCodeActions(entry, fileName));
741+
return map(codeActions, codeAction => this.convertCodeActions(codeAction, fileName));
737742
}
738743

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

src/server/protocol.ts

Lines changed: 45 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,8 @@ namespace ts.server.protocol {
9696
export type GetCodeFixesFull = "getCodeFixes-full";
9797
export type GetSupportedCodeFixes = "getSupportedCodeFixes";
9898

99-
export type GetRefactorsForRange = "getRefactorsForRange";
100-
export type GetCodeActionsForRefactor = "getCodeActionsForRefactor";
99+
export type GetApplicableRefactors = "getApplicableRefactors";
100+
export type GetRefactorCodeActions = "getRefactorCodeActions";
101101
}
102102

103103
/**
@@ -397,53 +397,54 @@ namespace ts.server.protocol {
397397
position?: number;
398398
}
399399

400-
/**
401-
* An diagnostic information suggesting refactors at applicable positions without
402-
* clients asking.
403-
*/
404-
export interface RefactorDiagnostic {
405-
text: string;
406-
code: number;
407-
start: Location;
408-
end: Location;
409-
}
410-
411-
export interface RefactorDiagnosticEventBody {
412-
file: string;
413-
diagnostics: RefactorDiagnostic[];
414-
}
415-
416-
/**
417-
* Returns a list of applicable refactors at a given position. This request does not actually
418-
* compute the refactors; instead it goes through faster checks to determine what is possible.
419-
*/
420-
export interface GetRefactorsForRangeRequest extends Request {
421-
command: CommandTypes.GetRefactorsForRange;
422-
arguments: GetRefactorsForRangeRequestArgs;
423-
}
400+
export type LocationOrSpanWithPosition = LocationWithPosition | { start: LocationWithPosition, end: LocationWithPosition };
424401

425-
export interface GetRefactorsForRangeRequestArgs extends FileRangeRequestArgs {
402+
export interface FileLocationOrSpanWithPositionRequestArgs extends FileRequestArgs {
403+
locationOrSpan: LocationOrSpanWithPosition;
426404
}
427405

428-
export interface GetRefactorsForRangeResponse extends Response {
429-
body?: RefactorDiagnostic[];
430-
}
431-
432-
/**
433-
* Computes the code actions for a given refactor. This is normally called after the user commited one
434-
* of the applicable refactors provided by the "GetApplicableRefactors" API.
435-
*/
436-
export interface GetCodeActionsForRefactorRequest extends Request {
437-
command: CommandTypes.GetCodeActionsForRefactor;
438-
arguments: GetCodeActionsForRefactorRequestArgs;
439-
}
440-
441-
export interface GetCodeActionsForRefactorRequestArgs extends GetRefactorsForRangeRequestArgs {
442-
refactorCode: number;
406+
export interface LocationWithPosition extends Location {
407+
position?: number;
443408
}
444409

445-
export interface GetCodeActionsForRefactorResponse extends Response {
446-
body?: CodeAction[];
410+
export namespace Refactor {
411+
export interface GetApplicableRefactorsRequest extends Request {
412+
command: CommandTypes.GetApplicableRefactors;
413+
arguments: GetApplicableRefactorsRequestArgs;
414+
}
415+
416+
export interface GetApplicableRefactorsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs {
417+
}
418+
419+
export interface ApplicableRefactorInfo {
420+
refactorKind: number;
421+
description: string;
422+
}
423+
424+
export interface GetApplicableRefactorsResponse extends Response {
425+
body?: ApplicableRefactorInfo[];
426+
}
427+
428+
export interface GetRefactorCodeActionsRequest extends Request {
429+
command: CommandTypes.GetRefactorCodeActions;
430+
arguments: GetRefactorCodeActionsRequestArgs;
431+
}
432+
433+
export interface GetRefactorCodeActionsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs {
434+
/* The kind of the applicable refactor */
435+
refactorKinds?: number[];
436+
/* The diagnostic code of a refactor diagnostic */
437+
diagnosticCodes?: number[];
438+
}
439+
440+
export interface GetRefactorCodeActionsResponse extends Response {
441+
body?: CodeAction[];
442+
}
443+
444+
export interface RefactorDiagnosticEventBody {
445+
file: string;
446+
diagnostics: Diagnostic[];
447+
}
447448
}
448449

449450
/**

0 commit comments

Comments
 (0)