Skip to content

Commit a7e7388

Browse files
author
Andy Hanson
committed
Test for action description of code actions, and simplify description for extracting method to file
1 parent 038d256 commit a7e7388

18 files changed

+118
-47
lines changed

‎src/compiler/diagnosticMessages.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3696,7 +3696,7 @@
36963696
"code": 95003
36973697
},
36983698

3699-
"Extract function into '{0}'": {
3699+
"Extract function into {0}": {
37003700
"category": "Message",
37013701
"code": 95004
37023702
}

‎src/harness/fourslash.ts

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2761,20 +2761,25 @@ namespace FourSlash {
27612761
});
27622762
}
27632763

2764-
public verifyRefactorAvailable(negative: boolean, name?: string, subName?: string) {
2764+
public verifyRefactorAvailable(negative: boolean, name: string, actionName?: string) {
27652765
const selection = this.getSelection();
27662766

27672767
let refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, selection) || [];
2768-
if (name) {
2769-
refactors = refactors.filter(r => r.name === name && (subName === undefined || r.actions.some(a => a.name === subName)));
2770-
}
2768+
refactors = refactors.filter(r => r.name === name && (actionName === undefined || r.actions.some(a => a.name === actionName)));
27712769
const isAvailable = refactors.length > 0;
27722770

2773-
if (negative && isAvailable) {
2774-
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some: ${refactors.map(r => r.name).join(", ")}`);
2771+
if (negative) {
2772+
if (isAvailable) {
2773+
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found: ${refactors.map(r => r.name).join(", ")}`);
2774+
}
27752775
}
2776-
else if (!negative && !isAvailable) {
2777-
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`);
2776+
else {
2777+
if (!isAvailable) {
2778+
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`);
2779+
}
2780+
if (refactors.length > 1) {
2781+
this.raiseError(`2 available refactors both have name ${name} and action ${actionName}`);
2782+
}
27782783
}
27792784
}
27802785

@@ -2794,14 +2799,22 @@ namespace FourSlash {
27942799
}
27952800
}
27962801

2797-
public applyRefactor(refactorName: string, actionName: string) {
2802+
public applyRefactor({ refactorName, actionName, actionDescription }: FourSlashInterface.ApplyRefactorOptions) {
27982803
const range = this.getSelection();
27992804
const refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, range);
2800-
const refactor = ts.find(refactors, r => r.name === refactorName);
2805+
const refactor = refactors.find(r => r.name === refactorName);
28012806
if (!refactor) {
28022807
this.raiseError(`The expected refactor: ${refactorName} is not available at the marker location.`);
28032808
}
28042809

2810+
const action = refactor.actions.find(a => a.name === actionName);
2811+
if (!action) {
2812+
this.raiseError(`The expected action: ${action} is not included in: ${refactor.actions.map(a => a.name)}`);
2813+
}
2814+
if (action.description !== actionDescription) {
2815+
this.raiseError(`Expected action description to be ${JSON.stringify(actionDescription)}, got: ${JSON.stringify(action.description)}`);
2816+
}
2817+
28052818
const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactorName, actionName);
28062819
for (const edit of editInfo.edits) {
28072820
this.applyEdits(edit.fileName, edit.textChanges, /*isFormattingEdit*/ false);
@@ -3682,8 +3695,8 @@ namespace FourSlashInterface {
36823695
this.state.verifyApplicableRefactorAvailableForRange(this.negative);
36833696
}
36843697

3685-
public refactorAvailable(name?: string, subName?: string) {
3686-
this.state.verifyRefactorAvailable(this.negative, name, subName);
3698+
public refactorAvailable(name: string, actionName?: string) {
3699+
this.state.verifyRefactorAvailable(this.negative, name, actionName);
36873700
}
36883701
}
36893702

@@ -4081,8 +4094,8 @@ namespace FourSlashInterface {
40814094
this.state.enableFormatting = false;
40824095
}
40834096

4084-
public applyRefactor(refactorName: string, actionName: string) {
4085-
this.state.applyRefactor(refactorName, actionName);
4097+
public applyRefactor(options: ApplyRefactorOptions) {
4098+
this.state.applyRefactor(options);
40864099
}
40874100
}
40884101

@@ -4295,4 +4308,10 @@ namespace FourSlashInterface {
42954308
return { classificationType, text, textSpan };
42964309
}
42974310
}
4311+
4312+
export interface ApplyRefactorOptions {
4313+
refactorName: string;
4314+
actionName: string;
4315+
actionDescription: string;
4316+
}
42984317
}

