Merge pull request #6543 from beccamc/u/beccam/dedupeDiagnostics

beccamc · web-flow · commit 3802c7c79dec · 2023-11-06T14:09:43.000-06:00
Dedupe Build and Live Diagnostics
diff --git a/package.json b/package.json
@@ -51,6 +51,12 @@
         "name": "Microsoft.CodeAnalysis.LanguageClient.SolutionSnapshotProvider",
         "version": "0.1"
       }
+    },
+    {
+      "moniker": {
+        "name": "Microsoft.VisualStudio.CSharpExtension.BuildResultService",
+        "version": "0.1"
+      }
     }
   ],
   "scripts": {
diff --git a/src/lsptoolshost/buildDiagnosticsService.ts b/src/lsptoolshost/buildDiagnosticsService.ts
@@ -0,0 +1,148 @@
+/*---------------------------------------------------------------------------------------------
+ *  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 { languageServerOptions } from '../shared/options';
+
+export enum AnalysisSetting {
+    FullSolution = 'fullSolution',
+    OpenFiles = 'openFiles',
+    None = 'none',
+}
+
+export class BuildDiagnosticsService {
+    /** All the build results sent by the DevKit extension. */
+    private _allBuildDiagnostics: { [uri: string]: vscode.Diagnostic[] } = {};
+
+    /** The diagnostic results from build displayed by VS Code. When live diagnostics are available for a file, these are errors that only build knows about.
+     * When live diagnostics aren't loaded for a file, then these are all of the diagnostics reported by the build.*/
+    private _diagnosticsReportedByBuild: vscode.DiagnosticCollection;
+
+    constructor(buildDiagnostics: vscode.DiagnosticCollection) {
+        this._diagnosticsReportedByBuild = buildDiagnostics;
+    }
+
+    public clearDiagnostics() {
+        this._diagnosticsReportedByBuild.clear();
+    }
+
+    public async setBuildDiagnostics(buildDiagnostics: { [uri: string]: vscode.Diagnostic[] }, buildOnlyIds: string[]) {
+        this._allBuildDiagnostics = buildDiagnostics;
+        const displayedBuildDiagnostics = new Array<[vscode.Uri, vscode.Diagnostic[]]>();
+        const allDocuments = vscode.workspace.textDocuments;
+
+        for (const [uriPath, diagnosticList] of Object.entries(this._allBuildDiagnostics)) {
+            // Check if the document is open
+            const uri = vscode.Uri.file(uriPath);
+            const document = allDocuments.find((d) => this.compareUri(d.uri, uri));
+            const isDocumentOpen = document !== undefined ? !document.isClosed : false;
+
+            // Show the build-only diagnostics
+            displayedBuildDiagnostics.push([
+                uri,
+                BuildDiagnosticsService.filterDiagnosticsFromBuild(diagnosticList, buildOnlyIds, isDocumentOpen),
+            ]);
+        }
+
+        this._diagnosticsReportedByBuild.set(displayedBuildDiagnostics);
+    }
+
+    private compareUri(a: vscode.Uri, b: vscode.Uri): boolean {
+        return a.fsPath.localeCompare(b.fsPath) === 0;
+    }
+
+    public async _onFileOpened(document: vscode.TextDocument, buildOnlyIds: string[]) {
+        const uri = document.uri;
+        const currentFileBuildDiagnostics = this._allBuildDiagnostics[uri.fsPath];
+
+        // The document is now open in the editor and live diagnostics are being shown. Filter diagnostics
+        // reported by the build to show build-only problems.
+        if (currentFileBuildDiagnostics) {
+            const buildDiagnostics = BuildDiagnosticsService.filterDiagnosticsFromBuild(
+                currentFileBuildDiagnostics,
+                buildOnlyIds,
+                true
+            );
+            this._diagnosticsReportedByBuild.set(uri, buildDiagnostics);
+        }
+    }
+
+    public static filterDiagnosticsFromBuild(
+        diagnosticList: vscode.Diagnostic[],
+        buildOnlyIds: string[],
+        isDocumentOpen: boolean
+    ): vscode.Diagnostic[] {
+        const analyzerDiagnosticScope = languageServerOptions.analyzerDiagnosticScope as AnalysisSetting;
+        const compilerDiagnosticScope = languageServerOptions.compilerDiagnosticScope as AnalysisSetting;
+
+        // If compiler and analyzer diagnostics are set to "none", show everything reported by the build
+        if (analyzerDiagnosticScope === AnalysisSetting.None && compilerDiagnosticScope === AnalysisSetting.None) {
+            return diagnosticList;
+        }
+
+        // Filter the diagnostics reported by the build. Some may already be shown by live diagnostics.
+        const buildDiagnosticsToDisplay: vscode.Diagnostic[] = [];
+
+        // If it is a project system diagnostic (e.g. "Target framework out of support")
+        // then always show it. It cannot be reported by live.
+        const projectSystemDiagnostics = diagnosticList.filter((d) =>
+            BuildDiagnosticsService.isProjectSystemDiagnostic(d)
+        );
+        buildDiagnosticsToDisplay.push(...projectSystemDiagnostics);
+
+        // If it is a "build-only"diagnostics (i.e. it can only be found by building)
+        // then always show it. It cannot be reported by live.
+        const buildOnlyDiagnostics = diagnosticList.filter((d) =>
+            BuildDiagnosticsService.isBuildOnlyDiagnostic(buildOnlyIds, d)
+        );
+        buildDiagnosticsToDisplay.push(...buildOnlyDiagnostics);
+
+        // Check the analyzer diagnostic setting. If the setting is "none" or if the file is closed,
+        // then no live analyzers are being shown and bulid analyzers should be added.
+        // If FSA is on, then this is a no-op as FSA will report all analyzer diagnostics
+        if (
+            analyzerDiagnosticScope === AnalysisSetting.None ||
+            (analyzerDiagnosticScope === AnalysisSetting.OpenFiles && !isDocumentOpen)
+        ) {
+            const analyzerDiagnostics = diagnosticList.filter(
+                // Needs to be analyzer diagnostics and not already reported as "build only"
+                (d) => BuildDiagnosticsService.isAnalyzerDiagnostic(d) && !this.isBuildOnlyDiagnostic(buildOnlyIds, d)
+            );
+            buildDiagnosticsToDisplay.push(...analyzerDiagnostics);
+        }
+
+        // Check the compiler diagnostic setting. If the setting is "none" or if the file is closed,
+        // then no live compiler diagnostics are being shown and bulid compiler diagnostics should be added.
+        // If FSA is on, then this is a no-op as FSA will report all compiler diagnostics
+        if (
+            compilerDiagnosticScope === AnalysisSetting.None ||
+            (compilerDiagnosticScope === AnalysisSetting.OpenFiles && !isDocumentOpen)
+        ) {
+            const compilerDiagnostics = diagnosticList.filter(
+                // Needs to be analyzer diagnostics and not already reported as "build only"
+                (d) => BuildDiagnosticsService.isCompilerDiagnostic(d) && !this.isBuildOnlyDiagnostic(buildOnlyIds, d)
+            );
+            buildDiagnosticsToDisplay.push(...compilerDiagnostics);
+        }
+
+        return buildDiagnosticsToDisplay;
+    }
+
+    private static isBuildOnlyDiagnostic(buildOnlyIds: string[], d: vscode.Diagnostic): boolean {
+        return buildOnlyIds.find((b_id) => b_id === d.code) !== undefined;
+    }
+
+    private static isCompilerDiagnostic(d: vscode.Diagnostic): boolean {
+        const regex = '[cC][sS][0-9]{4}';
+        return d.code ? d.code.toString().match(regex) !== null : false;
+    }
+
+    private static isAnalyzerDiagnostic(d: vscode.Diagnostic): boolean {
+        return d.code ? !this.isCompilerDiagnostic(d) : false;
+    }
+
+    private static isProjectSystemDiagnostic(d: vscode.Diagnostic): boolean {
+        return d.code ? d.code.toString().startsWith('NETSDK') : false;
+    }
+}
diff --git a/src/lsptoolshost/roslynLanguageServer.ts b/src/lsptoolshost/roslynLanguageServer.ts
@@ -26,6 +26,7 @@ import {
     SocketMessageReader,
     MessageTransports,
     RAL,
+    CancellationToken,
 } from 'vscode-languageclient/node';
 import { PlatformInformation } from '../shared/platform';
 import { readConfigurations } from './configurationMiddleware';
