Merge pull request microsoft#18563 from amcasey/GH18546

amcasey · web-flow · commit cb63ea5242ec · 2017-09-19T10:47:16.000-07:00
Stop requiring that the full range of a declaration fall within the selection
diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts
@@ -768,6 +768,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
@@ -1205,7 +1205,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
@@ -0,0 +1,19 @@
+// ==ORIGINAL==
+function F() {
+    function G() { }
+}
+// ==SCOPE::inner function in function 'F'==
+function F() {
+    /*RENAME*/newFunction();
+
+    function newFunction() {
+        function G() { }
+    }
+}
+// ==SCOPE::function in 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
@@ -1,8 +1,5 @@
 /// <reference path='fourslash.ts' />
 
-// 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 to function in global scope",
     newContent: `function f() {
-    /*RENAME*/newFunction(N);
+    /*RENAME*/newFunction();
 }
-function newFunction(N: any) {
+function newFunction() {
     namespace N { }
 }
 `

Original file line number	Diff line number	Diff line change
`@@ -768,6 +768,11 @@ function parsePrimaryExpression(): any {`
`768`	`768`	`}`
`769`	`769`	`}\|]`
`770`	`770`	`}`
	`771`	+}`);
	`772`	`+ // Selection excludes leading trivia of declaration`
	`773`	`+ testExtractMethod("extractMethod33",`
	`774`	+ `function F() {
	`775`	`+ [#\|function G() { }\|]`
`771`	`776`	}`);
`772`	`777`	`});`
`773`	`778`
Original file line number	Diff line number	Diff line change
`@@ -1205,7 +1205,7 @@ namespace ts.refactor.extractMethod {`
`1205`	`1205`	`if (!declInFile) {`
`1206`	`1206`	`return undefined;`
`1207`	`1207`	`}`
`1208`		`- if (rangeContainsRange(enclosingTextRange, declInFile)) {`
	`1208`	`+ if (rangeContainsStartEnd(enclosingTextRange, declInFile.getStart(), declInFile.end)) {`
`1209`	`1209`	`// declaration is located in range to be extracted - do nothing`
`1210`	`1210`	`return undefined;`
`1211`	`1211`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,5 @@`
`1`	`1`	`/// <reference path='fourslash.ts' />`
`2`	`2`
`3`		`-// TODO: GH#18546`
`4`		`-// For now this tests that at least we don't crash.`
`5`		`-`
`6`	`3`	`////function f() {`
`7`	`4`	`//// /start/namespace N {}/end/`
`8`	`5`	`////}`
`@@ -13,9 +10,9 @@ edit.applyRefactor({`
`13`	`10`	`actionName: "scope_1",`
`14`	`11`	`actionDescription: "Extract to function in global scope",`
`15`	`12`	newContent: `function f() {
`16`		`- /RENAME/newFunction(N);`
	`13`	`+ /RENAME/newFunction();`
`17`	`14`	`}`
`18`		`-function newFunction(N: any) {`
	`15`	`+function newFunction() {`
`19`	`16`	`namespace N { }`
`20`	`17`	`}`
`21`	`18`	`