Handle the combination of a write and a void return

amcasey · amcasey · commit 0e50da62c473 · 2017-09-07T14:43:02.000-07:00
When the return type is void, there's no `returnValueProperty`, but that doesn't mean we don't need a `return` at the call site. Fixes microsoft#18140.
diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts
@@ -613,6 +613,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
@@ -748,6 +748,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
@@ -0,0 +1,26 @@
+// ==ORIGINAL==
+function foo() {
+    let x = 10;
+    x++;
+    return;
+}
+// ==SCOPE::function 'foo'==
+function foo() {
+    let x = 10;
+    return newFunction();
+
+    function newFunction() {
+        x++;
+        return;
+    }
+}
+// ==SCOPE::global scope==
+function foo() {
+    let x = 10;
+    x = newFunction(x);
+    return;
+}
+function newFunction(x: number) {
+    x++;
+    return x;
+}

Original file line number	Diff line number	Diff line change
`@@ -748,6 +748,10 @@ namespace ts.refactor.extractMethod {`
`748`	`748`	`}`
`749`	`749`	`else {`
`750`	`750`	`newNodes.push(createStatement(createBinary(assignments[0].name, SyntaxKind.EqualsToken, call)));`
	`751`	`+`
	`752`	`+ if (range.facts & RangeFacts.HasReturn) {`
	`753`	`+ newNodes.push(createReturn());`
	`754`	`+ }`
`751`	`755`	`}`
`752`	`756`	`}`
`753`	`757`	`else {`