Skip to content

Commit 3a0ab74

Browse files
author
Andy
authored
Test for action description of code actions, and simplify description for extracting method to file (microsoft#18030)
* Test for action description of code actions, and simplify description for extracting method to file * Add unit test file missing from tsconfig.json (only affects gulp) and update tests * Use the actual number * Use "module scope" or "global scope" instead of "this file"
1 parent 62eaaf9 commit 3a0ab74

31 files changed

+163
-90
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(`${refactors.length} 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/harness/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@
125125
"./unittests/printer.ts",
126126
"./unittests/transform.ts",
127127
"./unittests/customTransforms.ts",
128+
"./unittests/extractMethods.ts",
128129
"./unittests/textChanges.ts",
129130
"./unittests/telemetry.ts",
130131
"./unittests/programMissingFiles.ts"

‎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 scope.externalModuleIndicator ? "module scope" : "global scope";
589589
}
590590
else {
591591
return "unknown";

‎tests/baselines/reference/extractMethod/extractMethod1.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace A {
1414
}
1515
}
1616
}
17-
==SCOPE::function a==
17+
==SCOPE::function 'a'==
1818
namespace A {
1919
let x = 1;
2020
function foo() {
@@ -34,7 +34,7 @@ namespace A {
3434
}
3535
}
3636
}
37-
==SCOPE::namespace B==
37+
==SCOPE::namespace 'B'==
3838
namespace A {
3939
let x = 1;
4040
function foo() {
@@ -55,7 +55,7 @@ namespace A {
5555
}
5656
}
5757
}
58-
==SCOPE::namespace A==
58+
==SCOPE::namespace 'A'==
5959
namespace A {
6060
let x = 1;
6161
function foo() {
@@ -76,7 +76,7 @@ namespace A {
7676
return a;
7777
}
7878
}
79-
==SCOPE::file '/a.ts'==
79+
==SCOPE::global scope==
8080
namespace A {
8181
let x = 1;
8282
function foo() {

‎tests/baselines/reference/extractMethod/extractMethod10.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace A {
99
}
1010
}
1111
}
12-
==SCOPE::class C==
12+
==SCOPE::class 'C'==
1313
namespace A {
1414
export interface I { x: number };
1515
class C {
@@ -24,7 +24,7 @@ namespace A {
2424
}
2525
}
2626
}
27-
==SCOPE::namespace A==
27+
==SCOPE::namespace 'A'==
2828
namespace A {
2929
export interface I { x: number };
3030
class C {
@@ -39,7 +39,7 @@ namespace A {
3939
return a1.x + 10;
4040
}
4141
}
42-
==SCOPE::file '/a.ts'==
42+
==SCOPE::global scope==
4343
namespace A {
4444
export interface I { x: number };
4545
class C {

‎tests/baselines/reference/extractMethod/extractMethod11.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace A {
1111
}
1212
}
1313
}
14-
==SCOPE::class C==
14+
==SCOPE::class 'C'==
1515
namespace A {
1616
let y = 1;
1717
class C {
@@ -30,7 +30,7 @@ namespace A {
3030
}
3131
}
3232
}
33-
==SCOPE::namespace A==
33+
==SCOPE::namespace 'A'==
3434
namespace A {
3535
let y = 1;
3636
class C {
@@ -49,7 +49,7 @@ namespace A {
4949
return { __return: a1.x + 10, z };
5050
}
5151
}
52-
==SCOPE::file '/a.ts'==
52+
==SCOPE::global scope==
5353
namespace A {
5454
let y = 1;
5555
class C {

‎tests/baselines/reference/extractMethod/extractMethod12.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace A {
1313
}
1414
}
1515
}
16-
==SCOPE::class C==
16+
==SCOPE::class 'C'==
1717
namespace A {
1818
let y = 1;
1919
class C {

‎tests/baselines/reference/extractMethod/extractMethod2.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ namespace A {
1212
}
1313
}
1414
}
15-
==SCOPE::function a==
15+
==SCOPE::function 'a'==
1616
namespace A {
1717
let x = 1;
1818
function foo() {
@@ -30,7 +30,7 @@ namespace A {
3030
}
3131
}
3232
}
33-
==SCOPE::namespace B==
33+
==SCOPE::namespace 'B'==
3434
namespace A {
3535
let x = 1;
3636
function foo() {
@@ -48,7 +48,7 @@ namespace A {
4848
}
4949
}
5050
}
51-
==SCOPE::namespace A==
51+
==SCOPE::namespace 'A'==
5252
namespace A {
5353
let x = 1;
5454
function foo() {
@@ -66,7 +66,7 @@ namespace A {
6666
return foo();
6767
}
6868
}
69-
==SCOPE::file '/a.ts'==
69+
==SCOPE::global scope==
7070
namespace A {
7171
let x = 1;
7272
function foo() {

‎tests/baselines/reference/extractMethod/extractMethod3.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace A {
1111
}
1212
}
1313
}
14-
==SCOPE::function a==
14+
==SCOPE::function 'a'==
1515
namespace A {
1616
function foo() {
1717
}
@@ -28,7 +28,7 @@ namespace A {
2828
}
2929
}
3030
}
31-
==SCOPE::namespace B==
31+
==SCOPE::namespace 'B'==
3232
namespace A {
3333
function foo() {
3434
}
@@ -45,7 +45,7 @@ namespace A {
4545
}
4646
}
4747
}
48-
==SCOPE::namespace A==
48+
==SCOPE::namespace 'A'==
4949
namespace A {
5050
function foo() {
5151
}
@@ -62,7 +62,7 @@ namespace A {
6262
return foo();
6363
}
6464
}
65-
==SCOPE::file '/a.ts'==
65+
==SCOPE::global scope==
6666
namespace A {
6767
function foo() {
6868
}

‎tests/baselines/reference/extractMethod/extractMethod4.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace A {
1313
}
1414
}
1515
}
16-
==SCOPE::function a==
16+
==SCOPE::function 'a'==
1717
namespace A {
1818
function foo() {
1919
}
@@ -32,7 +32,7 @@ namespace A {
3232
}
3333
}
3434
}
35-
==SCOPE::namespace B==
35+
==SCOPE::namespace 'B'==
3636
namespace A {
3737
function foo() {
3838
}
@@ -51,7 +51,7 @@ namespace A {
5151
}
5252
}
5353
}
54-
==SCOPE::namespace A==
54+
==SCOPE::namespace 'A'==
5555
namespace A {
5656
function foo() {
5757
}
@@ -70,7 +70,7 @@ namespace A {
7070
return foo();
7171
}
7272
}
73-
==SCOPE::file '/a.ts'==
73+
==SCOPE::global scope==
7474
namespace A {
7575
function foo() {
7676
}

‎tests/baselines/reference/extractMethod/extractMethod5.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace A {
1414
}
1515
}
1616
}
17-
==SCOPE::function a==
17+
==SCOPE::function 'a'==
1818
namespace A {
1919
let x = 1;
2020
export function foo() {
@@ -34,7 +34,7 @@ namespace A {
3434
}
3535
}
3636
}
37-
==SCOPE::namespace B==
37+
==SCOPE::namespace 'B'==
3838
namespace A {
3939
let x = 1;
4040
export function foo() {
@@ -55,7 +55,7 @@ namespace A {
5555
}
5656
}
5757
}
58-
==SCOPE::namespace A==
58+
==SCOPE::namespace 'A'==
5959
namespace A {
6060
let x = 1;
6161
export function foo() {
@@ -76,7 +76,7 @@ namespace A {
7676
return a;
7777
}
7878
}
79-
==SCOPE::file '/a.ts'==
79+
==SCOPE::global scope==
8080
namespace A {
8181
let x = 1;
8282
export function foo() {

0 commit comments

Comments
 (0)