Test for action description of code actions, and simplify description for extracting method to file (#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"
This commit is contained in:
Andy
2017-08-25 09:53:28 -07:00
committed by GitHub
parent 62eaaf9206
commit 3a0ab74ed6
31 changed files with 163 additions and 90 deletions
+1 -1
View File
@@ -3696,7 +3696,7 @@
"code": 95003
},
"Extract function into '{0}'": {
"Extract function into {0}": {
"category": "Message",
"code": 95004
}
+33 -14
View File
@@ -2761,20 +2761,25 @@ namespace FourSlash {
});
}
public verifyRefactorAvailable(negative: boolean, name?: string, subName?: string) {
public verifyRefactorAvailable(negative: boolean, name: string, actionName?: string) {
const selection = this.getSelection();
let refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, selection) || [];
if (name) {
refactors = refactors.filter(r => r.name === name && (subName === undefined || r.actions.some(a => a.name === subName)));
}
refactors = refactors.filter(r => r.name === name && (actionName === undefined || r.actions.some(a => a.name === actionName)));
const isAvailable = refactors.length > 0;
if (negative && isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some: ${refactors.map(r => r.name).join(", ")}`);
if (negative) {
if (isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found: ${refactors.map(r => r.name).join(", ")}`);
}
}
else if (!negative && !isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`);
else {
if (!isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`);
}
if (refactors.length > 1) {
this.raiseError(`${refactors.length} available refactors both have name ${name} and action ${actionName}`);
}
}
}
@@ -2794,14 +2799,22 @@ namespace FourSlash {
}
}
public applyRefactor(refactorName: string, actionName: string) {
public applyRefactor({ refactorName, actionName, actionDescription }: FourSlashInterface.ApplyRefactorOptions) {
const range = this.getSelection();
const refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, range);
const refactor = ts.find(refactors, r => r.name === refactorName);
const refactor = refactors.find(r => r.name === refactorName);
if (!refactor) {
this.raiseError(`The expected refactor: ${refactorName} is not available at the marker location.`);
}
const action = refactor.actions.find(a => a.name === actionName);
if (!action) {
this.raiseError(`The expected action: ${action} is not included in: ${refactor.actions.map(a => a.name)}`);
}
if (action.description !== actionDescription) {
this.raiseError(`Expected action description to be ${JSON.stringify(actionDescription)}, got: ${JSON.stringify(action.description)}`);
}
const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactorName, actionName);
for (const edit of editInfo.edits) {
this.applyEdits(edit.fileName, edit.textChanges, /*isFormattingEdit*/ false);
@@ -3682,8 +3695,8 @@ namespace FourSlashInterface {
this.state.verifyApplicableRefactorAvailableForRange(this.negative);
}
public refactorAvailable(name?: string, subName?: string) {
this.state.verifyRefactorAvailable(this.negative, name, subName);
public refactorAvailable(name: string, actionName?: string) {
this.state.verifyRefactorAvailable(this.negative, name, actionName);
}
}
@@ -4081,8 +4094,8 @@ namespace FourSlashInterface {
this.state.enableFormatting = false;
}
public applyRefactor(refactorName: string, actionName: string) {
this.state.applyRefactor(refactorName, actionName);
public applyRefactor(options: ApplyRefactorOptions) {
this.state.applyRefactor(options);
}
}
@@ -4295,4 +4308,10 @@ namespace FourSlashInterface {
return { classificationType, text, textSpan };
}
}
export interface ApplyRefactorOptions {
refactorName: string;
actionName: string;
actionDescription: string;
}
}
+1
View File
@@ -125,6 +125,7 @@
"./unittests/printer.ts",
"./unittests/transform.ts",
"./unittests/customTransforms.ts",
"./unittests/extractMethods.ts",
"./unittests/textChanges.ts",
"./unittests/telemetry.ts",
"./unittests/programMissingFiles.ts"
+9 -9
View File
@@ -560,32 +560,32 @@ namespace ts.refactor.extractMethod {
return "constructor";
case SyntaxKind.FunctionExpression:
return scope.name
? `function expression ${scope.name.getText()}`
? `function expression ${scope.name.text}`
: "anonymous function expression";
case SyntaxKind.FunctionDeclaration:
return `function ${scope.name.getText()}`;
return `function '${scope.name.text}'`;
case SyntaxKind.ArrowFunction:
return "arrow function";
case SyntaxKind.MethodDeclaration:
return `method ${scope.name.getText()}`;
return `method '${scope.name.getText()}`;
case SyntaxKind.GetAccessor:
return `get ${scope.name.getText()}`;
return `'get ${scope.name.getText()}'`;
case SyntaxKind.SetAccessor:
return `set ${scope.name.getText()}`;
return `'set ${scope.name.getText()}'`;
}
}
else if (isModuleBlock(scope)) {
return `namespace ${scope.parent.name.getText()}`;
return `namespace '${scope.parent.name.getText()}'`;
}
else if (isClassLike(scope)) {
return scope.kind === SyntaxKind.ClassDeclaration
? `class ${scope.name.text}`
? `class '${scope.name.text}'`
: scope.name.text
? `class expression ${scope.name.text}`
? `class expression '${scope.name.text}'`
: "anonymous class expression";
}
else if (isSourceFile(scope)) {
return `file '${scope.fileName}'`;
return scope.externalModuleIndicator ? "module scope" : "global scope";
}
else {
return "unknown";
@@ -14,7 +14,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
let x = 1;
function foo() {
@@ -34,7 +34,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
let x = 1;
function foo() {
@@ -55,7 +55,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let x = 1;
function foo() {
@@ -76,7 +76,7 @@ namespace A {
return a;
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
let x = 1;
function foo() {
@@ -9,7 +9,7 @@ namespace A {
}
}
}
==SCOPE::class C==
==SCOPE::class 'C'==
namespace A {
export interface I { x: number };
class C {
@@ -24,7 +24,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
export interface I { x: number };
class C {
@@ -39,7 +39,7 @@ namespace A {
return a1.x + 10;
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
export interface I { x: number };
class C {
@@ -11,7 +11,7 @@ namespace A {
}
}
}
==SCOPE::class C==
==SCOPE::class 'C'==
namespace A {
let y = 1;
class C {
@@ -30,7 +30,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let y = 1;
class C {
@@ -49,7 +49,7 @@ namespace A {
return { __return: a1.x + 10, z };
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
let y = 1;
class C {
@@ -13,7 +13,7 @@ namespace A {
}
}
}
==SCOPE::class C==
==SCOPE::class 'C'==
namespace A {
let y = 1;
class C {
@@ -12,7 +12,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
let x = 1;
function foo() {
@@ -30,7 +30,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
let x = 1;
function foo() {
@@ -48,7 +48,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let x = 1;
function foo() {
@@ -66,7 +66,7 @@ namespace A {
return foo();
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
let x = 1;
function foo() {
@@ -11,7 +11,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
function foo() {
}
@@ -28,7 +28,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
function foo() {
}
@@ -45,7 +45,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
function foo() {
}
@@ -62,7 +62,7 @@ namespace A {
return foo();
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
function foo() {
}
@@ -13,7 +13,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
function foo() {
}
@@ -32,7 +32,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
function foo() {
}
@@ -51,7 +51,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
function foo() {
}
@@ -70,7 +70,7 @@ namespace A {
return foo();
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
function foo() {
}
@@ -14,7 +14,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
let x = 1;
export function foo() {
@@ -34,7 +34,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
let x = 1;
export function foo() {
@@ -55,7 +55,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let x = 1;
export function foo() {
@@ -76,7 +76,7 @@ namespace A {
return a;
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
let x = 1;
export function foo() {
@@ -14,7 +14,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
let x = 1;
export function foo() {
@@ -34,7 +34,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
let x = 1;
export function foo() {
@@ -56,7 +56,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let x = 1;
export function foo() {
@@ -78,7 +78,7 @@ namespace A {
return { __return: foo(), a };
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
let x = 1;
export function foo() {
@@ -16,7 +16,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
let x = 1;
export namespace C {
@@ -38,7 +38,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
let x = 1;
export namespace C {
@@ -62,7 +62,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let x = 1;
export namespace C {
@@ -86,7 +86,7 @@ namespace A {
return { __return: C.foo(), a };
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
let x = 1;
export namespace C {
@@ -8,7 +8,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
let x = 1;
namespace B {
@@ -22,7 +22,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
let x = 1;
namespace B {
@@ -36,7 +36,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let x = 1;
namespace B {
@@ -50,7 +50,7 @@ namespace A {
return 1 + a1 + x;
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
let x = 1;
namespace B {
@@ -8,7 +8,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
export interface I { x: number };
namespace B {
@@ -22,7 +22,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
export interface I { x: number };
namespace B {
@@ -36,7 +36,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
export interface I { x: number };
namespace B {
@@ -50,7 +50,7 @@ namespace A {
return a1.x + 10;
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
export interface I { x: number };
namespace B {
+5 -2
View File
@@ -13,8 +13,11 @@
//// }
goTo.select('start', 'end')
verify.refactorAvailable('Extract Method');
edit.applyRefactor('Extract Method', "scope_0");
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_0",
actionDescription: "Extract function into class 'Foo'",
});
verify.currentFileContentIs(
`class Foo {
someMethod(m: number) {
+7 -2
View File
@@ -1,6 +1,11 @@
/// <reference path='fourslash.ts' />
//// (x => x)(/*1*/x => x/*2*/)(1);
//// export {}; // Make this a module
//// (x => x)(/*1*/x => x/*2*/)(1);
goTo.select('1', '2');
edit.applyRefactor('Extract Method', 'scope_0');
edit.applyRefactor({
refactorName: "Extract Method",
actionName: 'scope_0',
actionDescription: "Extract function into module scope",
});
+10 -2
View File
@@ -10,10 +10,18 @@
//// }
goTo.select('a', 'b');
edit.applyRefactor('Extract Method', 'scope_0');
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_0",
actionDescription: "Extract function into class 'C'",
});
goTo.select('c', 'd');
edit.applyRefactor('Extract Method', 'scope_0');
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_0",
actionDescription: "Extract function into class 'C'",
});
verify.currentFileContentIs(`class C {
static j = C.newFunction_1();
+5 -1
View File
@@ -11,7 +11,11 @@
//// }
goTo.select('a', 'b');
edit.applyRefactor('Extract Method', 'scope_1');
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_1",
actionDescription: "Extract function into global scope",
});
verify.currentFileContentIs(`function foo() {
var i = 10;
var __return: any;
+5 -1
View File
@@ -9,7 +9,11 @@
//// }
goTo.select('a', 'b');
edit.applyRefactor('Extract Method', 'scope_1');
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_1",
actionDescription: "Extract function into global scope",
});
verify.currentFileContentIs(`function foo() {
var i = 10;
+5 -2
View File
@@ -9,8 +9,11 @@
//// }
goTo.select('a', 'b')
verify.refactorAvailable('Extract Method');
edit.applyRefactor('Extract Method', "scope_1");
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_1",
actionDescription: "Extract function into global scope",
});
verify.currentFileContentIs(`function fn() {
const x = { m: 1 };
newFunction(x);
+6 -3
View File
@@ -5,12 +5,15 @@
//// function fn() {
//// /*a*/console.log("hi");/*b*/
//// }
////
////
//// function newFunction() { }
goTo.select('a', 'b')
verify.refactorAvailable('Extract Method');
edit.applyRefactor('Extract Method', "scope_0");
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_0",
actionDescription: "Extract function into function 'fn'",
});
verify.currentFileContentIs(`function fn() {
newFunction_1();
+5 -2
View File
@@ -10,8 +10,11 @@
//// }
//// }
goTo.select('start', 'end')
verify.refactorAvailable('Extract Method');
edit.applyRefactor('Extract Method', "scope_2");
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_2",
actionDescription: "Extract function into global scope",
});
verify.currentFileContentIs(
`namespace NS {
class Q {
+5 -1
View File
@@ -12,7 +12,11 @@ goTo.select('start', 'end')
verify.refactorAvailable('Extract Method');
edit.applyRefactor('Extract Method', "scope_0");
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_0",
actionDescription: "Extract function into class 'Foo'",
});
verify.currentFileContentIs(`class Foo {
static method() {
+5 -1
View File
@@ -7,7 +7,11 @@
//// }
goTo.select('a', 'b')
edit.applyRefactor('Extract Method', 'scope_1');
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_1",
actionDescription: "Extract function into global scope",
});
verify.currentFileContentIs(`function M() {
let a = [1,2,3];
let x = 0;
+5 -1
View File
@@ -8,7 +8,11 @@
//// }
goTo.select('a', 'b')
edit.applyRefactor('Extract Method', 'scope_0');
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_0",
actionDescription: "Extract function into function 'fn'",
});
verify.currentFileContentIs(`function fn() {
var q = newFunction()
q[0]++
+1 -1
View File
@@ -10,7 +10,7 @@
//// }
//// }
// Don't offer to to 'extract method' a single identifier
// Don't offer to 'extract method' a single identifier
goTo.marker('a');
verify.not.refactorAvailable('Extract Method');
+5 -1
View File
@@ -9,7 +9,11 @@
//// }
goTo.select('start', 'end');
edit.applyRefactor('Extract Method', 'scope_0');
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_0",
actionDescription: "Extract function into function 'f'",
});
verify.currentFileContentIs(
`function f() {
var x: 1 | 2 | 3 = newFunction();
+5 -1
View File
@@ -7,7 +7,11 @@
//// }
goTo.select('a', 'b');
edit.applyRefactor('Extract Method', 'scope_0');
edit.applyRefactor({
refactorName: "Extract Method",
actionName: "scope_0",
actionDescription: "Extract function into global scope",
});
verify.currentFileContentIs(`function fn(x = newFunction()) {
}
function newFunction() {
+2 -2
View File
@@ -159,7 +159,7 @@ declare namespace FourSlashInterface {
codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], diagnosticCode?: number): void;
applicableRefactorAvailableForRange(): void;
refactorAvailable(name?: string, subName?: string);
refactorAvailable(name: string, actionName?: string);
}
class verify extends verifyNegatable {
assertHasRanges(ranges: Range[]): void;
@@ -310,7 +310,7 @@ declare namespace FourSlashInterface {
enableFormatting(): void;
disableFormatting(): void;
applyRefactor(refactorName: string, actionName: string): void;
applyRefactor(options: { refactorName: string, actionName: string, actionDescription: string }): void;
}
class debug {
printCurrentParameterHelp(): void;