Skip to content

Commit 3644771

Browse files
author
Andy
authored
Test for action description of code actions, and simplify description for extracting method to file (#18030) (#18044)
* 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 a39ae1f commit 3644771

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
@@ -3688,7 +3688,7 @@
36883688
"code": 95003
36893689
},
36903690

3691-
"Extract function into '{0}'": {
3691+
"Extract function into {0}": {
36923692
"category": "Message",
36933693
"code": 95004
36943694
}

‎src/harness/fourslash.ts

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2743,20 +2743,25 @@ namespace FourSlash {
27432743
});
27442744
}
27452745

2746-
public verifyRefactorAvailable(negative: boolean, name?: string, subName?: string) {
2746+
public verifyRefactorAvailable(negative: boolean, name: string, actionName?: string) {
27472747
const selection = this.getSelection();
27482748

27492749
let refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, selection) || [];
2750-
if (name) {
2751-
refactors = refactors.filter(r => r.name === name && (subName === undefined || r.actions.some(a => a.name === subName)));
2752-
}
2750+
refactors = refactors.filter(r => r.name === name && (actionName === undefined || r.actions.some(a => a.name === actionName)));
27532751
const isAvailable = refactors.length > 0;
27542752

2755-
if (negative && isAvailable) {
2756-
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some: ${refactors.map(r => r.name).join(", ")}`);
2753+
if (negative) {
2754+
if (isAvailable) {
2755+
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found: ${refactors.map(r => r.name).join(", ")}`);
2756+
}
27572757
}
2758-
else if (!negative && !isAvailable) {
2759-
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`);
2758+
else {
2759+
if (!isAvailable) {
2760+
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`);
2761+
}
2762+
if (refactors.length > 1) {
2763+
this.raiseError(`${refactors.length} available refactors both have name ${name} and action ${actionName}`);
2764+
}
27602765
}
27612766
}
27622767

@@ -2776,14 +2781,22 @@ namespace FourSlash {
27762781
}
27772782
}
27782783

2779-
public applyRefactor(refactorName: string, actionName: string) {
2784+
public applyRefactor({ refactorName, actionName, actionDescription }: FourSlashInterface.ApplyRefactorOptions) {
27802785
const range = this.getSelection();
27812786
const refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, range);
2782-
const refactor = ts.find(refactors, r => r.name === refactorName);
2787+
const refactor = refactors.find(r => r.name === refactorName);
27832788
if (!refactor) {
27842789
this.raiseError(`The expected refactor: ${refactorName} is not available at the marker location.`);
27852790
}
27862791

2792+
const action = refactor.actions.find(a => a.name === actionName);
2793+
if (!action) {
2794+
this.raiseError(`The expected action: ${action} is not included in: ${refactor.actions.map(a => a.name)}`);
2795+
}
2796+
if (action.description !== actionDescription) {
2797+
this.raiseError(`Expected action description to be ${JSON.stringify(actionDescription)}, got: ${JSON.stringify(action.description)}`);
2798+
}
2799+
27872800
const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactorName, actionName);
27882801
for (const edit of editInfo.edits) {
27892802
this.applyEdits(edit.fileName, edit.textChanges, /*isFormattingEdit*/ false);
@@ -3660,8 +3673,8 @@ namespace FourSlashInterface {
36603673
this.state.verifyApplicableRefactorAvailableForRange(this.negative);
36613674
}
36623675

3663-
public refactorAvailable(name?: string, subName?: string) {
3664-
this.state.verifyRefactorAvailable(this.negative, name, subName);
3676+
public refactorAvailable(name: string, actionName?: string) {
3677+
this.state.verifyRefactorAvailable(this.negative, name, actionName);
36653678
}
36663679
}
36673680

@@ -4059,8 +4072,8 @@ namespace FourSlashInterface {
40594072
this.state.enableFormatting = false;
40604073
}
40614074

4062-
public applyRefactor(refactorName: string, actionName: string) {
4063-
this.state.applyRefactor(refactorName, actionName);
4075+
public applyRefactor(options: ApplyRefactorOptions) {
4076+
this.state.applyRefactor(options);
40644077
}
40654078
}
40664079

@@ -4273,4 +4286,10 @@ namespace FourSlashInterface {
42734286
return { classificationType, text, textSpan };
42744287
}
42754288
}
4289+
4290+
export interface ApplyRefactorOptions {
4291+
refactorName: string;
4292+
actionName: string;
4293+
actionDescription: string;
4294+
}
42764295
}

‎src/harness/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@
123123
"./unittests/printer.ts",
124124
"./unittests/transform.ts",
125125
"./unittests/customTransforms.ts",
126+
"./unittests/extractMethods.ts",
126127
"./unittests/textChanges.ts",
127128
"./unittests/telemetry.ts",
128129
"./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)