@@ -57,6 +58,7 @@ import { commonOptions, languageServerOptions, omnisharpOptions } from '../share
 import { NamedPipeInformation } from './roslynProtocol';
 import { IDisposable } from '../disposable';
 import { registerRestoreCommands } from './restore';
+import { BuildDiagnosticsService } from './buildDiagnosticsService';
 
 let _channel: vscode.OutputChannel;
 let _traceChannel: vscode.OutputChannel;
@@ -95,6 +97,8 @@ export class RoslynLanguageServer {
     /** The project files previously opened; we hold onto this for the same reason as _solutionFile. */
     private _projectFiles: vscode.Uri[] = new Array<vscode.Uri>();
 
+    public _buildDiagnosticService: BuildDiagnosticsService;
+
     constructor(
         private _languageClient: RoslynLanguageClient,
         private _platformInfo: PlatformInformation,
@@ -109,6 +113,10 @@ export class RoslynLanguageServer {
         this.registerExtensionsChanged();
         this.registerTelemetryChanged();
 
+        const diagnosticsReportedByBuild = vscode.languages.createDiagnosticCollection('csharp-build');
+        this._buildDiagnosticService = new BuildDiagnosticsService(diagnosticsReportedByBuild);
+        this.registerDocumentOpenForDiagnostics();
+
         // Register Razor dynamic file info handling
         this.registerDynamicFileInfo();
 
@@ -570,7 +578,7 @@ export class RoslynLanguageServer {
             });
         });
 
