Merged PR 474124: Fix complex edit completion in Razor

In Razor, we use the vscode command `vscode.executeCompletionItemProvider` to retrieve completions. This works for items that don't need another call for resolution. Unfortunately, Roslyn uses resolution to lazily compute text edits instead of providing them for the completion list. This presents two problems:

1. The data field is where Roslyn puts it's `ResultId` in order to retrieve information on the resolution call. This doesn't exist in the vscode CompletionItem type and was being removed on calling the command.
2. resolveCompletionItem was never being called.

This hooks everything up to call Roslyn for both `provideCompletionItems` and `resolveCompletionItem` when applicable.

Related work items: #1824087

Author: Andrew Hall (METAL)

src/lsptoolshost/commands.ts

@@ -51,6 +51,10 @@ async function restartServer(languageServer: RoslynLanguageServer): Promise<void
 /**
  * Callback after a completion item with complex edit is committed. The change needs to be made outside completion resolve
  * handling
+ * 
+ * IMPORTANT: @see RazorCompletionItemProvider.resolveCompletionItem matches the arguments for this commands
+ * so it can remap correctly in razor files. Any updates to this function signature requires updates there as well.
+ * 
  * @param uriStr The uri containing the location of the document where the completion item was committed in.
  * @param textEdits The additional complex edit for the committed completion item.
  * @param isSnippetString Indicates if the TextEdit contains a snippet string.

src/lsptoolshost/roslynLanguageServer.ts

@@ -33,6 +33,10 @@ import {
     CodeActionParams,
     CodeActionRequest,
     CodeActionResolveRequest,
+    CompletionParams,
+    CompletionRequest,
+    CompletionResolveRequest,
+    CompletionItem,
 } from 'vscode-languageclient/node';
 import { PlatformInformation } from '../shared/platform';
 import { acquireDotNetProcessDependencies } from './dotnetRuntime';
@@ -67,6 +71,8 @@ export class RoslynLanguageServer {
     public static readonly roslynPullDiagnosticCommand: string = 'roslyn.pullDiagnosticRazorCSharp';
     public static readonly provideCodeActionsCommand: string = 'roslyn.provideCodeActions';
     public static readonly resolveCodeActionCommand: string = 'roslyn.resolveCodeAction';
+    public static readonly provideCompletionsCommand: string = 'roslyn.provideCompletions';
+    public static readonly resolveCompletionsCommand: string = 'roslyn.resolveCompletion';
     public static readonly razorInitializeCommand: string = 'razor.initialize';
 
     // These are notifications we will get from the LSP server and will forward to the Razor extension.
@@ -476,6 +482,13 @@ export class RoslynLanguageServer {
             return await this.sendRequest(CodeActionResolveRequest.type, request, CancellationToken.None);
         });
 
+        vscode.commands.registerCommand(RoslynLanguageServer.provideCompletionsCommand, async (request: CompletionParams) => {
+            return await this.sendRequest(CompletionRequest.type, request, CancellationToken.None);
+        });
+        vscode.commands.registerCommand(RoslynLanguageServer.resolveCompletionsCommand, async (request: CompletionItem) => {
+            return await this.sendRequest(CompletionResolveRequest.type, request, CancellationToken.None);
+        });
+
         // Roslyn is responsible for producing a json file containing information for Razor, that comes from the compilation for
         // a project. We want to defer this work until necessary, so this command is called by the Razor document manager to tell
         // us when they need us to initialize the Razor things.

src/razor/src/Completion/ProvisionalCompletionOrchestrator.ts

@@ -129,7 +129,8 @@ export class ProvisionalCompletionOrchestrator {
             projectedDocument.uri,
             htmlPosition,
             provisionalPosition,
-            completionContext.triggerCharacter);
+            completionContext,
+            projection.languageKind);
 
         // We track when we add provisional dots to avoid doing unnecessary work on commonly invoked events.
         this.provisionalDotsMayBeActive = true;

src/razor/src/Completion/RazorCompletionItemProvider.ts

@@ -11,27 +11,60 @@ import { RazorLanguageServiceClient } from '../RazorLanguageServiceClient';
 import { RazorLogger } from '../RazorLogger';
 import { getUriPath } from '../UriPaths';
 import { ProvisionalCompletionOrchestrator } from './ProvisionalCompletionOrchestrator';
+import { LanguageKind } from '../RPC/LanguageKind';
+import { RoslynLanguageServer } from '../../../lsptoolshost/roslynLanguageServer';
+import { CompletionItem, CompletionParams, CompletionTriggerKind } from 'vscode-languageclient';
+import { UriConverter } from '../../../lsptoolshost/uriConverter';
+import * as RazorConventions from '../RazorConventions';
+import { MappingHelpers } from '../Mapping/MappingHelpers';
 
 export class RazorCompletionItemProvider
     extends RazorLanguageFeatureBase
     implements vscode.CompletionItemProvider {
 
     public static async getCompletions(
         projectedUri: vscode.Uri, hostDocumentPosition: vscode.Position,
-        projectedPosition: vscode.Position, triggerCharacter: string | undefined) {
+        projectedPosition: vscode.Position, context: vscode.CompletionContext,
+        language: LanguageKind) {
 
         if (projectedUri) {
             // "@" is not a valid trigger character for C# / HTML and therefore we need to translate
             // it into a non-trigger invocation.
-            const modifiedTriggerCharacter = triggerCharacter === '@' ? undefined : triggerCharacter;
-
-            const completions = await vscode
-                .commands
-                .executeCommand<vscode.CompletionList | vscode.CompletionItem[]>(
-                    'vscode.executeCompletionItemProvider',
-                    projectedUri,
-                    projectedPosition,
-                    modifiedTriggerCharacter);
+            const modifiedTriggerCharacter = context.triggerCharacter === '@' ? undefined : context.triggerCharacter;
+
+            let completions: vscode.CompletionList | vscode.CompletionItem[];
+
+            // For CSharp, completions need to keep the "data" field
+            // on the completion item for lazily resolving the edits in
+            // the resolveCompletionItem step. Using the vs code command
+            // drops that field because it doesn't exist in the declared vs code
+            // CompletionItem type.
+            if (language === LanguageKind.CSharp) {
+                const params: CompletionParams = {
+                    context: {
+                        triggerKind: getTriggerKind(context.triggerKind),
+                        triggerCharacter: modifiedTriggerCharacter
+                    },
+                    textDocument: {
+                        uri: UriConverter.serialize(projectedUri),
+                    },
+                    position: projectedPosition
+                };
+
+                completions = await vscode
+                    .commands
+                    .executeCommand<vscode.CompletionList | vscode.CompletionItem[]>(
+                        RoslynLanguageServer.provideCompletionsCommand,
+                        params);
+            } else {
+                completions = await vscode
+                    .commands
+                    .executeCommand<vscode.CompletionList | vscode.CompletionItem[]>(
+                        'vscode.executeCompletionItemProvider',
+                        projectedUri,
+                        projectedPosition,
+                        modifiedTriggerCharacter);
+            }
 
             const completionItems =
                 completions instanceof Array ? completions  // was vscode.CompletionItem[]
@@ -68,7 +101,7 @@ export class RazorCompletionItemProvider
                         (range as any).replacing = new vscode.Range(replacingRangeStart, replacingRangeEnd);
                     }
 
-                    if (range instanceof vscode.Range &&  range.start && range.end) {
+                    if (range instanceof vscode.Range && range.start && range.end) {
                         const rangeStart = this.offsetColumn(completionCharacterOffset, hostDocumentPosition.line, range.start);
                         const rangeEnd = this.offsetColumn(completionCharacterOffset, hostDocumentPosition.line, range.end);
                         completionItem.range = new vscode.Range(rangeStart, rangeEnd);
@@ -78,7 +111,7 @@ export class RazorCompletionItemProvider
                 // textEdit is deprecated in favor of .range. Clear out its value to avoid any unexpected behavior.
                 completionItem.textEdit = undefined;
 
-                if (triggerCharacter === '@' &&
+                if (context.triggerCharacter === '@' &&
                     completionItem.commitCharacters) {
                     // We remove `{`, '(', and '*' from the commit characters to prevent auto-completing the first
                     // completion item with a curly brace when a user intended to type `@{}` or `@()`.
@@ -138,7 +171,61 @@ export class RazorCompletionItemProvider
             projection.uri,
             position,
             projection.position,
-            context.triggerCharacter);
+            context,
+            projection.languageKind);
+
         return completionList;
     }
+
+    public async resolveCompletionItem(item: vscode.CompletionItem, token: vscode.CancellationToken): Promise<vscode.CompletionItem> {
+        // We assume that only the RoslynLanguageServer provides data, which
+        // if it does we use LSP calls directly to Roslyn since there's no
+        // equivalent vscode command to generically do that.
+        if ((<CompletionItem>item).data) {
+            let newItem = await vscode
+                .commands
+                .executeCommand<vscode.CompletionItem>(
+                    RoslynLanguageServer.resolveCompletionsCommand,
+                    item);
+
+            if (!newItem) {
+                return item;
+            }
+
+            item = newItem;
+
+            if (item.command && item.command.arguments?.length === 4) {
+                let uri = vscode.Uri.parse(item.command.arguments[0]);
+
+                if (uri && RazorConventions.isRazorCSharpFile(uri)) {
+                    let razorUri = RazorConventions.getRazorDocumentUri(uri);
+                    let textEdit = item.command.arguments[1] as vscode.TextEdit;
+
+                    let remappedEdit = await MappingHelpers.remapGeneratedFileTextEdit(razorUri, textEdit, this.serviceClient, this.logger, token);
+
+                    if (remappedEdit) {
+                        item.command.arguments[0] = razorUri;
+                        item.command.arguments[1] = remappedEdit;
+                    }
+                }
+            }
+        }
+
+        return item;
+    }
+}
+
+function getTriggerKind(triggerKind: vscode.CompletionTriggerKind): CompletionTriggerKind {
+    switch (triggerKind) {
+        case vscode.CompletionTriggerKind.Invoke:
+            return CompletionTriggerKind.Invoked;
+        case vscode.CompletionTriggerKind.TriggerCharacter:
+            return CompletionTriggerKind.TriggerCharacter;
+        case vscode.CompletionTriggerKind.TriggerForIncompleteCompletions:
+            return CompletionTriggerKind.TriggerForIncompleteCompletions;
+        default:
+            throw new Error(`Unexpected completion trigger kind: ${triggerKind}`);
+
+    }
 }
+

src/razor/src/Mapping/MappingHelpers.ts

@@ -35,36 +35,55 @@ export class MappingHelpers {
 
             // Re-map each edit to its position in the original Razor document.
             for (const edit of edits) {
-                const remappedResponse = await serviceClient.mapToDocumentRanges(
-                    LanguageKind.CSharp,
-                    [edit.range],
-                    documentUri);
-
-                if (!remappedResponse ||
-                    !remappedResponse.ranges ||
-                    !remappedResponse.ranges[0]) {
-                    // This is kind of wrong. Workspace edits commonly consist of a bunch of different edits which
-                    // don't make sense individually. If we try to introspect on them individually there won't be
-                    // enough context to do anything intelligent. But we also need to know if the edit can just
-                    // be handled by mapToDocumentRange (something went wrong here), so we ignore the edit.
-                    logger.logWarning(`Unable to remap file ${uri.path} at ${edit.range}.`);
-                    continue;
-                } else {
-                    const remappedEdit = new vscode.TextEdit(remappedResponse.ranges[0], edit.newText);
-
-                    logger.logVerbose(
-                        `Re-mapping text ${edit.newText} at ${edit.range} in ${uri.path} to ${remappedResponse.ranges[0]} in ${documentUri.path}`);
+                let remappedEdit = await MappingHelpers.remapGeneratedFileTextEdit(documentUri, edit, serviceClient, logger, token);
 
+                if (remappedEdit) {
                     this.addElementToDictionary(map, documentUri, remappedEdit);
                 }
-
             }
         }
 
         const result = this.mapToTextEdit(map);
         return result;
     }
 
+    /**
+     * Try to map a textedit from a generated document onto the razor file
+     * @param uri The document uri, expected to be the generated document
+     * @param textEdit The edit to map from a generated document
+     * @returns A mapped edit, or undefined if the edit couldn't be mapped (or is in a razor file)
+     */
+    public static async remapGeneratedFileTextEdit(
+        uri: vscode.Uri,
+        textEdit: vscode.TextEdit,
+        serviceClient: RazorLanguageServiceClient,
+        logger: RazorLogger,
+        token: vscode.CancellationToken): Promise<vscode.TextEdit | undefined> {
+
+        const remappedResponse = await serviceClient.mapToDocumentRanges(
+            LanguageKind.CSharp,
+            [textEdit.range],
+            uri);
+
+        if (!remappedResponse ||
+            !remappedResponse.ranges ||
+            !remappedResponse.ranges[0]) {
+            // This is kind of wrong. Workspace edits commonly consist of a bunch of different edits which
+            // don't make sense individually. If we try to introspect on them individually there won't be
+            // enough context to do anything intelligent. But we also need to know if the edit can just
+            // be handled by mapToDocumentRange (something went wrong here), so we ignore the edit.
+            logger.logWarning(`Unable to remap file ${uri.path} at ${textEdit.range}.`);
+            return undefined;
+        } else {
+            const remappedEdit = new vscode.TextEdit(remappedResponse.ranges[0], textEdit.newText);
+
+            logger.logVerbose(
+                `Re-mapping text ${textEdit.newText} at ${textEdit.range} in ${uri.path} to ${remappedResponse.ranges[0]} in ${uri.path}`);
+
+            return remappedEdit;
+        }
+    }
+
     public static async remapGeneratedFileLocation(
         location: vscode.Location,
         serviceClient: RazorLanguageServiceClient,