Skip to content

Commit

Permalink
Test for action description of code actions, and simplify description…
Browse files Browse the repository at this point in the history 
… 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"
  • Loading branch information
Andy committed Aug 25, 2017
1 parent a39ae1f commit 3644771
Show file tree
Hide file tree
Showing 31 changed files with 163 additions and 90 deletions.
2 changes: 1 addition & 1 deletion src/compiler/diagnosticMessages.json
View file
Expand Up @@ -3688,7 +3688,7 @@
"code": 95003
},

"Extract function into '{0}'": {
"Extract function into {0}": {
"category": "Message",
"code": 95004
}
Expand Down
47 changes: 33 additions & 14 deletions src/harness/fourslash.ts
View file
Expand Up @@ -2743,20 +2743,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}`);
}
}
}

Expand All @@ -2776,14 +2781,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);
Expand Down Expand Up @@ -3660,8 +3673,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);
}
}

Expand Down Expand Up @@ -4059,8 +4072,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);
}
}

Expand Down Expand Up @@ -4273,4 +4286,10 @@ namespace FourSlashInterface {
return { classificationType, text, textSpan };
}
}

export interface ApplyRefactorOptions {
refactorName: string;
actionName: string;
actionDescription: string;
}
}
1 change: 1 addition & 0 deletions src/harness/tsconfig.json
View file
Expand Up @@ -123,6 +123,7 @@
"./unittests/printer.ts",
"./unittests/transform.ts",
"./unittests/customTransforms.ts",
"./unittests/extractMethods.ts",
"./unittests/textChanges.ts",
"./unittests/telemetry.ts",
"./unittests/programMissingFiles.ts"
Expand Down
18 changes: 9 additions & 9 deletions src/services/refactors/extractMethod.ts
View file
Expand Up @@ -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";
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/extractMethod/extractMethod1.js
View file
Expand Up @@ -14,7 +14,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
let x = 1;
function foo() {
Expand All @@ -34,7 +34,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
let x = 1;
function foo() {
Expand All @@ -55,7 +55,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let x = 1;
function foo() {
Expand All @@ -76,7 +76,7 @@ namespace A {
return a;
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
let x = 1;
function foo() {
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/extractMethod/extractMethod10.js
View file
Expand Up @@ -9,7 +9,7 @@ namespace A {
}
}
}
==SCOPE::class C==
==SCOPE::class 'C'==
namespace A {
export interface I { x: number };
class C {
Expand All @@ -24,7 +24,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
export interface I { x: number };
class C {
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/extractMethod/extractMethod11.js
View file
Expand Up @@ -11,7 +11,7 @@ namespace A {
}
}
}
==SCOPE::class C==
==SCOPE::class 'C'==
namespace A {
let y = 1;
class C {
Expand All @@ -30,7 +30,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let y = 1;
class C {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/extractMethod/extractMethod12.js
View file
Expand Up @@ -13,7 +13,7 @@ namespace A {
}
}
}
==SCOPE::class C==
==SCOPE::class 'C'==
namespace A {
let y = 1;
class C {
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/extractMethod/extractMethod2.js
View file
Expand Up @@ -12,7 +12,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
let x = 1;
function foo() {
Expand All @@ -30,7 +30,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
let x = 1;
function foo() {
Expand All @@ -48,7 +48,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let x = 1;
function foo() {
Expand All @@ -66,7 +66,7 @@ namespace A {
return foo();
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
let x = 1;
function foo() {
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/extractMethod/extractMethod3.js
View file
Expand Up @@ -11,7 +11,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
function foo() {
}
Expand All @@ -28,7 +28,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
function foo() {
}
Expand All @@ -45,7 +45,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
function foo() {
}
Expand All @@ -62,7 +62,7 @@ namespace A {
return foo();
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
function foo() {
}
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/extractMethod/extractMethod4.js
View file
Expand Up @@ -13,7 +13,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
function foo() {
}
Expand All @@ -32,7 +32,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
function foo() {
}
Expand All @@ -51,7 +51,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
function foo() {
}
Expand All @@ -70,7 +70,7 @@ namespace A {
return foo();
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
function foo() {
}
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/extractMethod/extractMethod5.js
View file
Expand Up @@ -14,7 +14,7 @@ namespace A {
}
}
}
==SCOPE::function a==
==SCOPE::function 'a'==
namespace A {
let x = 1;
export function foo() {
Expand All @@ -34,7 +34,7 @@ namespace A {
}
}
}
==SCOPE::namespace B==
==SCOPE::namespace 'B'==
namespace A {
let x = 1;
export function foo() {
Expand All @@ -55,7 +55,7 @@ namespace A {
}
}
}
==SCOPE::namespace A==
==SCOPE::namespace 'A'==
namespace A {
let x = 1;
export function foo() {
Expand All @@ -76,7 +76,7 @@ namespace A {
return a;
}
}
==SCOPE::file '/a.ts'==
==SCOPE::global scope==
namespace A {
let x = 1;
export function foo() {
Expand Down

0 comments on commit 3644771

Please sign in to comment.