Refine extraction of expression statements

1) Replace range, rather than node, to leave trivia intact.
2) Only replace node in the innermost scope - otherwise insert as usual
and delete the original statement.

Author: amcasey

src/harness/unittests/extractConstants.ts

@@ -33,6 +33,13 @@ namespace ts {
         testExtractConstant("extractConstant_ExpressionStatementExpression",
             `[#|"hello"|];`);
 
+        testExtractConstant("extractConstant_ExpressionStatementInNestedScope", `
+let i = 0;
+function F() {
+    [#|i++|];
+}
+        `);
+
         testExtractConstant("extractConstant_BlockScopes_NoDependencies",
             `for (let i = 0; i < 10; i++) {
     for (let j = 0; j < 10; j++) {

src/services/refactors/extractSymbol.ts

@@ -911,12 +911,13 @@ namespace ts.refactor.extractSymbol {
                 const localReference = createIdentifier(localNameText);
                 changeTracker.replaceRange(context.file, { pos: node.getStart(), end: node.end }, localReference);
             }
-            else if (node.parent.kind === SyntaxKind.ExpressionStatement) {
-                // If the parent is an expression statement, replace the statement with the declaration.
+            else if (node.parent.kind === SyntaxKind.ExpressionStatement && scope === findAncestor(node, isScope)) {
+                // If the parent is an expression statement and the target scope is the immediately enclosing one,
+                // replace the statement with the declaration.
                 const newVariableStatement = createVariableStatement(
                     /*modifiers*/ undefined,
                     createVariableDeclarationList([newVariableDeclaration], NodeFlags.Const));
-                changeTracker.replaceNodeWithNodes(context.file, node.parent, [newVariableStatement], { nodeSeparator: context.newLineCharacter });
+                changeTracker.replaceRange(context.file, { pos: node.parent.getStart(), end: node.parent.end }, newVariableStatement);
             }
             else {
                 const newVariableStatement = createVariableStatement(
@@ -940,8 +941,14 @@ namespace ts.refactor.extractSymbol {
                 }
 
                 // Consume
-                const localReference = createIdentifier(localNameText);
-                changeTracker.replaceRange(context.file, { pos: node.getStart(), end: node.end }, localReference);
+                if (node.parent.kind === SyntaxKind.ExpressionStatement) {
+                    // If the parent is an expression statement, delete it.
+                    changeTracker.deleteRange(context.file, { pos: node.parent.getStart(), end: node.parent.end });
+                }
+                else {
+                    const localReference = createIdentifier(localNameText);
+                    changeTracker.replaceRange(context.file, { pos: node.getStart(), end: node.end }, localReference);
+                }
             }
         }
 

tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.js

@@ -0,0 +1,23 @@
+// ==ORIGINAL==
+
+let i = 0;
+function F() {
+    i++;
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+let i = 0;
+function F() {
+    const /*RENAME*/newLocal = i++;
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+
+let i = 0;
+const /*RENAME*/newLocal = i++;
+
+function F() {
+    
+}
+        

tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.ts

@@ -0,0 +1,23 @@
+// ==ORIGINAL==
+
+let i = 0;
+function F() {
+    i++;
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+let i = 0;
+function F() {
+    const /*RENAME*/newLocal = i++;
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+
+let i = 0;
+const /*RENAME*/newLocal = i++;
+
+function F() {
+    
+}
+