Merge pull request #6785 from dibarbet/diagnostic_integration_tests

dibarbet · web-flow · commit b3330ae5dd8d · 2024-01-17T15:54:39.000-08:00
Add diagnostics integration tests
diff --git a/src/lsptoolshost/onAutoInsert.ts b/src/lsptoolshost/onAutoInsert.ts
@@ -40,7 +40,15 @@ export function registerOnAutoInsert(languageServer: RoslynLanguageServer) {
 
             source.cancel();
             source = new vscode.CancellationTokenSource();
-            await applyAutoInsertEdit(e, changeTrimmed, languageServer, source.token);
+            try {
+                await applyAutoInsertEdit(e, changeTrimmed, languageServer, source.token);
+            } catch (e) {
+                if (e instanceof vscode.CancellationError) {
+                    return;
+                }
+
+                throw e;
+            }
         }
     });
 }
diff --git a/src/lsptoolshost/restore.ts b/src/lsptoolshost/restore.ts
@@ -41,10 +41,19 @@ async function chooseProjectAndRestore(
     languageServer: RoslynLanguageServer,
     restoreChannel: vscode.OutputChannel
 ): Promise<void> {
-    const projects = await languageServer.sendRequest0(
-        RestorableProjects.type,
-        new vscode.CancellationTokenSource().token
-    );
+    let projects: string[];
+    try {
+        projects = await languageServer.sendRequest0(
+            RestorableProjects.type,
+            new vscode.CancellationTokenSource().token
+        );
+    } catch (e) {
+        if (e instanceof vscode.CancellationError) {
+            return;
+        }
+
+        throw e;
+    }
 
     const items = projects.map((p) => {
         const projectName = path.basename(p);
diff --git a/src/lsptoolshost/roslynLanguageServer.ts b/src/lsptoolshost/roslynLanguageServer.ts
@@ -28,6 +28,7 @@ import {
     RAL,
     CancellationToken,
     RequestHandler,
+    ResponseError,
 } from 'vscode-languageclient/node';
 import { PlatformInformation } from '../shared/platform';
 import { readConfigurations } from './configurationMiddleware';
@@ -270,8 +271,12 @@ export class RoslynLanguageServer {
             throw new Error('Tried to send request while server is not started.');
         }
 
-        const response = await this._languageClient.sendRequest(type, params, token);
-        return response;
+        try {
+            const response = await this._languageClient.sendRequest(type, params, token);
+            return response;
+        } catch (e) {
+            throw this.convertServerError(type.method, e);
+        }
     }
 
     /**
@@ -285,8 +290,12 @@ export class RoslynLanguageServer {
             throw new Error('Tried to send request while server is not started.');
         }
 
-        const response = await this._languageClient.sendRequest(type, token);
-        return response;
+        try {
+            const response = await this._languageClient.sendRequest(type, token);
+            return response;
+        } catch (e) {
+            throw this.convertServerError(type.method, e);
+        }
     }
 
     public async sendRequestWithProgress<P extends PartialResultParams, R, PR, E, RO>(
@@ -302,10 +311,15 @@ export class RoslynLanguageServer {
         const disposable = this._languageClient.onProgress(type, partialResultToken, async (partialResult) => {
             await onProgress(partialResult);
         });
-        const response = await this._languageClient
-            .sendRequest(type, params, cancellationToken)
-            .finally(() => disposable.dispose());
-        return response;
+
+        try {
+            const response = await this._languageClient.sendRequest(type, params, cancellationToken);
+            return response;
+        } catch (e) {
+            throw this.convertServerError(type.method, e);
+        } finally {
+            disposable.dispose();
+        }
     }
 
     /**
@@ -368,6 +382,25 @@ export class RoslynLanguageServer {
         }
     }
 
+    private convertServerError(request: string, e: any): Error {
+        let error: Error;
+        if (e instanceof ResponseError && e.code === -32800) {
+            // Convert the LSP RequestCancelled error (code -32800) to a CancellationError so we can handle cancellation uniformly.
+            error = new vscode.CancellationError();
+        } else if (e instanceof Error) {
+            error = e;
+        } else if (typeof e === 'string') {
+            error = new Error(e);
+        } else {
+            error = new Error(`Unknown error: ${e.toString()}`);
+        }
+
+        if (!(error instanceof vscode.CancellationError)) {
+            _channel.appendLine(`Error making ${request} request: ${error.message}`);
+        }
+        return error;
+    }
+
     private async openDefaultSolutionOrProjects(): Promise<void> {
         // If Dev Kit isn't installed, then we are responsible for picking the solution to open, assuming the user hasn't explicitly
         // disabled it.
@@ -723,8 +756,19 @@ export class RoslynLanguageServer {
         // When a file is opened process any build diagnostics that may be shown
         this._languageClient.addDisposable(
             vscode.workspace.onDidOpenTextDocument(async (event) => {
-                const buildIds = await this.getBuildOnlyDiagnosticIds(CancellationToken.None);
-                this._buildDiagnosticService._onFileOpened(event, buildIds);
+                try {
+                    const buildIds = await this.getBuildOnlyDiagnosticIds(CancellationToken.None);
+                    this._buildDiagnosticService._onFileOpened(event, buildIds);
+                } catch (e) {
+                    if (e instanceof vscode.CancellationError) {
+                        // The request was cancelled (not due to us) - this means the server is no longer accepting requests
+                        // so there is nothing for us to do here.
+                        return;
+                    }
+
+                    // Let non-cancellation errors bubble up.
+                    throw e;
+                }
             })
         );
     }
diff --git a/test/integrationTests/diagnosticsHelpers.ts b/test/integrationTests/diagnosticsHelpers.ts
@@ -0,0 +1,72 @@
+/*---------------------------------------------------------------------------------------------
+ *  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 { expect } from '@jest/globals';
+import { restartLanguageServer } from './integrationHelpers';
+
+function sortDiagnostics(diagnostics: vscode.Diagnostic[]): vscode.Diagnostic[] {
+    return diagnostics.sort((a, b) => {
+        const rangeCompare = a.range.start.compareTo(b.range.start);
+        if (rangeCompare !== 0) {
+            return rangeCompare;
+        }
+
+        return getCode(a).localeCompare(getCode(b));
+    });
+}
+
+export async function waitForExpectedDiagnostics(
+    assertExpectedDiagnostics: (input: [vscode.Uri, vscode.Diagnostic[]][]) => void,
+    file?: vscode.Uri
+): Promise<void> {
+    let duration = 30 * 1000;
+    const step = 500;
+    let error: any = undefined;
+    while (duration > 0) {
+        const diagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [];
+        if (file) {
+            diagnostics.push([file, sortDiagnostics(vscode.languages.getDiagnostics(file))]);
+        } else {
+            const allDiagnostics = vscode.languages.getDiagnostics();
+            for (const [uri, uriDiagnostics] of allDiagnostics) {
+                diagnostics.push([uri, sortDiagnostics(uriDiagnostics)]);
+            }
+        }
+
+        try {
+            assertExpectedDiagnostics(diagnostics);
+            return;
+        } catch (e) {
+            error = e;
+            // Wait for a bit and try again.
+            await new Promise((r) => setTimeout(r, step));
+            duration -= step;
+        }
+    }
+
+    throw new Error(`Polling did not succeed within the alotted duration: ${error}`);
+}
+
+export async function setBackgroundAnalysisScopes(scopes: { compiler: string; analyzer: string }): Promise<void> {
+    const dotnetConfig = vscode.workspace.getConfiguration('dotnet');
+    await dotnetConfig.update('backgroundAnalysis.compilerDiagnosticsScope', scopes.compiler);
+    await dotnetConfig.update('backgroundAnalysis.analyzerDiagnosticsScope', scopes.analyzer);
+
+    // Restart the language server to ensure diagnostics are reported with the correct configuration.
+    // While in normal user scenarios it isn't necessary to restart the server to pickup diagnostic config changes,
+    // we need to do it in integration tests for two reasons:
+    //  1. We don't know when the server finishes processing the config change (its sent as a notification from client -> server).
+    //  2. Even if we processed the config change, the VSCode API does not provide a way to re-request diagnostics after the config change.
+    await restartLanguageServer();
+}
+
+export function getCode(diagnostic: vscode.Diagnostic): string {
+    const code: {
+        value: string | number;
+        target: vscode.Uri;
+    } = diagnostic.code as any;
+    expect(code).toBeDefined();
+    return code.value.toString();
+}
diff --git a/test/integrationTests/documentDiagnostics.integration.test.ts b/test/integrationTests/documentDiagnostics.integration.test.ts
@@ -0,0 +1,147 @@
+/*---------------------------------------------------------------------------------------------
+ *  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 { describe, test, beforeAll, afterAll, expect } from '@jest/globals';
+import testAssetWorkspace from './testAssets/testAssetWorkspace';
+import { AnalysisSetting } from '../../src/lsptoolshost/buildDiagnosticsService';
+import * as integrationHelpers from './integrationHelpers';
+import path = require('path');
+import { getCode, setBackgroundAnalysisScopes, waitForExpectedDiagnostics } from './diagnosticsHelpers';
+describe(`[${testAssetWorkspace.description}] Test diagnostics`, function () {
+    beforeAll(async function () {
+        await integrationHelpers.activateCSharpExtension();
+    });
+
+    afterAll(async () => {
+        await testAssetWorkspace.cleanupWorkspace();
+    });
+
+    describe('Open document diagnostics', () => {
+        let file: vscode.Uri;
+        beforeAll(async () => {
+            file = await integrationHelpers.openFileInWorkspaceAsync(path.join('src', 'app', 'diagnostics.cs'));
+        });
+
+        test('Compiler and analyzer diagnostics reported for open file when set to OpenFiles', async () => {
+            await setBackgroundAnalysisScopes({
+                compiler: AnalysisSetting.OpenFiles,
+                analyzer: AnalysisSetting.OpenFiles,
+            });
+
+            await waitForExpectedFileDiagnostics((diagnostics) => {
+                expect(diagnostics).toHaveLength(4);
+
+                expect(getCode(diagnostics[0])).toBe('IDE0005');
+                expect(diagnostics[0].message).toBe('Using directive is unnecessary.');
+                expect(diagnostics[0].range).toEqual(new vscode.Range(0, 0, 0, 16));
+                expect(diagnostics[0].severity).toBe(vscode.DiagnosticSeverity.Hint);
+
+                expect(getCode(diagnostics[1])).toBe('CA1822');
+                expect(diagnostics[1].message).toBe(
+                    "Member 'FooBarBar' does not access instance data and can be marked as static"
+                );
+                expect(diagnostics[1].range).toEqual(new vscode.Range(6, 20, 6, 29));
+                expect(diagnostics[1].severity).toBe(vscode.DiagnosticSeverity.Hint);
+
+                expect(getCode(diagnostics[2])).toBe('CS0219');
+                expect(diagnostics[2].message).toBe("The variable 'notUsed' is assigned but its value is never used");
+                expect(diagnostics[2].range).toEqual(new vscode.Range(8, 16, 8, 23));
+                expect(diagnostics[2].severity).toBe(vscode.DiagnosticSeverity.Warning);
+
+                expect(getCode(diagnostics[3])).toBe('IDE0059');
+                expect(diagnostics[3].message).toBe("Unnecessary assignment of a value to 'notUsed'");
+                expect(diagnostics[3].range).toEqual(new vscode.Range(8, 16, 8, 23));
+                expect(diagnostics[3].severity).toBe(vscode.DiagnosticSeverity.Hint);
+            }, file);
+        });
+
+        test('Compiler diagnostics reported for open file when set to FullSolution', async () => {
+            await setBackgroundAnalysisScopes({
+                compiler: AnalysisSetting.FullSolution,
+                analyzer: AnalysisSetting.OpenFiles,
+            });
+
+            await waitForExpectedFileDiagnostics((diagnostics) => {
+                expect(diagnostics).toHaveLength(4);
+
+                expect(getCode(diagnostics[2])).toBe('CS0219');
+                expect(diagnostics[2].message).toBe("The variable 'notUsed' is assigned but its value is never used");
+                expect(diagnostics[2].range).toEqual(new vscode.Range(8, 16, 8, 23));
+                expect(diagnostics[2].severity).toBe(vscode.DiagnosticSeverity.Warning);
+            }, file);
+        });
+
+        test('No compiler diagnostics reported for open file when set to None', async () => {
+            await setBackgroundAnalysisScopes({
+                compiler: AnalysisSetting.None,
+                analyzer: AnalysisSetting.OpenFiles,
+            });
+
+            await integrationHelpers.restartLanguageServer();
+
+            await waitForExpectedFileDiagnostics((diagnostics) => {
+                expect(diagnostics).toHaveLength(3);
+
+                expect(diagnostics.some((d) => getCode(d).startsWith('CS'))).toBe(false);
+            }, file);
+        });
+
+        test('Analyzer diagnostics reported for open file when set to FullSolution', async () => {
+            await setBackgroundAnalysisScopes({
+                compiler: AnalysisSetting.OpenFiles,
+                analyzer: AnalysisSetting.FullSolution,
+            });
+
+            await waitForExpectedFileDiagnostics((diagnostics) => {
+                expect(diagnostics).toHaveLength(4);
+
+                expect(getCode(diagnostics[0])).toBe('IDE0005');
+                expect(diagnostics[0].message).toBe('Using directive is unnecessary.');
+                expect(diagnostics[0].range).toEqual(new vscode.Range(0, 0, 0, 16));
+                expect(diagnostics[0].severity).toBe(vscode.DiagnosticSeverity.Hint);
+
+                expect(getCode(diagnostics[1])).toBe('CA1822');
+                expect(diagnostics[1].message).toBe(
+                    "Member 'FooBarBar' does not access instance data and can be marked as static"
+                );
+                expect(diagnostics[1].range).toEqual(new vscode.Range(6, 20, 6, 29));
+                expect(diagnostics[1].severity).toBe(vscode.DiagnosticSeverity.Hint);
+
+                expect(getCode(diagnostics[3])).toBe('IDE0059');
+                expect(diagnostics[3].message).toBe("Unnecessary assignment of a value to 'notUsed'");
+                expect(diagnostics[3].range).toEqual(new vscode.Range(8, 16, 8, 23));
+                expect(diagnostics[3].severity).toBe(vscode.DiagnosticSeverity.Hint);
+            }, file);
+        });
+
+        test('No analyzer diagnostics reported for open file when set to None', async () => {
+            await setBackgroundAnalysisScopes({
+                compiler: AnalysisSetting.OpenFiles,
+                analyzer: AnalysisSetting.None,
+            });
+
+            await waitForExpectedFileDiagnostics((diagnostics) => {
+                expect(diagnostics).toHaveLength(1);
+
+                expect(diagnostics.some((d) => getCode(d).startsWith('IDE') || getCode(d).startsWith('CA'))).toBe(
+                    false
+                );
+            }, file);
+        });
+    });
+});
+
+async function waitForExpectedFileDiagnostics(
+    assertExpectedDiagnostics: (input: vscode.Diagnostic[]) => void,
+    file: vscode.Uri
+): Promise<void> {
+    return waitForExpectedDiagnostics((diagnostics) => {
+        const fileDiagnostics = diagnostics.find((d) => d[0].toString() === file.toString());
+        expect(fileDiagnostics).toBeDefined();
+
+        return assertExpectedDiagnostics(fileDiagnostics![1]);
+    });
+}
diff --git a/test/integrationTests/workspaceDiagnostics.integration.test.ts b/test/integrationTests/workspaceDiagnostics.integration.test.ts
