Merge pull request #6869 from dibarbet/fix_override_completion

Deserialize URI in override completion before attempting to open the document

Author: dibarbet

src/lsptoolshost/commands.ts

@@ -106,7 +106,8 @@ async function completionComplexEdit(
     const componentName = '[roslyn.client.completionComplexEdit]';
 
     // Find TextDocument, opening if needed.
-    const document = await vscode.workspace.openTextDocument(uriStr);
+    const uri = UriConverter.deserialize(uriStr);
+    const document = await vscode.workspace.openTextDocument(uri);
     if (document === undefined) {
         outputAndThrow(outputChannel, `${componentName} Can't open document with path: '${uriStr}'`);
     }

test/integrationTests/completion.integration.test.ts

@@ -0,0 +1,83 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as vscode from 'vscode';
+import * as path from 'path';
+import { describe, beforeAll, beforeEach, afterAll, test, expect } from '@jest/globals';
+import testAssetWorkspace from './testAssets/testAssetWorkspace';
+import { activateCSharpExtension, openFileInWorkspaceAsync } from './integrationHelpers';
+
+describe(`[${testAssetWorkspace.description}] Test Completion`, function () {
+    beforeAll(async function () {
+        await activateCSharpExtension();
+    });
+
+    beforeEach(async function () {
+        const fileName = path.join('src', 'app', 'completion.cs');
+        await openFileInWorkspaceAsync(fileName);
+    });
+
+    afterAll(async () => {
+        await testAssetWorkspace.cleanupWorkspace();
+    });
+
+    test('Returns completion items', async () => {
+        const completionList = await getCompletionsAsync(new vscode.Position(8, 12), undefined, 10);
+        expect(completionList.items.length).toBeGreaterThan(0);
+        expect(completionList.items.map((item) => item.label)).toContain('Console');
+    });
+
+    test('Resolve adds documentation', async () => {
+        const completionList = await getCompletionsAsync(new vscode.Position(8, 12), undefined, 10);
+        const documentation = completionList.items.slice(0, 10).filter((item) => item.documentation);
+        expect(documentation.length).toEqual(10);
+    });
+
+    test('Override completion is applied', async () => {
+        const completionList = await getCompletionsAsync(new vscode.Position(12, 24), ' ', 10);
+        expect(completionList.items.length).toBeGreaterThan(0);
+        const methodOverrideItem = completionList.items.find(
+            (item) => item.label === 'Method(singleCsproj2.NeedsImport n)'
+        );
+        expect(methodOverrideItem).toBeDefined();
+        expect(methodOverrideItem!.kind).toEqual(vscode.CompletionItemKind.Method);
+        expect(methodOverrideItem!.command).toBeDefined();
+        expect(methodOverrideItem!.command!.command).toEqual('roslyn.client.completionComplexEdit');
+
+        await vscode.commands.executeCommand(
+            methodOverrideItem!.command!.command,
+            methodOverrideItem!.command!.arguments![0],
+            methodOverrideItem!.command!.arguments![1],
+            methodOverrideItem!.command!.arguments![2],
+            methodOverrideItem!.command!.arguments![3]
+        );
+
+        const usingLine = vscode.window.activeTextEditor!.document.lineAt(1).text;
+        const methodOverrideLine = vscode.window.activeTextEditor!.document.lineAt(13).text;
+        const methodOverrideImplLine = vscode.window.activeTextEditor!.document.lineAt(15).text;
+        expect(usingLine).toContain('using singleCsproj2;');
+        expect(methodOverrideLine).toContain('override void Method(NeedsImport n)');
+        expect(methodOverrideImplLine).toContain('base.Method(n);');
+    });
+
+    async function getCompletionsAsync(
+        position: vscode.Position,
+        triggerCharacter: string | undefined,
+        completionsToResolve: number
+    ): Promise<vscode.CompletionList> {
+        const activeEditor = vscode.window.activeTextEditor;
+        if (!activeEditor) {
+            throw new Error('No active editor');
+        }
+
+        return await vscode.commands.executeCommand(
+            'vscode.executeCompletionItemProvider',
+            activeEditor.document.uri,
+            position,
+            triggerCharacter,
+            completionsToResolve
+        );
+    }
+});

test/integrationTests/testAssets/slnWithCsproj/src/app/completion.cs

@@ -6,9 +6,10 @@ class Completion : CompletionBase
     {
         static void shouldHaveCompletions(string[] args)
         {
+            
             Completion a = new Completion();
         }
 
-        // override  // Trailing space is intentional
+        public override // Trailing space is intentional
     }
 }

test/integrationTests/workspaceDiagnostics.integration.test.ts

@@ -36,7 +36,7 @@ describe(`[${testAssetWorkspace.description}] Test diagnostics`, function () {
                     .flatMap(([_, diagnostics]) => diagnostics);
 
                 expect(diagnosticsInDiagnosticsCs).toHaveLength(4);
-                expect(diagnosticsInCompletionCs).toHaveLength(6);
+                expect(diagnosticsInCompletionCs).toHaveLength(4);
 
                 // Compiler diagnostic in diagnostics.cs
                 expect(getCode(diagnosticsInDiagnosticsCs[2])).toBe('CS0219');
@@ -115,7 +115,7 @@ describe(`[${testAssetWorkspace.description}] Test diagnostics`, function () {
                     .flatMap(([_, diagnostics]) => diagnostics);
 
                 expect(diagnosticsInDiagnosticsCs).toHaveLength(1);
-                expect(diagnosticsInCompletionCs).toHaveLength(0);
+                expect(diagnosticsInCompletionCs).toHaveLength(1);
 
                 expect(
                     diagnosticsInDiagnosticsCs.some((d) => getCode(d).startsWith('IDE') || getCode(d).startsWith('CA'))
@@ -144,7 +144,7 @@ describe(`[${testAssetWorkspace.description}] Test diagnostics`, function () {
                     .flatMap(([_, diagnostics]) => diagnostics);
 
                 expect(diagnosticsInDiagnosticsCs).toHaveLength(1);
-                expect(diagnosticsInCompletionCs).toHaveLength(0);
+                expect(diagnosticsInCompletionCs).toHaveLength(1);
 
                 expect(
                     diagnosticsInDiagnosticsCs.some((d) => getCode(d).startsWith('IDE') || getCode(d).startsWith('CA'))