Change Extract to Function to name earlier. (#11568)

* Change Extract to Function to name earlier.

Author: sean-mcmanus

Extension/src/LanguageServer/Providers/renameProvider.ts

@@ -24,16 +24,6 @@ export class RenameProvider implements vscode.RenameProvider {
     }
 
     public async provideRenameEdits(document: vscode.TextDocument, position: vscode.Position, newName: string, _token: vscode.CancellationToken): Promise<vscode.WorkspaceEdit | undefined> {
-        // Bypass the normal rename processing during Extract to function,
-        // since we already know the locations of the required edits.
-        if (this.client.renameDataForExtractToFunction.length > 0) {
-            const workspaceEditResult: vscode.WorkspaceEdit = new vscode.WorkspaceEdit();
-            for (const renameData of this.client.renameDataForExtractToFunction) {
-                workspaceEditResult.replace(renameData.uri, renameData.range, newName);
-            }
-            this.client.renameDataForExtractToFunction = [];
-            return workspaceEditResult;
-        }
         await this.client.ready;
         workspaceReferences.cancelCurrentReferenceRequest(CancellationSender.NewRequest);
 

Extension/src/LanguageServer/client.ts

@@ -342,6 +342,7 @@ export interface CreateDeclarationOrDefinitionResult extends WorkspaceEditResult
 
 export interface ExtractToFunctionParams extends SelectionParams {
     extractAsGlobal: boolean;
+    name: string;
 }
 
 interface ShowMessageWindowParams {
@@ -3452,10 +3453,14 @@ export class DefaultClient implements Client {
             return;
         }
 
-        // TODO: Show a quick pick to get the name before generating the code.
-        // That would allow the formatting to be done without waiting for the rename.
-        // Also, it's less error prone and eliminates a class of bugs in which the
-        // rename position can be incorrect.
+        let functionName: string | undefined = await vscode.window.showInputBox({
+            title: localize('handle.extract.name', 'Name the extracted function'),
+            placeHolder: localize('handle.extract.new.function', 'NewFunction')
+        });
+
+        if (functionName === undefined || functionName === "") {
+            functionName = "NewFunction";
+        }
 
         const params: ExtractToFunctionParams = {
             uri: editor.document.uri.toString(),
@@ -3469,7 +3474,8 @@ export class DefaultClient implements Client {
                     line: editor.selection.end.line
                 }
             },
-            extractAsGlobal
+            extractAsGlobal,
+            name: functionName
         };
 
         const result: WorkspaceEditResult = await this.languageClient.sendRequest(ExtractToFunctionRequest, params);
@@ -3482,15 +3488,15 @@ export class DefaultClient implements Client {
 
         // Handle error messaging
         if (result.errorText) {
-            void vscode.window.showErrorMessage(result.errorText);
+            void vscode.window.showErrorMessage(`${localize("handle.extract.error",
+                "Extract to function failed: {0}", result.errorText)}`);
             return;
         }
 
         let workspaceEdits: vscode.WorkspaceEdit = new vscode.WorkspaceEdit();
         let replaceEditRange: vscode.Range | undefined;
         let hasProcessedReplace: boolean = false;
         const formatUriAndRanges: VsCodeUriAndRange[] = [];
-        this.renameDataForExtractToFunction = [];
         let lineOffset: number = 0;
         let headerFileLineOffset: number = 0;
         let isSourceFile: boolean = true;
@@ -3515,7 +3521,7 @@ export class DefaultClient implements Client {
                     range = new vscode.Range(new vscode.Position(range.start.line + headerFileLineOffset, range.start.character),
                         new vscode.Position(range.end.line + headerFileLineOffset, range.end.character));
                 }
-                const isReplace: boolean = !range.isEmpty;
+                const isReplace: boolean = !range.isEmpty && isSourceFile;
                 lineOffset += nextLineOffset;
                 nextLineOffset = (edit.newText.match(/\n/g) ?? []).length;
                 let rangeStartLine: number = range.start.line + lineOffset;
@@ -3559,36 +3565,32 @@ export class DefaultClient implements Client {
                     new vscode.Position(rangeStartLine + (nextLineOffset < 0 ? 0 : nextLineOffset),
                         isReplace ? range.end.character :
                             range.end.character + edit.newText.length - rangeStartCharacter))});
-                const newFunctionString: string = "NewFunction";
-
-                // Handle additional declaration lines added before the new function call.
-                let currentText: string = edit.newText.substring(rangeStartCharacter);
-                let currentTextNextLineStart: number = currentText.indexOf("\n");
-                let currentTextNewFunctionStart: number = currentText.indexOf(newFunctionString);
-                let currentTextNextLineStartUpdated: boolean = false;
-                while (currentTextNextLineStart !== -1 && currentTextNextLineStart < currentTextNewFunctionStart) {
-                    ++rangeStartLine;
-                    currentText = currentText.substring(currentTextNextLineStart + 1);
-                    currentTextNextLineStart = currentText.indexOf("\n");
-                    currentTextNewFunctionStart = currentText.indexOf(newFunctionString);
-                    currentTextNextLineStartUpdated = true;
-                }
-                rangeStartCharacter = (rangeStartCharacter === 0 && !currentTextNextLineStartUpdated ? range.start.character : 0) +
-                    currentTextNewFunctionStart;
-                if (rangeStartCharacter < 0) {
-                    // newFunctionString is missing -- unexpected error.
-                    void vscode.window.showErrorMessage(`${localize("invalid.edit",
-                        "Extract to function failed. An invalid edit was generated: '{0}'", edit.newText)}`);
-                    continue;
-                }
-                const currentEditRange: vscode.Range = new vscode.Range(
-                    new vscode.Position(rangeStartLine, rangeStartCharacter),
-                    new vscode.Position(rangeStartLine, rangeStartCharacter + newFunctionString.length));
                 if (isReplace) {
-                    replaceEditRange = currentEditRange;
+                    // Handle additional declaration lines added before the new function call.
+                    let currentText: string = edit.newText.substring(rangeStartCharacter);
+                    let currentTextNextLineStart: number = currentText.indexOf("\n");
+                    let currentTextNewFunctionStart: number = currentText.indexOf(functionName);
+                    let currentTextNextLineStartUpdated: boolean = false;
+                    while (currentTextNextLineStart !== -1 && currentTextNextLineStart < currentTextNewFunctionStart) {
+                        ++rangeStartLine;
+                        currentText = currentText.substring(currentTextNextLineStart + 1);
+                        currentTextNextLineStart = currentText.indexOf("\n");
+                        currentTextNewFunctionStart = currentText.indexOf(functionName);
+                        currentTextNextLineStartUpdated = true;
+                    }
+                    rangeStartCharacter = (rangeStartCharacter === 0 && !currentTextNextLineStartUpdated ? range.start.character : 0) +
+                        currentTextNewFunctionStart;
+                    if (rangeStartCharacter < 0) {
+                        // functionName is missing -- unexpected error.
+                        void vscode.window.showErrorMessage(`${localize("invalid.edit",
+                            "Extract to function failed. An invalid edit was generated: '{0}'", edit.newText)}`);
+                        continue;
+                    }
+                    replaceEditRange = new vscode.Range(
+                        new vscode.Position(rangeStartLine, rangeStartCharacter),
+                        new vscode.Position(rangeStartLine, rangeStartCharacter + functionName.length));
                     nextLineOffset -= range.end.line - range.start.line;
                 }
-                this.renameDataForExtractToFunction.push({ uri, range: currentEditRange });
             }
         }
 
@@ -3601,7 +3603,6 @@ export class DefaultClient implements Client {
 
         const firstUri: vscode.Uri = formatUriAndRanges[0].uri;
         await vscode.window.showTextDocument(firstUri, { selection: replaceEditRange });
-        await vscode.commands.executeCommand("editor.action.rename", firstUri, replaceEditRange.start);
 
         // Format the new text edits.
         const formatEdits: vscode.WorkspaceEdit = new vscode.WorkspaceEdit();
@@ -3645,8 +3646,6 @@ export class DefaultClient implements Client {
         }
     }
 
-    public renameDataForExtractToFunction: VsCodeUriAndRange[] = [];
-
     public onInterval(): void {
         // These events can be discarded until the language client is ready.
         // Don't queue them up with this.notifyWhenLanguageClientReady calls.