From 12403d9f70d06787f53020a878725f36f92f95fc Mon Sep 17 00:00:00 2001
From: Ryan Cavanaugh <RyanCavanaugh@users.noreply.github.com>
Date: Thu, 10 Aug 2017 13:07:42 -0700
Subject: [PATCH] Various fixes

---
 src/services/refactors/extractMethod.ts   | 26 ++++++++---------------
 src/services/textChanges.ts               |  4 +---
 tests/cases/fourslash/extract-method25.ts | 19 +++++++++++++++++
 3 files changed, 29 insertions(+), 20 deletions(-)
 create mode 100644 tests/cases/fourslash/extract-method25.ts

diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts
index 65e29138e95..0f58da4ef1d 100644
--- a/src/services/refactors/extractMethod.ts
+++ b/src/services/refactors/extractMethod.ts
@@ -235,7 +235,7 @@ namespace ts.refactor.extractMethod {
                 return { errors };
             }
 
-            // If our selection is the expression in an ExrpessionStatement, expand
+            // If our selection is the expression in an ExpressionStatement, expand
             // the selection to include the enclosing Statement (this stops us
             // from trying to care about the return value of the extracted function
             // and eliminates double semicolon insertion in certain scenarios)
@@ -458,9 +458,9 @@ namespace ts.refactor.extractMethod {
         }
     }
 
-    function isValidExtractionTarget(node: Node) {
+    function isValidExtractionTarget(node: Node): node is Scope {
         // Note that we don't use isFunctionLike because we don't want to put the extracted closure *inside* a method
-        return (node.kind === SyntaxKind.FunctionDeclaration) || (node.kind === SyntaxKind.Constructor) || isSourceFile(node) || isModuleBlock(node) || isClassLike(node);
+        return (node.kind === SyntaxKind.FunctionDeclaration) || isSourceFile(node) || isModuleBlock(node) || isClassLike(node);
     }
 
     /**
@@ -488,7 +488,7 @@ namespace ts.refactor.extractMethod {
             //  * Class declaration or expression
             //  * Module/namespace or source file
             if (current !== start && isValidExtractionTarget(current)) {
-                (scopes = scopes || []).push(current as FunctionLikeDeclaration);
+                (scopes = scopes || []).push(current);
             }
 
             // A function parameter's initializer is actually in the outer scope, not the function declaration
@@ -613,16 +613,8 @@ namespace ts.refactor.extractMethod {
         const checker = context.program.getTypeChecker();
 
         // Make a unique name for the extracted function
-        let functionNameText: __String;
-        if (isClassLike(scope)) {
-            const type = range.facts & RangeFacts.InStaticRegion ? checker.getTypeOfSymbolAtLocation(scope.symbol, scope) : checker.getDeclaredTypeOfSymbol(scope.symbol);
-            const props = checker.getPropertiesOfType(type);
-            functionNameText = getUniqueName(n => props.every(p => p.name !== n));
-        }
-        else {
-            const file = scope.getSourceFile();
-            functionNameText = getUniqueName(n => !file.identifiers.has(n as string));
-        }
+        const file = scope.getSourceFile();
+        const functionNameText: __String = getUniqueName(n => !file.identifiers.has(n as string));
         const isJS = isInJavaScriptFile(scope);
 
         const functionName = createIdentifier(functionNameText as string);
@@ -669,12 +661,12 @@ namespace ts.refactor.extractMethod {
         if (isClassLike(scope)) {
             // always create private method in TypeScript files
             const modifiers: Modifier[] = isJS ? [] : [createToken(SyntaxKind.PrivateKeyword)];
-            if (range.facts & RangeFacts.IsAsyncFunction) {
-                modifiers.push(createToken(SyntaxKind.AsyncKeyword));
-            }
             if (range.facts & RangeFacts.InStaticRegion) {
                 modifiers.push(createToken(SyntaxKind.StaticKeyword));
             }
+            if (range.facts & RangeFacts.IsAsyncFunction) {
+                modifiers.push(createToken(SyntaxKind.AsyncKeyword));
+            }
             newFunction = createMethod(
                 /*decorators*/ undefined,
                 modifiers,
diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
index 5480c8f34a5..bddfc205db4 100644
--- a/src/services/textChanges.ts
+++ b/src/services/textChanges.ts
@@ -153,13 +153,11 @@ namespace ts.textChanges {
     }
 
     export function getAdjustedEndPosition(sourceFile: SourceFile, node: Node, options: ConfigurableEnd) {
-        if (options.useNonAdjustedEndPosition) {
+        if (options.useNonAdjustedEndPosition || isExpression(node)) {
             return node.getEnd();
         }
         const end = node.getEnd();
         const newEnd = skipTrivia(sourceFile.text, end, /*stopAfterLineBreak*/ true);
-        // check if last character before newPos is linebreak
-        // if yes - considered all skipped trivia to be trailing trivia of the node
         return newEnd !== end && isLineBreak(sourceFile.text.charCodeAt(newEnd - 1))
             ? newEnd
             : end;
diff --git a/tests/cases/fourslash/extract-method25.ts b/tests/cases/fourslash/extract-method25.ts
new file mode 100644
index 00000000000..ac7e7a23004
--- /dev/null
+++ b/tests/cases/fourslash/extract-method25.ts
@@ -0,0 +1,19 @@
+/// <reference path='fourslash.ts' />
+
+// Preserve newlines correctly when semicolons aren't present
+
+//// function fn() {
+////     var q = /*a*/[0]/*b*/
+////     q[0]++
+//// }
+
+goTo.select('a', 'b')
+edit.applyRefactor('Extract Method', 'scope_0');
+verify.currentFileContentIs(`function fn() {
+    var q = newFunction()
+    q[0]++
+
+    function newFunction() {
+        return [0];
+    }
+}`);