From f80995653af3a3492b9491a7b10a654724dd7370 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Mon, 18 Sep 2017 11:23:14 -0700 Subject: [PATCH 01/10] Tidy up Extract Method fourslash tests Partial port of ea4fdc5a9 --- src/harness/unittests/extractMethods.ts | 8 +- .../{extractMethod1.js => extractMethod1.ts} | 10 +- ...{extractMethod10.js => extractMethod10.ts} | 8 +- ...{extractMethod11.js => extractMethod11.ts} | 8 +- ...{extractMethod12.js => extractMethod12.ts} | 4 +- .../{extractMethod2.js => extractMethod2.ts} | 10 +- .../{extractMethod3.js => extractMethod3.ts} | 10 +- .../{extractMethod4.js => extractMethod4.ts} | 10 +- .../{extractMethod5.js => extractMethod5.ts} | 10 +- .../{extractMethod6.js => extractMethod6.ts} | 10 +- .../{extractMethod7.js => extractMethod7.ts} | 10 +- .../{extractMethod8.js => extractMethod8.ts} | 10 +- .../{extractMethod9.js => extractMethod9.ts} | 10 +- tests/baselines/reference/extractMethod1.js | 98 ---------------- tests/baselines/reference/extractMethod10.js | 70 ----------- tests/baselines/reference/extractMethod11.js | 86 -------------- tests/baselines/reference/extractMethod12.js | 36 ------ tests/baselines/reference/extractMethod2.js | 85 -------------- tests/baselines/reference/extractMethod3.js | 80 ------------- tests/baselines/reference/extractMethod4.js | 90 -------------- tests/baselines/reference/extractMethod5.js | 98 ---------------- tests/baselines/reference/extractMethod6.js | 101 ---------------- tests/baselines/reference/extractMethod7.js | 111 ------------------ tests/baselines/reference/extractMethod8.js | 57 --------- tests/baselines/reference/extractMethod9.js | 65 ---------- 25 files changed, 59 insertions(+), 1036 deletions(-) rename tests/baselines/reference/extractMethod/{extractMethod1.js => extractMethod1.ts} (92%) rename tests/baselines/reference/extractMethod/{extractMethod10.js => extractMethod10.ts} (91%) rename tests/baselines/reference/extractMethod/{extractMethod11.js => extractMethod11.ts} (93%) rename tests/baselines/reference/extractMethod/{extractMethod12.js => extractMethod12.ts} (94%) rename tests/baselines/reference/extractMethod/{extractMethod2.js => extractMethod2.ts} (91%) rename tests/baselines/reference/extractMethod/{extractMethod3.js => extractMethod3.ts} (91%) rename tests/baselines/reference/extractMethod/{extractMethod4.js => extractMethod4.ts} (92%) rename tests/baselines/reference/extractMethod/{extractMethod5.js => extractMethod5.ts} (92%) rename tests/baselines/reference/extractMethod/{extractMethod6.js => extractMethod6.ts} (93%) rename tests/baselines/reference/extractMethod/{extractMethod7.js => extractMethod7.ts} (94%) rename tests/baselines/reference/extractMethod/{extractMethod8.js => extractMethod8.ts} (89%) rename tests/baselines/reference/extractMethod/{extractMethod9.js => extractMethod9.ts} (90%) delete mode 100644 tests/baselines/reference/extractMethod1.js delete mode 100644 tests/baselines/reference/extractMethod10.js delete mode 100644 tests/baselines/reference/extractMethod11.js delete mode 100644 tests/baselines/reference/extractMethod12.js delete mode 100644 tests/baselines/reference/extractMethod2.js delete mode 100644 tests/baselines/reference/extractMethod3.js delete mode 100644 tests/baselines/reference/extractMethod4.js delete mode 100644 tests/baselines/reference/extractMethod5.js delete mode 100644 tests/baselines/reference/extractMethod6.js delete mode 100644 tests/baselines/reference/extractMethod7.js delete mode 100644 tests/baselines/reference/extractMethod8.js delete mode 100644 tests/baselines/reference/extractMethod9.js diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts index 28f45c6997b..8b937826d0a 100644 --- a/src/harness/unittests/extractMethods.ts +++ b/src/harness/unittests/extractMethods.ts @@ -552,7 +552,7 @@ namespace A { function testExtractMethod(caption: string, text: string) { it(caption, () => { - Harness.Baseline.runBaseline(`extractMethod/${caption}.js`, () => { + Harness.Baseline.runBaseline(`extractMethod/${caption}.ts`, () => { const t = extractTest(text); const selectionRange = t.ranges.get("selection"); if (!selectionRange) { @@ -562,7 +562,7 @@ namespace A { path: "/a.ts", content: t.source }; - const host = projectSystem.createServerHost([f]); + const host = projectSystem.createServerHost([f, projectSystem.libFile]); const projectService = projectSystem.createProjectService(host); projectService.openClientFile(f.path); const program = projectService.inferredProjects[0].getLanguageService().getProgram(); @@ -579,12 +579,12 @@ namespace A { assert.equal(result.errors, undefined, "expect no errors"); const results = refactor.extractMethod.getPossibleExtractions(result.targetRange, context); const data: string[] = []; - data.push(`==ORIGINAL==`); + data.push(`// ==ORIGINAL==`); data.push(sourceFile.text); for (const r of results) { const { renameLocation, edits } = refactor.extractMethod.getExtractionAtIndex(result.targetRange, context, results.indexOf(r)); assert.lengthOf(edits, 1); - data.push(`==SCOPE::${r.scopeDescription}==`); + data.push(`// ==SCOPE::${r.scopeDescription}==`); const newText = textChanges.applyChanges(sourceFile.text, edits[0].textChanges); const newTextWithRename = newText.slice(0, renameLocation) + "/*RENAME*/" + newText.slice(renameLocation); data.push(newTextWithRename); diff --git a/tests/baselines/reference/extractMethod/extractMethod1.js b/tests/baselines/reference/extractMethod/extractMethod1.ts similarity index 92% rename from tests/baselines/reference/extractMethod/extractMethod1.js rename to tests/baselines/reference/extractMethod/extractMethod1.ts index 29936f95612..9ae0f78f205 100644 --- a/tests/baselines/reference/extractMethod/extractMethod1.js +++ b/tests/baselines/reference/extractMethod/extractMethod1.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== namespace A { let x = 1; function foo() { @@ -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::global scope== +// ==SCOPE::global scope== namespace A { let x = 1; function foo() { diff --git a/tests/baselines/reference/extractMethod/extractMethod10.js b/tests/baselines/reference/extractMethod/extractMethod10.ts similarity index 91% rename from tests/baselines/reference/extractMethod/extractMethod10.js rename to tests/baselines/reference/extractMethod/extractMethod10.ts index 346c44871eb..c218e2e964a 100644 --- a/tests/baselines/reference/extractMethod/extractMethod10.js +++ b/tests/baselines/reference/extractMethod/extractMethod10.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== namespace A { export interface I { x: number }; class C { @@ -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::global scope== +// ==SCOPE::global scope== namespace A { export interface I { x: number }; class C { diff --git a/tests/baselines/reference/extractMethod/extractMethod11.js b/tests/baselines/reference/extractMethod/extractMethod11.ts similarity index 93% rename from tests/baselines/reference/extractMethod/extractMethod11.js rename to tests/baselines/reference/extractMethod/extractMethod11.ts index fe826586527..47ed083f911 100644 --- a/tests/baselines/reference/extractMethod/extractMethod11.js +++ b/tests/baselines/reference/extractMethod/extractMethod11.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== namespace A { let y = 1; 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::global scope== +// ==SCOPE::global scope== namespace A { let y = 1; class C { diff --git a/tests/baselines/reference/extractMethod/extractMethod12.js b/tests/baselines/reference/extractMethod/extractMethod12.ts similarity index 94% rename from tests/baselines/reference/extractMethod/extractMethod12.js rename to tests/baselines/reference/extractMethod/extractMethod12.ts index 659b93d4457..83ff5afb807 100644 --- a/tests/baselines/reference/extractMethod/extractMethod12.js +++ b/tests/baselines/reference/extractMethod/extractMethod12.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== 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 { diff --git a/tests/baselines/reference/extractMethod/extractMethod2.js b/tests/baselines/reference/extractMethod/extractMethod2.ts similarity index 91% rename from tests/baselines/reference/extractMethod/extractMethod2.js rename to tests/baselines/reference/extractMethod/extractMethod2.ts index 93f28348392..789afb1f6f1 100644 --- a/tests/baselines/reference/extractMethod/extractMethod2.js +++ b/tests/baselines/reference/extractMethod/extractMethod2.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== namespace A { let x = 1; function foo() { @@ -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::global scope== +// ==SCOPE::global scope== namespace A { let x = 1; function foo() { diff --git a/tests/baselines/reference/extractMethod/extractMethod3.js b/tests/baselines/reference/extractMethod/extractMethod3.ts similarity index 91% rename from tests/baselines/reference/extractMethod/extractMethod3.js rename to tests/baselines/reference/extractMethod/extractMethod3.ts index 54d31d2458c..2c45b2fae64 100644 --- a/tests/baselines/reference/extractMethod/extractMethod3.js +++ b/tests/baselines/reference/extractMethod/extractMethod3.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== namespace A { 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::global scope== +// ==SCOPE::global scope== namespace A { function foo() { } diff --git a/tests/baselines/reference/extractMethod/extractMethod4.js b/tests/baselines/reference/extractMethod/extractMethod4.ts similarity index 92% rename from tests/baselines/reference/extractMethod/extractMethod4.js rename to tests/baselines/reference/extractMethod/extractMethod4.ts index dbb5b7f3a88..873aab0e5f5 100644 --- a/tests/baselines/reference/extractMethod/extractMethod4.js +++ b/tests/baselines/reference/extractMethod/extractMethod4.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== 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::global scope== +// ==SCOPE::global scope== namespace A { function foo() { } diff --git a/tests/baselines/reference/extractMethod/extractMethod5.js b/tests/baselines/reference/extractMethod/extractMethod5.ts similarity index 92% rename from tests/baselines/reference/extractMethod/extractMethod5.js rename to tests/baselines/reference/extractMethod/extractMethod5.ts index c14c88608ff..807b973a42c 100644 --- a/tests/baselines/reference/extractMethod/extractMethod5.js +++ b/tests/baselines/reference/extractMethod/extractMethod5.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== 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() { @@ -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::global scope== +// ==SCOPE::global scope== namespace A { let x = 1; export function foo() { diff --git a/tests/baselines/reference/extractMethod/extractMethod6.js b/tests/baselines/reference/extractMethod/extractMethod6.ts similarity index 93% rename from tests/baselines/reference/extractMethod/extractMethod6.js rename to tests/baselines/reference/extractMethod/extractMethod6.ts index e1d976818f6..335eed9fef6 100644 --- a/tests/baselines/reference/extractMethod/extractMethod6.js +++ b/tests/baselines/reference/extractMethod/extractMethod6.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== 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::global scope== +// ==SCOPE::global scope== namespace A { let x = 1; export function foo() { diff --git a/tests/baselines/reference/extractMethod/extractMethod7.js b/tests/baselines/reference/extractMethod/extractMethod7.ts similarity index 94% rename from tests/baselines/reference/extractMethod/extractMethod7.js rename to tests/baselines/reference/extractMethod/extractMethod7.ts index 8bcb79414c0..31c2e4e4693 100644 --- a/tests/baselines/reference/extractMethod/extractMethod7.js +++ b/tests/baselines/reference/extractMethod/extractMethod7.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== namespace A { let x = 1; export namespace C { @@ -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::global scope== +// ==SCOPE::global scope== namespace A { let x = 1; export namespace C { diff --git a/tests/baselines/reference/extractMethod/extractMethod8.js b/tests/baselines/reference/extractMethod/extractMethod8.ts similarity index 89% rename from tests/baselines/reference/extractMethod/extractMethod8.js rename to tests/baselines/reference/extractMethod/extractMethod8.ts index 98853e3484e..23b8629b58f 100644 --- a/tests/baselines/reference/extractMethod/extractMethod8.js +++ b/tests/baselines/reference/extractMethod/extractMethod8.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== namespace A { let x = 1; namespace B { @@ -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::global scope== +// ==SCOPE::global scope== namespace A { let x = 1; namespace B { diff --git a/tests/baselines/reference/extractMethod/extractMethod9.js b/tests/baselines/reference/extractMethod/extractMethod9.ts similarity index 90% rename from tests/baselines/reference/extractMethod/extractMethod9.js rename to tests/baselines/reference/extractMethod/extractMethod9.ts index 700caf708f0..ab82ae9c0ec 100644 --- a/tests/baselines/reference/extractMethod/extractMethod9.js +++ b/tests/baselines/reference/extractMethod/extractMethod9.ts @@ -1,4 +1,4 @@ -==ORIGINAL== +// ==ORIGINAL== namespace A { export interface I { x: number }; 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::global scope== +// ==SCOPE::global scope== namespace A { export interface I { x: number }; namespace B { diff --git a/tests/baselines/reference/extractMethod1.js b/tests/baselines/reference/extractMethod1.js deleted file mode 100644 index 699916b0dc2..00000000000 --- a/tests/baselines/reference/extractMethod1.js +++ /dev/null @@ -1,98 +0,0 @@ -==ORIGINAL== -namespace A { - let x = 1; - function foo() { - } - namespace B { - function a() { - let a = 1; - - let y = 5; - let z = x; - a = y; - foo(); - } - } -} -==SCOPE::function a== -namespace A { - let x = 1; - function foo() { - } - namespace B { - function a() { - let a = 1; - - newFunction(); - - function newFunction() { - let y = 5; - let z = x; - a = y; - foo(); - } - } - } -} -==SCOPE::namespace B== -namespace A { - let x = 1; - function foo() { - } - namespace B { - function a() { - let a = 1; - - ({ a } = newFunction(a)); - } - - function newFunction(a: any) { - let y = 5; - let z = x; - a = y; - foo(); - return { a }; - } - } -} -==SCOPE::namespace A== -namespace A { - let x = 1; - function foo() { - } - namespace B { - function a() { - let a = 1; - - ({ a } = newFunction(a)); - } - } - - function newFunction(a: any) { - let y = 5; - let z = x; - a = y; - foo(); - return { a }; - } -} -==SCOPE::file '/a.ts'== -namespace A { - let x = 1; - function foo() { - } - namespace B { - function a() { - let a = 1; - - ({ a } = newFunction(x, a, foo)); - } - } -} -function newFunction(x: any, a: any, foo: any) { - let y = 5; - let z = x; - a = y; - foo(); - return { a }; -} diff --git a/tests/baselines/reference/extractMethod10.js b/tests/baselines/reference/extractMethod10.js deleted file mode 100644 index e416a66e262..00000000000 --- a/tests/baselines/reference/extractMethod10.js +++ /dev/null @@ -1,70 +0,0 @@ -==ORIGINAL== -namespace A { - export interface I { x: number }; - class C { - a() { - let z = 1; - let a1: I = { x: 1 }; - return a1.x + 10; - } - } -} -==SCOPE::method a== -namespace A { - export interface I { x: number }; - class C { - a() { - let z = 1; - return newFunction(); - - function newFunction() { - let a1: I = { x: 1 }; - return a1.x + 10; - } - } - } -} -==SCOPE::class C== -namespace A { - export interface I { x: number }; - class C { - a() { - let z = 1; - return this.newFunction(); - } - - private newFunction() { - let a1: I = { x: 1 }; - return a1.x + 10; - } - } -} -==SCOPE::namespace A== -namespace A { - export interface I { x: number }; - class C { - a() { - let z = 1; - return newFunction(); - } - } - - function newFunction() { - let a1: I = { x: 1 }; - return a1.x + 10; - } -} -==SCOPE::file '/a.ts'== -namespace A { - export interface I { x: number }; - class C { - a() { - let z = 1; - return newFunction(); - } - } -} -function newFunction() { - let a1: A.I = { x: 1 }; - return a1.x + 10; -} diff --git a/tests/baselines/reference/extractMethod11.js b/tests/baselines/reference/extractMethod11.js deleted file mode 100644 index 4cbd31d4453..00000000000 --- a/tests/baselines/reference/extractMethod11.js +++ /dev/null @@ -1,86 +0,0 @@ -==ORIGINAL== -namespace A { - let y = 1; - class C { - a() { - let z = 1; - let a1 = { x: 1 }; - y = 10; - z = 42; - return a1.x + 10; - } - } -} -==SCOPE::method a== -namespace A { - let y = 1; - class C { - a() { - let z = 1; - return newFunction(); - - function newFunction() { - let a1 = { x: 1 }; - y = 10; - z = 42; - return a1.x + 10; - } - } - } -} -==SCOPE::class C== -namespace A { - let y = 1; - class C { - a() { - let z = 1; - var __return: any; - ({ z, __return } = this.newFunction(z)); - return __return; - } - - private newFunction(z: any) { - let a1 = { x: 1 }; - y = 10; - z = 42; - return { z, __return: a1.x + 10 }; - } - } -} -==SCOPE::namespace A== -namespace A { - let y = 1; - class C { - a() { - let z = 1; - var __return: any; - ({ z, __return } = newFunction(z)); - return __return; - } - } - - function newFunction(z: any) { - let a1 = { x: 1 }; - y = 10; - z = 42; - return { z, __return: a1.x + 10 }; - } -} -==SCOPE::file '/a.ts'== -namespace A { - let y = 1; - class C { - a() { - let z = 1; - var __return: any; - ({ y, z, __return } = newFunction(y, z)); - return __return; - } - } -} -function newFunction(y: any, z: any) { - let a1 = { x: 1 }; - y = 10; - z = 42; - return { y, z, __return: a1.x + 10 }; -} diff --git a/tests/baselines/reference/extractMethod12.js b/tests/baselines/reference/extractMethod12.js deleted file mode 100644 index da0788a937f..00000000000 --- a/tests/baselines/reference/extractMethod12.js +++ /dev/null @@ -1,36 +0,0 @@ -==ORIGINAL== -namespace A { - let y = 1; - class C { - b() {} - a() { - let z = 1; - let a1 = { x: 1 }; - y = 10; - z = 42; - this.b(); - return a1.x + 10; - } - } -} -==SCOPE::class C== -namespace A { - let y = 1; - class C { - b() {} - a() { - let z = 1; - var __return: any; - ({ z, __return } = this.newFunction(z)); - return __return; - } - - private newFunction(z: any) { - let a1 = { x: 1 }; - y = 10; - z = 42; - this.b(); - return { z, __return: a1.x + 10 }; - } - } -} \ No newline at end of file diff --git a/tests/baselines/reference/extractMethod2.js b/tests/baselines/reference/extractMethod2.js deleted file mode 100644 index a89fe52f2fb..00000000000 --- a/tests/baselines/reference/extractMethod2.js +++ /dev/null @@ -1,85 +0,0 @@ -==ORIGINAL== -namespace A { - let x = 1; - function foo() { - } - namespace B { - function a() { - - let y = 5; - let z = x; - return foo(); - } - } -} -==SCOPE::function a== -namespace A { - let x = 1; - function foo() { - } - namespace B { - function a() { - - return newFunction(); - - function newFunction() { - let y = 5; - let z = x; - return foo(); - } - } - } -} -==SCOPE::namespace B== -namespace A { - let x = 1; - function foo() { - } - namespace B { - function a() { - - return newFunction(); - } - - function newFunction() { - let y = 5; - let z = x; - return foo(); - } - } -} -==SCOPE::namespace A== -namespace A { - let x = 1; - function foo() { - } - namespace B { - function a() { - - return newFunction(); - } - } - - function newFunction() { - let y = 5; - let z = x; - return foo(); - } -} -==SCOPE::file '/a.ts'== -namespace A { - let x = 1; - function foo() { - } - namespace B { - function a() { - - return newFunction(x, foo); - } - } -} -function newFunction(x: any, foo: any) { - let y = 5; - let z = x; - return foo(); -} diff --git a/tests/baselines/reference/extractMethod3.js b/tests/baselines/reference/extractMethod3.js deleted file mode 100644 index 847e196130c..00000000000 --- a/tests/baselines/reference/extractMethod3.js +++ /dev/null @@ -1,80 +0,0 @@ -==ORIGINAL== -namespace A { - function foo() { - } - namespace B { - function* a(z: number) { - - let y = 5; - yield z; - return foo(); - } - } -} -==SCOPE::function a== -namespace A { - function foo() { - } - namespace B { - function* a(z: number) { - - return yield* newFunction(); - - function* newFunction() { - let y = 5; - yield z; - return foo(); - } - } - } -} -==SCOPE::namespace B== -namespace A { - function foo() { - } - namespace B { - function* a(z: number) { - - return yield* newFunction(z); - } - - function* newFunction(z: any) { - let y = 5; - yield z; - return foo(); - } - } -} -==SCOPE::namespace A== -namespace A { - function foo() { - } - namespace B { - function* a(z: number) { - - return yield* newFunction(z); - } - } - - function* newFunction(z: any) { - let y = 5; - yield z; - return foo(); - } -} -==SCOPE::file '/a.ts'== -namespace A { - function foo() { - } - namespace B { - function* a(z: number) { - - return yield* newFunction(z, foo); - } - } -} -function* newFunction(z: any, foo: any) { - let y = 5; - yield z; - return foo(); -} diff --git a/tests/baselines/reference/extractMethod4.js b/tests/baselines/reference/extractMethod4.js deleted file mode 100644 index 25f410eeecb..00000000000 --- a/tests/baselines/reference/extractMethod4.js +++ /dev/null @@ -1,90 +0,0 @@ -==ORIGINAL== -namespace A { - function foo() { - } - namespace B { - async function a(z: number, z1: any) { - - let y = 5; - if (z) { - await z1; - } - return foo(); - } - } -} -==SCOPE::function a== -namespace A { - function foo() { - } - namespace B { - async function a(z: number, z1: any) { - - return await newFunction(); - - async function newFunction() { - let y = 5; - if (z) { - await z1; - } - return foo(); - } - } - } -} -==SCOPE::namespace B== -namespace A { - function foo() { - } - namespace B { - async function a(z: number, z1: any) { - - return await newFunction(z, z1); - } - - async function newFunction(z: any, z1: any) { - let y = 5; - if (z) { - await z1; - } - return foo(); - } - } -} -==SCOPE::namespace A== -namespace A { - function foo() { - } - namespace B { - async function a(z: number, z1: any) { - - return await newFunction(z, z1); - } - } - - async function newFunction(z: any, z1: any) { - let y = 5; - if (z) { - await z1; - } - return foo(); - } -} -==SCOPE::file '/a.ts'== -namespace A { - function foo() { - } - namespace B { - async function a(z: number, z1: any) { - - return await newFunction(z, z1, foo); - } - } -} -async function newFunction(z: any, z1: any, foo: any) { - let y = 5; - if (z) { - await z1; - } - return foo(); -} diff --git a/tests/baselines/reference/extractMethod5.js b/tests/baselines/reference/extractMethod5.js deleted file mode 100644 index 135c4ed5f62..00000000000 --- a/tests/baselines/reference/extractMethod5.js +++ /dev/null @@ -1,98 +0,0 @@ -==ORIGINAL== -namespace A { - let x = 1; - export function foo() { - } - namespace B { - function a() { - let a = 1; - - let y = 5; - let z = x; - a = y; - foo(); - } - } -} -==SCOPE::function a== -namespace A { - let x = 1; - export function foo() { - } - namespace B { - function a() { - let a = 1; - - newFunction(); - - function newFunction() { - let y = 5; - let z = x; - a = y; - foo(); - } - } - } -} -==SCOPE::namespace B== -namespace A { - let x = 1; - export function foo() { - } - namespace B { - function a() { - let a = 1; - - ({ a } = newFunction(a)); - } - - function newFunction(a: any) { - let y = 5; - let z = x; - a = y; - foo(); - return { a }; - } - } -} -==SCOPE::namespace A== -namespace A { - let x = 1; - export function foo() { - } - namespace B { - function a() { - let a = 1; - - ({ a } = newFunction(a)); - } - } - - function newFunction(a: any) { - let y = 5; - let z = x; - a = y; - foo(); - return { a }; - } -} -==SCOPE::file '/a.ts'== -namespace A { - let x = 1; - export function foo() { - } - namespace B { - function a() { - let a = 1; - - ({ a } = newFunction(x, a)); - } - } -} -function newFunction(x: any, a: any) { - let y = 5; - let z = x; - a = y; - A.foo(); - return { a }; -} diff --git a/tests/baselines/reference/extractMethod6.js b/tests/baselines/reference/extractMethod6.js deleted file mode 100644 index 94fcc8e6e31..00000000000 --- a/tests/baselines/reference/extractMethod6.js +++ /dev/null @@ -1,101 +0,0 @@ -==ORIGINAL== -namespace A { - let x = 1; - export function foo() { - } - namespace B { - function a() { - let a = 1; - - let y = 5; - let z = x; - a = y; - return foo(); - } - } -} -==SCOPE::function a== -namespace A { - let x = 1; - export function foo() { - } - namespace B { - function a() { - let a = 1; - - return newFunction(); - - function newFunction() { - let y = 5; - let z = x; - a = y; - return foo(); - } - } - } -} -==SCOPE::namespace B== -namespace A { - let x = 1; - export function foo() { - } - namespace B { - function a() { - let a = 1; - - var __return: any; - ({ a, __return } = newFunction(a)); - return __return; - } - - function newFunction(a: any) { - let y = 5; - let z = x; - a = y; - return { a, __return: foo() }; - } - } -} -==SCOPE::namespace A== -namespace A { - let x = 1; - export function foo() { - } - namespace B { - function a() { - let a = 1; - - var __return: any; - ({ a, __return } = newFunction(a)); - return __return; - } - } - - function newFunction(a: any) { - let y = 5; - let z = x; - a = y; - return { a, __return: foo() }; - } -} -==SCOPE::file '/a.ts'== -namespace A { - let x = 1; - export function foo() { - } - namespace B { - function a() { - let a = 1; - - var __return: any; - ({ a, __return } = newFunction(x, a)); - return __return; - } - } -} -function newFunction(x: any, a: any) { - let y = 5; - let z = x; - a = y; - return { a, __return: A.foo() }; -} diff --git a/tests/baselines/reference/extractMethod7.js b/tests/baselines/reference/extractMethod7.js deleted file mode 100644 index c7c3cc8d77a..00000000000 --- a/tests/baselines/reference/extractMethod7.js +++ /dev/null @@ -1,111 +0,0 @@ -==ORIGINAL== -namespace A { - let x = 1; - export namespace C { - export function foo() { - } - } - namespace B { - function a() { - let a = 1; - - let y = 5; - let z = x; - a = y; - return C.foo(); - } - } -} -==SCOPE::function a== -namespace A { - let x = 1; - export namespace C { - export function foo() { - } - } - namespace B { - function a() { - let a = 1; - - return newFunction(); - - function newFunction() { - let y = 5; - let z = x; - a = y; - return C.foo(); - } - } - } -} -==SCOPE::namespace B== -namespace A { - let x = 1; - export namespace C { - export function foo() { - } - } - namespace B { - function a() { - let a = 1; - - var __return: any; - ({ a, __return } = newFunction(a)); - return __return; - } - - function newFunction(a: any) { - let y = 5; - let z = x; - a = y; - return { a, __return: C.foo() }; - } - } -} -==SCOPE::namespace A== -namespace A { - let x = 1; - export namespace C { - export function foo() { - } - } - namespace B { - function a() { - let a = 1; - - var __return: any; - ({ a, __return } = newFunction(a)); - return __return; - } - } - - function newFunction(a: any) { - let y = 5; - let z = x; - a = y; - return { a, __return: C.foo() }; - } -} -==SCOPE::file '/a.ts'== -namespace A { - let x = 1; - export namespace C { - export function foo() { - } - } - namespace B { - function a() { - let a = 1; - - var __return: any; - ({ a, __return } = newFunction(x, a)); - return __return; - } - } -} -function newFunction(x: any, a: any) { - let y = 5; - let z = x; - a = y; - return { a, __return: A.C.foo() }; -} diff --git a/tests/baselines/reference/extractMethod8.js b/tests/baselines/reference/extractMethod8.js deleted file mode 100644 index f030b0ea4af..00000000000 --- a/tests/baselines/reference/extractMethod8.js +++ /dev/null @@ -1,57 +0,0 @@ -==ORIGINAL== -namespace A { - let x = 1; - namespace B { - function a() { - let a1 = 1; - return 1 + a1 + x + 100; - } - } -} -==SCOPE::function a== -namespace A { - let x = 1; - namespace B { - function a() { - let a1 = 1; - return newFunction() + 100; - - function newFunction() { 1 + a1 + x; } - } - } -} -==SCOPE::namespace B== -namespace A { - let x = 1; - namespace B { - function a() { - let a1 = 1; - return newFunction(a1) + 100; - } - - function newFunction(a1: any) { 1 + a1 + x; } - } -} -==SCOPE::namespace A== -namespace A { - let x = 1; - namespace B { - function a() { - let a1 = 1; - return newFunction(a1) + 100; - } - } - - function newFunction(a1: any) { 1 + a1 + x; } -} -==SCOPE::file '/a.ts'== -namespace A { - let x = 1; - namespace B { - function a() { - let a1 = 1; - return newFunction(a1, x) + 100; - } - } -} -function newFunction(a1: any, x: any) { 1 + a1 + x; } diff --git a/tests/baselines/reference/extractMethod9.js b/tests/baselines/reference/extractMethod9.js deleted file mode 100644 index fcc5dcd23de..00000000000 --- a/tests/baselines/reference/extractMethod9.js +++ /dev/null @@ -1,65 +0,0 @@ -==ORIGINAL== -namespace A { - export interface I { x: number }; - namespace B { - function a() { - let a1: I = { x: 1 }; - return a1.x + 10; - } - } -} -==SCOPE::function a== -namespace A { - export interface I { x: number }; - namespace B { - function a() { - return newFunction(); - - function newFunction() { - let a1: I = { x: 1 }; - return a1.x + 10; - } - } - } -} -==SCOPE::namespace B== -namespace A { - export interface I { x: number }; - namespace B { - function a() { - return newFunction(); - } - - function newFunction() { - let a1: I = { x: 1 }; - return a1.x + 10; - } - } -} -==SCOPE::namespace A== -namespace A { - export interface I { x: number }; - namespace B { - function a() { - return newFunction(); - } - } - - function newFunction() { - let a1: I = { x: 1 }; - return a1.x + 10; - } -} -==SCOPE::file '/a.ts'== -namespace A { - export interface I { x: number }; - namespace B { - function a() { - return newFunction(); - } - } -} -function newFunction() { - let a1: A.I = { x: 1 }; - return a1.x + 10; -} From d36f254f5e529814105956c3a664202467ce996d Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 31 Aug 2017 13:21:08 -0700 Subject: [PATCH 02/10] Fix extract method for anon class expressions (#18168) Check `scope.name` when trying to extract from an anon class (cherry picked from commit c7b4ed3a91964915b953b34ad2ae72f36e7d6efe) --- src/harness/unittests/extractMethods.ts | 8 ++++++ src/services/refactors/extractMethod.ts | 2 +- .../extractMethod/extractMethod20.ts | 28 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/extractMethod/extractMethod20.ts diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts index 8b937826d0a..9fc7bd689e9 100644 --- a/src/harness/unittests/extractMethods.ts +++ b/src/harness/unittests/extractMethods.ts @@ -547,6 +547,14 @@ namespace A { } } }`); + + testExtractMethod("extractMethod20", + `const _ = class { + a() { + [#|let a1 = { x: 1 }; + return a1.x + 10;|] + } +}`); }); diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index 505f514b04c..f2b644a9389 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -570,7 +570,7 @@ namespace ts.refactor.extractMethod { else if (isClassLike(scope)) { return scope.kind === SyntaxKind.ClassDeclaration ? `class '${scope.name.text}'` - : scope.name.text + : scope.name && scope.name.text ? `class expression '${scope.name.text}'` : "anonymous class expression"; } diff --git a/tests/baselines/reference/extractMethod/extractMethod20.ts b/tests/baselines/reference/extractMethod/extractMethod20.ts new file mode 100644 index 00000000000..1731109b919 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod20.ts @@ -0,0 +1,28 @@ +// ==ORIGINAL== +const _ = class { + a() { + let a1 = { x: 1 }; + return a1.x + 10; + } +} +// ==SCOPE::anonymous class expression== +const _ = class { + a() { + return this./*RENAME*/newFunction(); + } + + private newFunction() { + let a1 = { x: 1 }; + return a1.x + 10; + } +} +// ==SCOPE::global scope== +const _ = class { + a() { + return /*RENAME*/newFunction(); + } +} +function newFunction() { + let a1 = { x: 1 }; + return a1.x + 10; +} From 2dd647d08c07c78b60207e5849be40d830ad7491 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Thu, 7 Sep 2017 16:32:55 -0700 Subject: [PATCH 03/10] Merge pull request #18164 from amcasey/GH18140 Handle the combination of a write and a void return (cherry picked from commit 02cfb81ac0c9b870d31be14b16b0a08ea24ed32a) --- src/harness/unittests/extractMethods.ts | 7 +++++ src/services/refactors/extractMethod.ts | 4 +++ .../extractMethod/extractMethod21.ts | 26 +++++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 tests/baselines/reference/extractMethod/extractMethod21.ts diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts index 9fc7bd689e9..72617bf7740 100644 --- a/src/harness/unittests/extractMethods.ts +++ b/src/harness/unittests/extractMethods.ts @@ -554,6 +554,13 @@ namespace A { [#|let a1 = { x: 1 }; return a1.x + 10;|] } +}`); + // Write + void return + testExtractMethod("extractMethod21", + `function foo() { + let x = 10; + [#|x++; + return;|] }`); }); diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index f2b644a9389..bb9df097940 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -722,6 +722,10 @@ namespace ts.refactor.extractMethod { } else { newNodes.push(createStatement(createBinary(assignments[0].name, SyntaxKind.EqualsToken, call))); + + if (range.facts & RangeFacts.HasReturn) { + newNodes.push(createReturn()); + } } } else { diff --git a/tests/baselines/reference/extractMethod/extractMethod21.ts b/tests/baselines/reference/extractMethod/extractMethod21.ts new file mode 100644 index 00000000000..bbbee38781a --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod21.ts @@ -0,0 +1,26 @@ +// ==ORIGINAL== +function foo() { + let x = 10; + x++; + return; +} +// ==SCOPE::function 'foo'== +function foo() { + let x = 10; + return /*RENAME*/newFunction(); + + function newFunction() { + x++; + return; + } +} +// ==SCOPE::global scope== +function foo() { + let x = 10; + x = /*RENAME*/newFunction(x); + return; +} +function newFunction(x: number) { + x++; + return x; +} From 12d1ea50ca55bb724d3cef40ecd88f8dbacfd3e1 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Fri, 8 Sep 2017 10:50:05 -0700 Subject: [PATCH 04/10] Merge pull request #18165 from amcasey/GH18144 Simplify and correct PermittedJumps computation (cherry picked from commit deefb01c9d53020a46a1692bf73b34696dad5c0c) --- src/harness/unittests/extractMethods.ts | 33 ++++++++++ src/services/refactors/extractMethod.ts | 66 ++++++++----------- .../extractMethod/extractMethod22.ts | 31 +++++++++ 3 files changed, 90 insertions(+), 40 deletions(-) create mode 100644 tests/baselines/reference/extractMethod/extractMethod22.ts diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts index 72617bf7740..b6a3ee7cfe1 100644 --- a/src/harness/unittests/extractMethods.ts +++ b/src/harness/unittests/extractMethods.ts @@ -378,6 +378,30 @@ namespace A { "Cannot extract range containing conditional return statement." ]); + testExtractRangeFailed("extractRangeFailed7", + ` +function test(x: number) { + while (x) { + x--; + [#|break;|] + } +} + `, + [ + "Cannot extract range containing conditional break or continue statements." + ]); + testExtractRangeFailed("extractRangeFailed8", + ` +function test(x: number) { + switch (x) { + case 1: + [#|break;|] + } +} + `, + [ + "Cannot extract range containing conditional break or continue statements." + ]); testExtractRangeFailed("extract-method-not-for-token-expression-statement", `[#|a|]`, ["Select more than a single token."]); testExtractMethod("extractMethod1", @@ -561,6 +585,15 @@ namespace A { let x = 10; [#|x++; return;|] +}`); + // Return in finally block + testExtractMethod("extractMethod22", + `function test() { + try { + } + finally { + [#|return 1;|] + } }`); }); diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index bb9df097940..4849b75c348 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -340,45 +340,31 @@ namespace ts.refactor.extractMethod { return false; } const savedPermittedJumps = permittedJumps; - if (node.parent) { - switch (node.parent.kind) { - case SyntaxKind.IfStatement: - if ((node.parent).thenStatement === node || (node.parent).elseStatement === node) { - // forbid all jumps inside thenStatement or elseStatement - permittedJumps = PermittedJumps.None; - } - break; - case SyntaxKind.TryStatement: - if ((node.parent).tryBlock === node) { - // forbid all jumps inside try blocks - permittedJumps = PermittedJumps.None; - } - else if ((node.parent).finallyBlock === node) { - // allow unconditional returns from finally blocks - permittedJumps = PermittedJumps.Return; - } - break; - case SyntaxKind.CatchClause: - if ((node.parent).block === node) { - // forbid all jumps inside the block of catch clause - permittedJumps = PermittedJumps.None; - } - break; - case SyntaxKind.CaseClause: - if ((node).expression !== node) { - // allow unlabeled break inside case clauses - permittedJumps |= PermittedJumps.Break; - } - break; - default: - if (isIterationStatement(node.parent, /*lookInLabeledStatements*/ false)) { - if ((node.parent).statement === node) { - // allow unlabeled break/continue inside loops - permittedJumps |= PermittedJumps.Break | PermittedJumps.Continue; - } - } - break; - } + + switch (node.kind) { + case SyntaxKind.IfStatement: + permittedJumps = PermittedJumps.None; + break; + case SyntaxKind.TryStatement: + // forbid all jumps inside try blocks + permittedJumps = PermittedJumps.None; + break; + case SyntaxKind.Block: + if (node.parent && node.parent.kind === SyntaxKind.TryStatement && (node).finallyBlock === node) { + // allow unconditional returns from finally blocks + permittedJumps = PermittedJumps.Return; + } + break; + case SyntaxKind.CaseClause: + // allow unlabeled break inside case clauses + permittedJumps |= PermittedJumps.Break; + break; + default: + if (isIterationStatement(node, /*lookInLabeledStatements*/ false)) { + // allow unlabeled break/continue inside loops + permittedJumps |= PermittedJumps.Break | PermittedJumps.Continue; + } + break; } switch (node.kind) { @@ -405,7 +391,7 @@ namespace ts.refactor.extractMethod { } } else { - if (!(permittedJumps & (SyntaxKind.BreakStatement ? PermittedJumps.Break : PermittedJumps.Continue))) { + if (!(permittedJumps & (node.kind === SyntaxKind.BreakStatement ? PermittedJumps.Break : PermittedJumps.Continue))) { // attempt to break or continue in a forbidden context (errors || (errors = [])).push(createDiagnosticForNode(node, Messages.CannotExtractRangeContainingConditionalBreakOrContinueStatements)); } diff --git a/tests/baselines/reference/extractMethod/extractMethod22.ts b/tests/baselines/reference/extractMethod/extractMethod22.ts new file mode 100644 index 00000000000..9b85b89559d --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod22.ts @@ -0,0 +1,31 @@ +// ==ORIGINAL== +function test() { + try { + } + finally { + return 1; + } +} +// ==SCOPE::function 'test'== +function test() { + try { + } + finally { + return /*RENAME*/newFunction(); + } + + function newFunction() { + return 1; + } +} +// ==SCOPE::global scope== +function test() { + try { + } + finally { + return /*RENAME*/newFunction(); + } +} +function newFunction() { + return 1; +} From 334125ed2314b81ec1460d2daba68f721800ac4a Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 12 Sep 2017 16:48:45 -0700 Subject: [PATCH 05/10] Merge pull request #18343 from amcasey/InsertionPosition Improve insertion position of extracted methods (cherry picked from commit 40e459117aeb0b792a23d6805af884b594dd42c4) --- src/compiler/core.ts | 2 + src/harness/unittests/extractMethods.ts | 54 +++++++++++++++++++ src/services/refactors/extractMethod.ts | 44 ++++++++++++++- .../extractMethod/extractMethod23.ts | 43 +++++++++++++++ .../extractMethod/extractMethod24.ts | 43 +++++++++++++++ .../extractMethod/extractMethod25.ts | 26 +++++++++ .../extractMethod/extractMethod26.ts | 31 +++++++++++ .../extractMethod/extractMethod27.ts | 34 ++++++++++++ .../extractMethod/extractMethod28.ts | 34 ++++++++++++ tests/cases/fourslash/extract-method13.ts | 8 +-- 10 files changed, 313 insertions(+), 6 deletions(-) create mode 100644 tests/baselines/reference/extractMethod/extractMethod23.ts create mode 100644 tests/baselines/reference/extractMethod/extractMethod24.ts create mode 100644 tests/baselines/reference/extractMethod/extractMethod25.ts create mode 100644 tests/baselines/reference/extractMethod/extractMethod26.ts create mode 100644 tests/baselines/reference/extractMethod/extractMethod27.ts create mode 100644 tests/baselines/reference/extractMethod/extractMethod28.ts diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 309da10b18e..07c9deff1db 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -2596,4 +2596,6 @@ namespace ts { export function isCheckJsEnabledForFile(sourceFile: SourceFile, compilerOptions: CompilerOptions) { return sourceFile.checkJsDirective ? sourceFile.checkJsDirective.enabled : compilerOptions.checkJs; } + + export function assertTypeIsNever(_: never): void {} } diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts index b6a3ee7cfe1..b9b23234a57 100644 --- a/src/harness/unittests/extractMethods.ts +++ b/src/harness/unittests/extractMethods.ts @@ -594,6 +594,60 @@ function test(x: number) { finally { [#|return 1;|] } +}`); + // Extraction position - namespace + testExtractMethod("extractMethod23", + `namespace NS { + function M1() { } + function M2() { + [#|return 1;|] + } + function M3() { } +}`); + // Extraction position - function + testExtractMethod("extractMethod24", + `function Outer() { + function M1() { } + function M2() { + [#|return 1;|] + } + function M3() { } +}`); + // Extraction position - file + testExtractMethod("extractMethod25", + `function M1() { } +function M2() { + [#|return 1;|] +} +function M3() { }`); + // Extraction position - class without ctor + testExtractMethod("extractMethod26", + `class C { + M1() { } + M2() { + [#|return 1;|] + } + M3() { } +}`); + // Extraction position - class with ctor in middle + testExtractMethod("extractMethod27", + `class C { + M1() { } + M2() { + [#|return 1;|] + } + constructor() { } + M3() { } +}`); + // Extraction position - class with ctor at end + testExtractMethod("extractMethod28", + `class C { + M1() { } + M2() { + [#|return 1;|] + } + M3() { } + constructor() { } }`); }); diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index 4849b75c348..e5de0b5b98c 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -669,8 +669,14 @@ namespace ts.refactor.extractMethod { } const changeTracker = textChanges.ChangeTracker.fromCodeFixContext(context); - // insert function at the end of the scope - changeTracker.insertNodeBefore(context.file, scope.getLastToken(), newFunction, { prefix: context.newLineCharacter, suffix: context.newLineCharacter }); + const minInsertionPos = (isReadonlyArray(range.range) ? lastOrUndefined(range.range) : range.range).end; + const nodeToInsertBefore = getNodeToInsertBefore(minInsertionPos, scope); + if (nodeToInsertBefore) { + changeTracker.insertNodeBefore(context.file, nodeToInsertBefore, newFunction, { suffix: context.newLineCharacter + context.newLineCharacter }); + } + else { + changeTracker.insertNodeBefore(context.file, scope.getLastToken(), newFunction, { prefix: context.newLineCharacter, suffix: context.newLineCharacter }); + } const newNodes: Node[] = []; // replace range with function call @@ -752,6 +758,39 @@ namespace ts.refactor.extractMethod { const renameFilename = renameRange.getSourceFile().fileName; const renameLocation = getRenameLocation(edits, renameFilename, functionNameText); return { renameFilename, renameLocation, edits }; + + function getStatementsOrClassElements(scope: Scope): ReadonlyArray | ReadonlyArray { + if (isFunctionLike(scope)) { + const body = scope.body; + if (isBlock(body)) { + return body.statements; + } + } + else if (isModuleBlock(scope) || isSourceFile(scope)) { + return scope.statements; + } + else if (isClassLike(scope)) { + return scope.members; + } + else { + assertTypeIsNever(scope); + } + + return emptyArray; + } + + /** + * If `scope` contains a function after `minPos`, then return the first such function. + * Otherwise, return `undefined`. + */ + function getNodeToInsertBefore(minPos: number, scope: Scope): Node | undefined { + const children = getStatementsOrClassElements(scope); + for (const child of children) { + if (child.pos >= minPos && isFunctionLike(child) && !isConstructorDeclaration(child)) { + return child; + } + } + } } function getRenameLocation(edits: ReadonlyArray, renameFilename: string, functionNameText: string): number { @@ -784,6 +823,7 @@ namespace ts.refactor.extractMethod { } } + function transformFunctionBody(body: Node, writes: ReadonlyArray, substitutions: ReadonlyMap, hasReturn: boolean): { body: Block, returnValueProperty: string } { if (isBlock(body) && !writes && substitutions.size === 0) { // already block, no writes to propagate back, no substitutions - can use node as is diff --git a/tests/baselines/reference/extractMethod/extractMethod23.ts b/tests/baselines/reference/extractMethod/extractMethod23.ts new file mode 100644 index 00000000000..27fc7105e67 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod23.ts @@ -0,0 +1,43 @@ +// ==ORIGINAL== +namespace NS { + function M1() { } + function M2() { + return 1; + } + function M3() { } +} +// ==SCOPE::function 'M2'== +namespace NS { + function M1() { } + function M2() { + return /*RENAME*/newFunction(); + + function newFunction() { + return 1; + } + } + function M3() { } +} +// ==SCOPE::namespace 'NS'== +namespace NS { + function M1() { } + function M2() { + return /*RENAME*/newFunction(); + } + function newFunction() { + return 1; + } + + function M3() { } +} +// ==SCOPE::global scope== +namespace NS { + function M1() { } + function M2() { + return /*RENAME*/newFunction(); + } + function M3() { } +} +function newFunction() { + return 1; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod24.ts b/tests/baselines/reference/extractMethod/extractMethod24.ts new file mode 100644 index 00000000000..8a4a9d85372 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod24.ts @@ -0,0 +1,43 @@ +// ==ORIGINAL== +function Outer() { + function M1() { } + function M2() { + return 1; + } + function M3() { } +} +// ==SCOPE::function 'M2'== +function Outer() { + function M1() { } + function M2() { + return /*RENAME*/newFunction(); + + function newFunction() { + return 1; + } + } + function M3() { } +} +// ==SCOPE::function 'Outer'== +function Outer() { + function M1() { } + function M2() { + return /*RENAME*/newFunction(); + } + function newFunction() { + return 1; + } + + function M3() { } +} +// ==SCOPE::global scope== +function Outer() { + function M1() { } + function M2() { + return /*RENAME*/newFunction(); + } + function M3() { } +} +function newFunction() { + return 1; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod25.ts b/tests/baselines/reference/extractMethod/extractMethod25.ts new file mode 100644 index 00000000000..6a50d7c184f --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod25.ts @@ -0,0 +1,26 @@ +// ==ORIGINAL== +function M1() { } +function M2() { + return 1; +} +function M3() { } +// ==SCOPE::function 'M2'== +function M1() { } +function M2() { + return /*RENAME*/newFunction(); + + function newFunction() { + return 1; + } +} +function M3() { } +// ==SCOPE::global scope== +function M1() { } +function M2() { + return /*RENAME*/newFunction(); +} +function newFunction() { + return 1; +} + +function M3() { } \ No newline at end of file diff --git a/tests/baselines/reference/extractMethod/extractMethod26.ts b/tests/baselines/reference/extractMethod/extractMethod26.ts new file mode 100644 index 00000000000..a680f5993af --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod26.ts @@ -0,0 +1,31 @@ +// ==ORIGINAL== +class C { + M1() { } + M2() { + return 1; + } + M3() { } +} +// ==SCOPE::class 'C'== +class C { + M1() { } + M2() { + return this./*RENAME*/newFunction(); + } + private newFunction() { + return 1; + } + + M3() { } +} +// ==SCOPE::global scope== +class C { + M1() { } + M2() { + return /*RENAME*/newFunction(); + } + M3() { } +} +function newFunction() { + return 1; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod27.ts b/tests/baselines/reference/extractMethod/extractMethod27.ts new file mode 100644 index 00000000000..01d43051492 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod27.ts @@ -0,0 +1,34 @@ +// ==ORIGINAL== +class C { + M1() { } + M2() { + return 1; + } + constructor() { } + M3() { } +} +// ==SCOPE::class 'C'== +class C { + M1() { } + M2() { + return this./*RENAME*/newFunction(); + } + constructor() { } + private newFunction() { + return 1; + } + + M3() { } +} +// ==SCOPE::global scope== +class C { + M1() { } + M2() { + return /*RENAME*/newFunction(); + } + constructor() { } + M3() { } +} +function newFunction() { + return 1; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod28.ts b/tests/baselines/reference/extractMethod/extractMethod28.ts new file mode 100644 index 00000000000..b0f7a8d9f29 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod28.ts @@ -0,0 +1,34 @@ +// ==ORIGINAL== +class C { + M1() { } + M2() { + return 1; + } + M3() { } + constructor() { } +} +// ==SCOPE::class 'C'== +class C { + M1() { } + M2() { + return this./*RENAME*/newFunction(); + } + private newFunction() { + return 1; + } + + M3() { } + constructor() { } +} +// ==SCOPE::global scope== +class C { + M1() { } + M2() { + return /*RENAME*/newFunction(); + } + M3() { } + constructor() { } +} +function newFunction() { + return 1; +} diff --git a/tests/cases/fourslash/extract-method13.ts b/tests/cases/fourslash/extract-method13.ts index 9fe95848727..7c4586d0eec 100644 --- a/tests/cases/fourslash/extract-method13.ts +++ b/tests/cases/fourslash/extract-method13.ts @@ -37,12 +37,12 @@ edit.applyRefactor({ constructor(q: string = C.newFunction()) { } - private static newFunction(): string { - return "a" + "b"; - } - private static newFunction_1() { return 1 + 1; } + + private static newFunction(): string { + return "a" + "b"; + } }` }); From a667b0455da03f6cb32d47309454c81b8a016076 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 13 Sep 2017 16:25:19 -0700 Subject: [PATCH 06/10] Merge pull request #18423 from amcasey/GH18188 Call getShorthandAssignmentValueSymbol rather than getSymbolAtLocation (cherry picked from commit aade97111affde82044cfc1384746f141efd0694) --- src/harness/unittests/extractMethods.ts | 19 ++++++ src/services/refactors/extractMethod.ts | 6 +- .../extractMethod/extractMethod29.ts | 62 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/extractMethod/extractMethod29.ts diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts index b9b23234a57..727cad66ed2 100644 --- a/src/harness/unittests/extractMethods.ts +++ b/src/harness/unittests/extractMethods.ts @@ -648,6 +648,25 @@ function M3() { }`); } M3() { } constructor() { } +}`); + // Shorthand property names + testExtractMethod("extractMethod29", + `interface UnaryExpression { + kind: "Unary"; + operator: string; + operand: any; +} + +function parseUnaryExpression(operator: string): UnaryExpression { + [#|return { + kind: "Unary", + operator, + operand: parsePrimaryExpression(), + };|] +} + +function parsePrimaryExpression(): any { + throw "Not implemented"; }`); }); diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index e5de0b5b98c..6f7fd5447c7 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -1022,7 +1022,11 @@ namespace ts.refactor.extractMethod { } function recordUsagebySymbol(identifier: Identifier, usage: Usage, isTypeName: boolean) { - const symbol = checker.getSymbolAtLocation(identifier); + // If the identifier is both a property name and its value, we're only interested in its value + // (since the name is a declaration and will be included in the extracted range). + const symbol = identifier.parent && isShorthandPropertyAssignment(identifier.parent) && identifier.parent.name === identifier + ? checker.getShorthandAssignmentValueSymbol(identifier.parent) + : checker.getSymbolAtLocation(identifier); if (!symbol) { // cannot find symbol - do nothing return undefined; diff --git a/tests/baselines/reference/extractMethod/extractMethod29.ts b/tests/baselines/reference/extractMethod/extractMethod29.ts new file mode 100644 index 00000000000..fafa493d4c3 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod29.ts @@ -0,0 +1,62 @@ +// ==ORIGINAL== +interface UnaryExpression { + kind: "Unary"; + operator: string; + operand: any; +} + +function parseUnaryExpression(operator: string): UnaryExpression { + return { + kind: "Unary", + operator, + operand: parsePrimaryExpression(), + }; +} + +function parsePrimaryExpression(): any { + throw "Not implemented"; +} +// ==SCOPE::function 'parseUnaryExpression'== +interface UnaryExpression { + kind: "Unary"; + operator: string; + operand: any; +} + +function parseUnaryExpression(operator: string): UnaryExpression { + return /*RENAME*/newFunction(); + + function newFunction() { + return { + kind: "Unary", + operator, + operand: parsePrimaryExpression(), + }; + } +} + +function parsePrimaryExpression(): any { + throw "Not implemented"; +} +// ==SCOPE::global scope== +interface UnaryExpression { + kind: "Unary"; + operator: string; + operand: any; +} + +function parseUnaryExpression(operator: string): UnaryExpression { + return /*RENAME*/newFunction(operator); +} + +function newFunction(operator: string) { + return { + kind: "Unary", + operator, + operand: parsePrimaryExpression(), + }; +} + +function parsePrimaryExpression(): any { + throw "Not implemented"; +} \ No newline at end of file From fbb6cd57c0ab992c1e2fd579c4a5aed90dcf0a04 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 13 Sep 2017 16:42:55 -0700 Subject: [PATCH 07/10] Merge pull request #18448 from amcasey/NestedReturn Only introduce return properties at the top level (cherry picked from commit 288a57c16dbf2b55070ed350d139de618953d9ef) --- src/harness/unittests/extractMethods.ts | 27 +++++++++++ src/services/refactors/extractMethod.ts | 9 +++- .../extractMethod/extractMethod31.ts | 45 ++++++++++++++++++ .../extractMethod/extractMethod32.ts | 46 +++++++++++++++++++ 4 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 tests/baselines/reference/extractMethod/extractMethod31.ts create mode 100644 tests/baselines/reference/extractMethod/extractMethod32.ts diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts index 727cad66ed2..e7d86331a82 100644 --- a/src/harness/unittests/extractMethods.ts +++ b/src/harness/unittests/extractMethods.ts @@ -667,6 +667,33 @@ function parseUnaryExpression(operator: string): UnaryExpression { function parsePrimaryExpression(): any { throw "Not implemented"; +}`); + // Return in nested function + testExtractMethod("extractMethod31", + `namespace N { + + export const value = 1; + + () => { + var f: () => number; + [#|f = function (): number { + return value; + }|] + } +}`); + // Return in nested class + testExtractMethod("extractMethod32", + `namespace N { + + export const value = 1; + + () => { + [#|var c = class { + M() { + return value; + } + }|] + } }`); }); diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index 6f7fd5447c7..ebc13dd2696 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -830,6 +830,7 @@ namespace ts.refactor.extractMethod { return { body: createBlock(body.statements, /*multLine*/ true), returnValueProperty: undefined }; } let returnValueProperty: string; + let ignoreReturns = false; const statements = createNodeArray(isBlock(body) ? body.statements.slice(0) : [isStatement(body) ? body : createReturn(body)]); // rewrite body if either there are writes that should be propagated back via return statements or there are substitutions if (writes || substitutions.size) { @@ -852,7 +853,7 @@ namespace ts.refactor.extractMethod { } function visitor(node: Node): VisitResult { - if (node.kind === SyntaxKind.ReturnStatement && writes) { + if (!ignoreReturns && node.kind === SyntaxKind.ReturnStatement && writes) { const assignments: ObjectLiteralElementLike[] = getPropertyAssignmentsForWrites(writes); if ((node).expression) { if (!returnValueProperty) { @@ -868,8 +869,12 @@ namespace ts.refactor.extractMethod { } } else { + const oldIgnoreReturns = ignoreReturns; + ignoreReturns = ignoreReturns || isFunctionLike(node) || isClassLike(node); const substitution = substitutions.get(getNodeId(node).toString()); - return substitution || visitEachChild(node, visitor, nullTransformationContext); + const result = substitution || visitEachChild(node, visitor, nullTransformationContext); + ignoreReturns = oldIgnoreReturns; + return result; } } } diff --git a/tests/baselines/reference/extractMethod/extractMethod31.ts b/tests/baselines/reference/extractMethod/extractMethod31.ts new file mode 100644 index 00000000000..bc5a0f16d57 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod31.ts @@ -0,0 +1,45 @@ +// ==ORIGINAL== +namespace N { + + export const value = 1; + + () => { + var f: () => number; + f = function (): number { + return value; + } + } +} +// ==SCOPE::namespace 'N'== +namespace N { + + export const value = 1; + + () => { + var f: () => number; + f = /*RENAME*/newFunction(f); + } + + function newFunction(f: () => number) { + f = function(): number { + return value; + }; + return f; + } +} +// ==SCOPE::global scope== +namespace N { + + export const value = 1; + + () => { + var f: () => number; + f = /*RENAME*/newFunction(f); + } +} +function newFunction(f: () => number) { + f = function(): number { + return N.value; + }; + return f; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod32.ts b/tests/baselines/reference/extractMethod/extractMethod32.ts new file mode 100644 index 00000000000..c28aaf2a5a1 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod32.ts @@ -0,0 +1,46 @@ +// ==ORIGINAL== +namespace N { + + export const value = 1; + + () => { + var c = class { + M() { + return value; + } + } + } +} +// ==SCOPE::namespace 'N'== +namespace N { + + export const value = 1; + + () => { + /*RENAME*/newFunction(); + } + + function newFunction() { + var c = class { + M() { + return value; + } + }; + } +} +// ==SCOPE::global scope== +namespace N { + + export const value = 1; + + () => { + /*RENAME*/newFunction(); + } +} +function newFunction() { + var c = class { + M() { + return N.value; + } + }; +} From 063e8a72ec3aeab881150ce31ba474d1e92272fa Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 13 Sep 2017 16:43:14 -0700 Subject: [PATCH 08/10] Merge pull request #18427 from amcasey/GH17869 Forbid extraction of empty spans (cherry picked from commit be5c00f4c9626848fe8e812aad32b86020c296bc) --- src/harness/unittests/extractMethods.ts | 6 ++++++ src/services/refactors/extractMethod.ts | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts index e7d86331a82..b0ba6143042 100644 --- a/src/harness/unittests/extractMethods.ts +++ b/src/harness/unittests/extractMethods.ts @@ -404,6 +404,12 @@ function test(x: number) { ]); testExtractRangeFailed("extract-method-not-for-token-expression-statement", `[#|a|]`, ["Select more than a single token."]); + testExtractRangeFailed("extractRangeFailed9", + `var x = ([#||]1 + 2);`, + [ + "Statement or expression expected." + ]); + testExtractMethod("extractMethod1", `namespace A { let x = 1; diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index ebc13dd2696..93f320cb261 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -149,6 +149,11 @@ namespace ts.refactor.extractMethod { // exported only for tests export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan): RangeToExtract { const length = span.length || 0; + + if (length === 0) { + return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.StatementOrExpressionExpected)] }; + } + // Walk up starting from the the start position until we find a non-SourceFile node that subsumes the selected span. // This may fail (e.g. you select two statements in the root of a source file) let start = getParentNodeInSpan(getTokenAtPosition(sourceFile, span.start, /*includeJsDocComment*/ false), sourceFile, span); From f0b78437f88b25077e4816dff83ba375d4481329 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Fri, 15 Sep 2017 13:15:31 -0700 Subject: [PATCH 09/10] Merge pull request #18508 from amcasey/ExtractSingleToken Re-enable extraction of a single token (cherry picked from commit 47b61ac18c304028036914bba1cc65220b4732f9) --- src/harness/unittests/extractMethods.ts | 2 +- src/services/refactors/extractMethod.ts | 4 ++-- ...hod-not-for-token.ts => extract-method-not-for-empty.ts} | 0 tests/cases/fourslash/extract-method13.ts | 6 +++--- tests/cases/fourslash/extract-method7.ts | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) rename tests/cases/fourslash/{extract-method-not-for-token.ts => extract-method-not-for-empty.ts} (100%) diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts index b0ba6143042..3a3dbf13bed 100644 --- a/src/harness/unittests/extractMethods.ts +++ b/src/harness/unittests/extractMethods.ts @@ -402,7 +402,7 @@ function test(x: number) { [ "Cannot extract range containing conditional break or continue statements." ]); - testExtractRangeFailed("extract-method-not-for-token-expression-statement", `[#|a|]`, ["Select more than a single token."]); + testExtractRangeFailed("extract-method-not-for-token-expression-statement", `[#|a|]`, ["Select more than a single identifier."]); testExtractRangeFailed("extractRangeFailed9", `var x = ([#||]1 + 2);`, diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index 93f320cb261..c66f6d361bc 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -92,7 +92,7 @@ namespace ts.refactor.extractMethod { export const CannotExtractRangeThatContainsWritesToReferencesLocatedOutsideOfTheTargetRangeInGenerators: DiagnosticMessage = createMessage("Cannot extract range containing writes to references located outside of the target range in generators."); export const TypeWillNotBeVisibleInTheNewScope = createMessage("Type will not visible in the new scope."); export const FunctionWillNotBeVisibleInTheNewScope = createMessage("Function will not visible in the new scope."); - export const InsufficientSelection = createMessage("Select more than a single token."); + export const InsufficientSelection = createMessage("Select more than a single identifier."); export const CannotExtractExportedEntity = createMessage("Cannot extract exported declaration"); export const CannotCombineWritesAndReturns = createMessage("Cannot combine writes and returns"); export const CannotExtractReadonlyPropertyInitializerOutsideConstructor = createMessage("Cannot move initialization of read-only class property outside of the constructor"); @@ -232,7 +232,7 @@ namespace ts.refactor.extractMethod { } function checkRootNode(node: Node): Diagnostic[] | undefined { - if (isToken(isExpressionStatement(node) ? node.expression : node)) { + if (isIdentifier(isExpressionStatement(node) ? node.expression : node)) { return [createDiagnosticForNode(node, Messages.InsufficientSelection)]; } return undefined; diff --git a/tests/cases/fourslash/extract-method-not-for-token.ts b/tests/cases/fourslash/extract-method-not-for-empty.ts similarity index 100% rename from tests/cases/fourslash/extract-method-not-for-token.ts rename to tests/cases/fourslash/extract-method-not-for-empty.ts diff --git a/tests/cases/fourslash/extract-method13.ts b/tests/cases/fourslash/extract-method13.ts index 7c4586d0eec..9bc8970b989 100644 --- a/tests/cases/fourslash/extract-method13.ts +++ b/tests/cases/fourslash/extract-method13.ts @@ -5,7 +5,7 @@ //// class C { //// static j = /*c*/1 + 1/*d*/; -//// constructor(q: string = /*a*/"a" + "b"/*b*/) { +//// constructor(q: string = /*a*/"hello"/*b*/) { //// } //// } @@ -21,7 +21,7 @@ edit.applyRefactor({ } private static newFunction(): string { - return "a" + "b"; + return "hello"; } }` }); @@ -42,7 +42,7 @@ edit.applyRefactor({ } private static newFunction(): string { - return "a" + "b"; + return "hello"; } }` }); diff --git a/tests/cases/fourslash/extract-method7.ts b/tests/cases/fourslash/extract-method7.ts index 5e1fd13ed2f..8ef8eb38d3f 100644 --- a/tests/cases/fourslash/extract-method7.ts +++ b/tests/cases/fourslash/extract-method7.ts @@ -3,7 +3,7 @@ // You cannot extract a function initializer into the function's body. // The innermost scope (scope_0) is the sibling of the function, not the function itself. -//// function fn(x = /*a*/1 + 1/*b*/) { +//// function fn(x = /*a*/3/*b*/) { //// } goTo.select('a', 'b'); @@ -15,7 +15,7 @@ edit.applyRefactor({ `function fn(x = /*RENAME*/newFunction()) { } function newFunction() { - return 1 + 1; + return 3; } ` }); From 27bede8ef1ba0aba74018696cac2c19b7823a945 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Mon, 18 Sep 2017 19:12:06 -0700 Subject: [PATCH 10/10] Stop requiring that the full range of a declaration fall within the selection Fixes #18546 (cherry picked from commit af49c60a2cb415e9481dec58285c497bd7f49fed) --- src/harness/unittests/extractMethods.ts | 5 +++++ src/services/refactors/extractMethod.ts | 2 +- .../extractMethod/extractMethod33.ts | 19 +++++++++++++++++++ .../extract-method-empty-namespace.ts | 7 ++----- 4 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 tests/baselines/reference/extractMethod/extractMethod33.ts diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts index 3a3dbf13bed..8f5abd629f3 100644 --- a/src/harness/unittests/extractMethods.ts +++ b/src/harness/unittests/extractMethods.ts @@ -700,6 +700,11 @@ function parsePrimaryExpression(): any { } }|] } +}`); + // Selection excludes leading trivia of declaration + testExtractMethod("extractMethod33", + `function F() { + [#|function G() { }|] }`); }); diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index c66f6d361bc..c34b2afe398 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -1070,7 +1070,7 @@ namespace ts.refactor.extractMethod { if (!declInFile) { return undefined; } - if (rangeContainsRange(enclosingTextRange, declInFile)) { + if (rangeContainsStartEnd(enclosingTextRange, declInFile.getStart(), declInFile.end)) { // declaration is located in range to be extracted - do nothing return undefined; } diff --git a/tests/baselines/reference/extractMethod/extractMethod33.ts b/tests/baselines/reference/extractMethod/extractMethod33.ts new file mode 100644 index 00000000000..5bb04e9df40 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod33.ts @@ -0,0 +1,19 @@ +// ==ORIGINAL== +function F() { + function G() { } +} +// ==SCOPE::function 'F'== +function F() { + /*RENAME*/newFunction(); + + function newFunction() { + function G() { } + } +} +// ==SCOPE::global scope== +function F() { + /*RENAME*/newFunction(); +} +function newFunction() { + function G() { } +} diff --git a/tests/cases/fourslash/extract-method-empty-namespace.ts b/tests/cases/fourslash/extract-method-empty-namespace.ts index 59106a6fe3d..100b2605410 100644 --- a/tests/cases/fourslash/extract-method-empty-namespace.ts +++ b/tests/cases/fourslash/extract-method-empty-namespace.ts @@ -1,8 +1,5 @@ /// -// TODO: GH#18546 -// For now this tests that at least we don't crash. - ////function f() { //// /*start*/namespace N {}/*end*/ ////} @@ -13,9 +10,9 @@ edit.applyRefactor({ actionName: "scope_1", actionDescription: "Extract function into global scope", newContent: `function f() { - /*RENAME*/newFunction(N); + /*RENAME*/newFunction(); } -function newFunction(N: any) { +function newFunction() { namespace N { } } `