Demote some extraction ranges to produce better results

Attempt to extract return expressions, rather than return statements, and
initializers, rather than variable declarations.

src/harness/unittests/extractFunctions.ts

@@ -362,27 +362,32 @@ function parsePrimaryExpression(): any {
 }`);
 
         testExtractFunction("extractFunction_VariableDeclaration_Var", `
-[#|var x = 1;|]
+[#|var x = 1;
+"hello"|]
 x;
 `);
 
         testExtractFunction("extractFunction_VariableDeclaration_Let_Type", `
-[#|let x: number = 1;|]
+[#|let x: number = 1;
+"hello";|]
 x;
 `);
 
         testExtractFunction("extractFunction_VariableDeclaration_Let_NoType", `
-[#|let x = 1;|]
+[#|let x = 1;
+"hello";|]
 x;
 `);
 
         testExtractFunction("extractFunction_VariableDeclaration_Const_Type", `
-[#|const x: number = 1;|]
+[#|const x: number = 1;
+"hello";|]
 x;
 `);
 
         testExtractFunction("extractFunction_VariableDeclaration_Const_NoType", `
-[#|const x = 1;|]
+[#|const x = 1;
+"hello";|]
 x;
 `);
 
@@ -404,7 +409,8 @@ x; y; z;
 `);
 
         testExtractFunction("extractFunction_VariableDeclaration_ConsumedTwice", `
-[#|const x: number = 1;|]
+[#|const x: number = 1;
+"hello";|]
 x; x;
 `);
 

src/harness/unittests/extractRanges.ts

@@ -162,6 +162,19 @@ namespace ts {
                     }|]|]
                 }
             `);
+
+            // Variable statements
+            testExtractRange(`[#|let x = [$|1|];|]`);
+            testExtractRange(`[#|let x = [$|1|], y;|]`);
+            testExtractRange(`[#|[$|let x = 1, y = 1;|]|]`);
+
+            // Variable declarations
+            testExtractRange(`let [#|x = [$|1|]|];`);
+            testExtractRange(`let [#|x = [$|1|]|], y = 2;`);
+            testExtractRange(`let x = 1, [#|y = [$|2|]|];`);
+
+            // Return statements
+            testExtractRange(`[#|return [$|1|];|]`);
         });
 
         testExtractRangeFailed("extractRangeFailed1",
@@ -340,6 +353,18 @@ switch (x) {
             refactor.extractSymbol.Messages.CannotExtractRangeContainingConditionalBreakOrContinueStatements.message
         ]);
 
+        testExtractRangeFailed("extractRangeFailed12",
+        `let [#|x|];`,
+        [
+            refactor.extractSymbol.Messages.StatementOrExpressionExpected.message
+        ]);
+
+        testExtractRangeFailed("extractRangeFailed13",
+        `[#|return;|]`,
+        [
+            refactor.extractSymbol.Messages.CannotExtractRange.message
+        ]);
+
         testExtractRangeFailed("extract-method-not-for-token-expression-statement", `[#|a|]`, [refactor.extractSymbol.Messages.CannotExtractIdentifier.message]);
     });
 }

src/services/refactors/extractSymbol.ts

@@ -245,11 +245,42 @@ namespace ts.refactor.extractSymbol {
         }
 
         // We have a single node (start)
-        const errors = checkRootNode(start) || checkNode(start);
+        let node = start;
+
+        if (isReturnStatement(start)) {
+            if (!start.expression) {
+                // Makes no sense to extract an expression-less return statement.
+                return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.CannotExtractRange)] };
+            }
+
+            node = start.expression;
+        }
+        else if (isVariableStatement(start)) {
+            let numInitializers = 0;
+            let lastInitializer: Expression | undefined = undefined;
+            for (const declaration of start.declarationList.declarations) {
+                if (declaration.initializer) {
+                    numInitializers++;
+                    lastInitializer = declaration.initializer;
+                }
+            }
+
+            if (numInitializers === 1) {
+                node = lastInitializer;
+            }
+            // No special handling if there are multiple initializers.
+        }
+        else if (isVariableDeclaration(node)) {
+            if (node.initializer) {
+                node = node.initializer;
+            }
+        }
+
+        const errors = checkRootNode(node) || checkNode(node);
         if (errors) {
             return { errors };
         }
-        return { targetRange: { range: getStatementOrExpressionRange(start), facts: rangeFacts, declarations } };
+        return { targetRange: { range: getStatementOrExpressionRange(node), facts: rangeFacts, declarations } };
 
         function checkRootNode(node: Node): Diagnostic[] | undefined {
             if (isIdentifier(isExpressionStatement(node) ? node.expression : node)) {