Improve insertion position of extracted methods

Old: End of target scope
New: Before the first non-constructor function following the extracted
range in the target scope

Author: amcasey

src/compiler/utilities.ts

@@ -505,7 +505,7 @@ namespace ts {
         }
     }
 
-    function staticAssertNever(_: never): void {}
+    export function staticAssertNever(_: never): void {}
 
     // Gets the nearest enclosing block scope container that has the provided node
     // as a descendant, that is not the provided node.

src/services/refactors/extractMethod.ts

@@ -697,8 +697,14 @@ namespace ts.refactor.extractMethod {
         }
 
         const changeTracker = textChanges.ChangeTracker.fromContext(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
@@ -831,6 +837,39 @@ namespace ts.refactor.extractMethod {
             return "__return";
         }
 
+        function getStatementsOrClassElements(scope: Scope): ReadonlyArray<Statement> | ReadonlyArray<ClassElement> {
+            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 {
+                staticAssertNever(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 transformFunctionBody(body: Node) {
             if (isBlock(body) && !writes && substitutions.size === 0) {
                 // already block, no writes to propagate back, no substitutions - can use node as is

tests/cases/fourslash/extract-method13.ts

@@ -16,6 +16,16 @@ edit.applyRefactor({
     actionDescription: "Extract function into class 'C'",
 });
 
+verify.currentFileContentIs(`class C {
+    static j = 1 + 1;
+    constructor(q: string = C.newFunction()) {
+    }
+
+    private static newFunction(): string {
+        return "a" + "b";
+    }
+}`);
+
 goTo.select('c', 'd');
 edit.applyRefactor({
     refactorName: "Extract Method",
@@ -28,11 +38,11 @@ verify.currentFileContentIs(`class C {
     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";
+    }
 }`);