-        // The server process will create the named pipe used for communcation. Wait for it to be created,
+        // The server process will create the named pipe used for communication. Wait for it to be created,
         // and listen for the server to pass back the connection information via stdout.
         const namedPipeConnectionPromise = new Promise<NamedPipeInformation>((resolve) => {
             _channel.appendLine('waiting for named pipe information from server...');
@@ -696,6 +704,16 @@ export class RoslynLanguageServer {
         );
     }
 
+    private registerDocumentOpenForDiagnostics() {
+        // 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);
+            })
+        );
+    }
+
     private registerExtensionsChanged() {
         // subscribe to extension change events so that we can get notified if C# Dev Kit is added/removed later.
         this._languageClient.addDisposable(
@@ -817,6 +835,11 @@ export class RoslynLanguageServer {
     }
 
     public async getBuildOnlyDiagnosticIds(token: vscode.CancellationToken): Promise<string[]> {
+        // If the server isn't running, no build diagnostics to get
+        if (!this.isRunning()) {
+            return [];
+        }
+
         const response = await this.sendRequest0(RoslynProtocol.BuildOnlyDiagnosticIdsRequest.type, token);
         if (response) {
             return response.ids;
diff --git a/src/lsptoolshost/services/buildResultReporterService.ts b/src/lsptoolshost/services/buildResultReporterService.ts
@@ -0,0 +1,30 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+import { RoslynLanguageServer } from '../roslynLanguageServer';
+import { CancellationToken } from 'vscode-jsonrpc';
+import * as vscode from 'vscode';
+
+interface IBuildResultDiagnostics {
+    buildStarted(cancellationToken?: CancellationToken): Promise<void>;
+    reportBuildResult(
+        buildDiagnostics: { [uri: string]: vscode.Diagnostic[] },
+        cancellationToken?: CancellationToken
+    ): Promise<void>;
+}
+
+export class BuildResultDiagnostics implements IBuildResultDiagnostics {
+    constructor(private _languageServerPromise: Promise<RoslynLanguageServer>) {}
+
+    public async buildStarted(): Promise<void> {
+        const langServer: RoslynLanguageServer = await this._languageServerPromise;
+        langServer._buildDiagnosticService.clearDiagnostics();
+    }
+
+    public async reportBuildResult(buildDiagnostics: { [uri: string]: vscode.Diagnostic[] }): Promise<void> {
+        const langServer: RoslynLanguageServer = await this._languageServerPromise;
+        const buildOnlyIds: string[] = await langServer.getBuildOnlyDiagnosticIds(CancellationToken.None);
+        langServer._buildDiagnosticService.setBuildDiagnostics(buildDiagnostics, buildOnlyIds);
+    }
+}
diff --git a/src/lsptoolshost/services/descriptors.ts b/src/lsptoolshost/services/descriptors.ts
@@ -32,4 +32,13 @@ export default class Descriptors {
             protocolMajorVersion: 3,
         }
     );
+
+    static readonly csharpExtensionBuildResultService: ServiceRpcDescriptor = new ServiceJsonRpcDescriptor(
+        ServiceMoniker.create('Microsoft.VisualStudio.CSharpExtension.BuildResultService', '0.1'),
+        Formatters.MessagePack,
+        MessageDelimiters.BigEndianInt32LengthHeader,
+        {
+            protocolMajorVersion: 3,
+        }
+    );
 }
diff --git a/src/main.ts b/src/main.ts
@@ -57,6 +57,7 @@ import { ServerStateChange } from './lsptoolshost/serverStateChange';
 import { SolutionSnapshotProvider } from './lsptoolshost/services/solutionSnapshotProvider';
 import { RazorTelemetryDownloader } from './razor/razorTelemetryDownloader';
 import { commonOptions, omnisharpOptions, razorOptions } from './shared/options';
+import { BuildResultDiagnostics } from './lsptoolshost/services/buildResultReporterService';
 import { debugSessionTracker } from './coreclrDebug/provisionalDebugSessionTracker';
 
 export async function activate(
@@ -431,6 +432,10 @@ function profferBrokeredServices(
         serviceContainer.profferServiceFactory(
             Descriptors.solutionSnapshotProviderRegistration,
             (_mk, _op, _sb) => new SolutionSnapshotProvider(languageServerPromise)
+        ),
+        serviceContainer.profferServiceFactory(
+            Descriptors.csharpExtensionBuildResultService,
+            (_mk, _op, _sb) => new BuildResultDiagnostics(languageServerPromise)
         )
     );
 }
diff --git a/src/shared/options.ts b/src/shared/options.ts
@@ -75,6 +75,8 @@ export interface LanguageServerOptions {
     readonly preferCSharpExtension: boolean;
     readonly startTimeout: number;
     readonly crashDumpPath: string | undefined;
+    readonly analyzerDiagnosticScope: string;
+    readonly compilerDiagnosticScope: string;
 }
 
 export interface RazorOptions {
@@ -389,6 +391,12 @@ class LanguageServerOptionsImpl implements LanguageServerOptions {
     public get crashDumpPath() {
         return readOption<string | undefined>('dotnet.server.crashDumpPath', undefined);
     }
+    public get analyzerDiagnosticScope() {
+        return readOption<string>('dotnet.backgroundAnalysis.analyzerDiagnosticsScope', 'openFiles');
+    }
+    public get compilerDiagnosticScope() {
+        return readOption<string>('dotnet.backgroundAnalysis.compilerDiagnosticsScope', 'openFiles');
+    }
 }
 
 class RazorOptionsImpl implements RazorOptions {
diff --git a/test/integrationTests/buildDiagnostics.integration.test.ts b/test/integrationTests/buildDiagnostics.integration.test.ts

Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,12 @@`
`51`	`51`	`"name": "Microsoft.CodeAnalysis.LanguageClient.SolutionSnapshotProvider",`
`52`	`52`	`"version": "0.1"`
`53`	`53`	`}`
	`54`	`+ },`
	`55`	`+ {`
	`56`	`+ "moniker": {`
	`57`	`+ "name": "Microsoft.VisualStudio.CSharpExtension.BuildResultService",`
	`58`	`+ "version": "0.1"`
	`59`	`+ }`
`54`	`60`	`}`
`55`	`61`	`],`
`56`	`62`	`"scripts": {`
Original file line number	Diff line number	Diff line change
`@@ -32,4 +32,13 @@ export default class Descriptors {`
`32`	`32`	`protocolMajorVersion: 3,`
`33`	`33`	`}`
`34`	`34`	`);`
	`35`	`+`
	`36`	`+ static readonly csharpExtensionBuildResultService: ServiceRpcDescriptor = new ServiceJsonRpcDescriptor(`
	`37`	`+ ServiceMoniker.create('Microsoft.VisualStudio.CSharpExtension.BuildResultService', '0.1'),`
	`38`	`+ Formatters.MessagePack,`
	`39`	`+ MessageDelimiters.BigEndianInt32LengthHeader,`
	`40`	`+ {`
	`41`	`+ protocolMajorVersion: 3,`
	`42`	`+ }`
	`43`	`+ );`
`35`	`44`	`}`
Original file line number	Diff line number	Diff line change
`@@ -75,6 +75,8 @@ export interface LanguageServerOptions {`
`75`	`75`	`readonly preferCSharpExtension: boolean;`
`76`	`76`	`readonly startTimeout: number;`
`77`	`77`	`readonly crashDumpPath: string \| undefined;`
	`78`	`+ readonly analyzerDiagnosticScope: string;`
	`79`	`+ readonly compilerDiagnosticScope: string;`
`78`	`80`	`}`
`79`	`81`
`80`	`82`	`export interface RazorOptions {`
`@@ -389,6 +391,12 @@ class LanguageServerOptionsImpl implements LanguageServerOptions {`
`389`	`391`	`public get crashDumpPath() {`
`390`	`392`	`return readOption<string \| undefined>('dotnet.server.crashDumpPath', undefined);`
`391`	`393`	`}`
	`394`	`+ public get analyzerDiagnosticScope() {`
	`395`	`+ return readOption<string>('dotnet.backgroundAnalysis.analyzerDiagnosticsScope', 'openFiles');`
	`396`	`+ }`
	`397`	`+ public get compilerDiagnosticScope() {`
	`398`	`+ return readOption<string>('dotnet.backgroundAnalysis.compilerDiagnosticsScope', 'openFiles');`
	`399`	`+ }`
`392`	`400`	`}`
`393`	`401`
`394`	`402`	`class RazorOptionsImpl implements RazorOptions {`