‎src/services/refactors/extractMethod.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -560,32 +560,32 @@ namespace ts.refactor.extractMethod {
560560
return "constructor";
561561
case SyntaxKind.FunctionExpression:
562562
return scope.name
563-
? `function expression ${scope.name.getText()}`
563+
? `function expression ${scope.name.text}`
564564
: "anonymous function expression";
565565
case SyntaxKind.FunctionDeclaration:
566-
return `function ${scope.name.getText()}`;
566+
return `function '${scope.name.text}'`;
567567
case SyntaxKind.ArrowFunction:
568568
return "arrow function";
569569
case SyntaxKind.MethodDeclaration:
570-
return `method ${scope.name.getText()}`;
570+
return `method '${scope.name.getText()}`;
571571
case SyntaxKind.GetAccessor:
572-
return `get ${scope.name.getText()}`;
572+
return `'get ${scope.name.getText()}'`;
573573
case SyntaxKind.SetAccessor:
574-
return `set ${scope.name.getText()}`;
574+
return `'set ${scope.name.getText()}'`;
575575
}
576576
}
577577
else if (isModuleBlock(scope)) {
578-
return `namespace ${scope.parent.name.getText()}`;
578+
return `namespace '${scope.parent.name.getText()}'`;
579579
}
580580
else if (isClassLike(scope)) {
581581
return scope.kind === SyntaxKind.ClassDeclaration
582-
? `class ${scope.name.text}`
582+
? `class '${scope.name.text}'`
583583
: scope.name.text
584-
? `class expression ${scope.name.text}`
584+
? `class expression '${scope.name.text}'`
585585
: "anonymous class expression";
586586
}
587587
else if (isSourceFile(scope)) {
588-
return `file '${scope.fileName}'`;
588+
return "this file";
589589
}
590590
else {
591591
return "unknown";

‎tests/cases/fourslash/extract-method1.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@
1313
//// }
1414

1515
goTo.select('start', 'end')
16-
verify.refactorAvailable('Extract Method');
17-
edit.applyRefactor('Extract Method', "scope_0");
16+
edit.applyRefactor({
17+
refactorName: "Extract Method",
18+
actionName: "scope_0",
19+
actionDescription: "Extract function into class 'Foo'",
20+
});
1821
verify.currentFileContentIs(
1922
`class Foo {
2023
someMethod(m: number) {

‎tests/cases/fourslash/extract-method10.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
/// <reference path='fourslash.ts' />
22

3-
//// (x => x)(/*1*/x => x/*2*/)(1);
3+
//// (x => x)(/*1*/x => x/*2*/)(1);
44

55
goTo.select('1', '2');
6-
edit.applyRefactor('Extract Method', 'scope_0');
6+
edit.applyRefactor({
7+
refactorName: "Extract Method",
8+
actionName: 'scope_0',
9+
actionDescription: "Extract function into this file",
10+
});

‎tests/cases/fourslash/extract-method13.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,18 @@
1010
//// }
1111

1212
goTo.select('a', 'b');
13-
edit.applyRefactor('Extract Method', 'scope_0');
13+
edit.applyRefactor({
14+
refactorName: "Extract Method",
15+
actionName: "scope_0",
16+
actionDescription: "Extract function into class 'C'",
17+
});
1418

1519
goTo.select('c', 'd');
16-
edit.applyRefactor('Extract Method', 'scope_0');
20+
edit.applyRefactor({
21+
refactorName: "Extract Method",
22+
actionName: "scope_0",
23+
actionDescription: "Extract function into class 'C'",
24+
});
1725

1826
verify.currentFileContentIs(`class C {
1927
static j = C.newFunction_1();

‎tests/cases/fourslash/extract-method14.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@
1111
//// }
1212

1313
goTo.select('a', 'b');
14-
edit.applyRefactor('Extract Method', 'scope_1');
14+
edit.applyRefactor({
15+
refactorName: "Extract Method",
16+
actionName: "scope_1",
17+
actionDescription: "Extract function into this file",
18+
});
1519
verify.currentFileContentIs(`function foo() {
1620
var i = 10;
1721
var __return: any;

‎tests/cases/fourslash/extract-method15.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99
//// }
1010

1111
goTo.select('a', 'b');
12-
edit.applyRefactor('Extract Method', 'scope_1');
12+
edit.applyRefactor({
13+
refactorName: "Extract Method",
14+
actionName: "scope_1",
15+
actionDescription: "Extract function into this file",
16+
});
1317

1418
verify.currentFileContentIs(`function foo() {
1519
var i = 10;

‎tests/cases/fourslash/extract-method18.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@
99
//// }
1010

1111
goTo.select('a', 'b')
12-
verify.refactorAvailable('Extract Method');
13-
edit.applyRefactor('Extract Method', "scope_1");
12+
edit.applyRefactor({
13+
refactorName: "Extract Method",
14+
actionName: "scope_1",
15+
actionDescription: "Extract function into this file",
16+
});
1417
verify.currentFileContentIs(`function fn() {
1518
const x = { m: 1 };
1619
newFunction(x);

‎tests/cases/fourslash/extract-method19.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,15 @@
55
//// function fn() {
66
//// /*a*/console.log("hi");/*b*/
77
//// }
8-
////
8+
////
99
//// function newFunction() { }
1010

1111
goTo.select('a', 'b')
12-
verify.refactorAvailable('Extract Method');
13-
edit.applyRefactor('Extract Method', "scope_0");
12+
edit.applyRefactor({
13+
refactorName: "Extract Method",
14+
actionName: "scope_0",
15+
actionDescription: "Extract function into function 'fn'",
16+
});
1417
verify.currentFileContentIs(`function fn() {
1518
newFunction_1();
1619

0 commit comments

Comments
 (0)