From e2d94a2922a605fa14f6ae3864595d084d362540 Mon Sep 17 00:00:00 2001
From: Andrew Casey <andrew.casey@microsoft.com>
Date: Wed, 13 Sep 2017 12:50:41 -0700
Subject: [PATCH] Only introduce return properties at the top level

...not in nested functions.
---
 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 86ee5354a5c..956ceb78e02 100644
--- a/src/harness/unittests/extractMethods.ts
+++ b/src/harness/unittests/extractMethods.ts
@@ -733,6 +733,33 @@ function parsePrimaryExpression(): any {
         testExtractMethod("extractMethod30",
             `function F<T>() {
     [#|let t: T;|]
+}`);
+        // 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 eed49524656..2410ce8cee5 100644
--- a/src/services/refactors/extractMethod.ts
+++ b/src/services/refactors/extractMethod.ts
@@ -855,6 +855,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(<Expression>body)]);
         // rewrite body if either there are writes that should be propagated back via return statements or there are substitutions
         if (writes || substitutions.size) {
@@ -877,7 +878,7 @@ namespace ts.refactor.extractMethod {
         }
 
         function visitor(node: Node): VisitResult<Node> {
-            if (node.kind === SyntaxKind.ReturnStatement && writes) {
+            if (!ignoreReturns && node.kind === SyntaxKind.ReturnStatement && writes) {
                 const assignments: ObjectLiteralElementLike[] = getPropertyAssignmentsForWrites(writes);
                 if ((<ReturnStatement>node).expression) {
                     if (!returnValueProperty) {
@@ -893,8 +894,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..754814dcc26
--- /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::function in 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::function in 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..b9b870a08fa
--- /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::function in namespace 'N'==
+namespace N {
+
+    export const value = 1;
+
+    () => {
+        /*RENAME*/newFunction();
+    }
+
+    function newFunction() {
+        var c = class {
+            M() {
+                return value;
+            }
+        };
+    }
+}
+// ==SCOPE::function in global scope==
+namespace N {
+
+    export const value = 1;
+
+    () => {
+        /*RENAME*/newFunction();
+    }
+}
+function newFunction() {
+    var c = class {
+        M() {
+            return N.value;
+        }
+    };
+}