src/goLanguageServer.ts: deduplicate diagnostics from language client
The language client and the diagnostic providers in the extension may
produce duplicate diagnostics. Deduplicate diagnostics from the same line,
favoring diagnostics from the language client.
Fixes golang/vscode-go#142
Change-Id: Ibde75c576a54cc1f31492d3870c50fa908ea31b7
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/282557
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
diff --git a/src/goLanguageServer.ts b/src/goLanguageServer.ts
index 7b6798e..c977da4 100644
--- a/src/goLanguageServer.ts
+++ b/src/goLanguageServer.ts
@@ -41,7 +41,7 @@
import { GoImplementationProvider } from './goImplementations';
import { installTools, promptForMissingTool, promptForUpdatingTool } from './goInstallTools';
import { parseLiveFile } from './goLiveErrors';
-import { restartLanguageServer } from './goMain';
+import { buildDiagnosticCollection, lintDiagnosticCollection, restartLanguageServer, vetDiagnosticCollection } from './goMain';
import { GO_MODE } from './goMode';
import { GoDocumentSymbolProvider } from './goOutline';
import { GoReferenceProvider } from './goReferences';
@@ -60,7 +60,8 @@
getGoConfig,
getGoplsConfig,
getGoVersion,
- getWorkspaceFolderPath
+ getWorkspaceFolderPath,
+ removeDuplicateDiagnostics
} from './util';
import { getToolFromToolPath } from './utils/pathUtils';
@@ -80,7 +81,7 @@
// Global variables used for management of the language client.
// They are global so that the server can be easily restarted with
// new configurations.
-let languageClient: LanguageClient;
+export let languageClient: LanguageClient;
let languageServerDisposable: vscode.Disposable;
let latestConfig: LanguageServerConfig;
export let serverOutputChannel: vscode.OutputChannel;
@@ -387,6 +388,17 @@
}
}, []);
},
+ handleDiagnostics: (
+ uri: vscode.Uri,
+ diagnostics: vscode.Diagnostic[],
+ next: HandleDiagnosticsSignature
+ ) => {
+ // Deduplicate diagnostics with those found by the other tools.
+ removeDuplicateDiagnostics(vetDiagnosticCollection, uri, diagnostics);
+ removeDuplicateDiagnostics(buildDiagnosticCollection, uri, diagnostics);
+ removeDuplicateDiagnostics(lintDiagnosticCollection, uri, diagnostics);
+ return next(uri, diagnostics);
+ },
provideCompletionItem: async (
document: vscode.TextDocument,
position: vscode.Position,
diff --git a/src/util.ts b/src/util.ts
index 6211b57..896914b 100644
--- a/src/util.ts
+++ b/src/util.ts
@@ -13,6 +13,7 @@
import { NearestNeighborDict, Node } from './avlTree';
import { extensionId } from './const';
import { toolExecutionEnvironment } from './goEnv';
+import { languageClient } from './goLanguageServer';
import { buildDiagnosticCollection, lintDiagnosticCollection, vetDiagnosticCollection } from './goMain';
import { getCurrentPackage } from './goModules';
import { outputChannel } from './goStatus';
@@ -910,27 +911,42 @@
if (diagnosticCollection === buildDiagnosticCollection) {
// If there are lint/vet warnings on current file, remove the ones co-inciding with the new build errors
- if (lintDiagnosticCollection && lintDiagnosticCollection.has(fileUri)) {
- lintDiagnosticCollection.set(
- fileUri,
- deDupeDiagnostics(newDiagnostics, lintDiagnosticCollection.get(fileUri).slice())
- );
- }
-
- if (vetDiagnosticCollection && vetDiagnosticCollection.has(fileUri)) {
- vetDiagnosticCollection.set(
- fileUri,
- deDupeDiagnostics(newDiagnostics, vetDiagnosticCollection.get(fileUri).slice())
- );
- }
+ removeDuplicateDiagnostics(lintDiagnosticCollection, fileUri, newDiagnostics);
+ removeDuplicateDiagnostics(vetDiagnosticCollection, fileUri, newDiagnostics);
} else if (buildDiagnosticCollection && buildDiagnosticCollection.has(fileUri)) {
// If there are build errors on current file, ignore the new lint/vet warnings co-inciding with them
newDiagnostics = deDupeDiagnostics(buildDiagnosticCollection.get(fileUri).slice(), newDiagnostics);
}
+ // If there are errors from the language client that are on the current file, ignore the warnings co-inciding
+ // with them.
+ if (languageClient) {
+ newDiagnostics = deDupeDiagnostics(languageClient.diagnostics.get(fileUri).slice(), newDiagnostics);
+ }
diagnosticCollection.set(fileUri, newDiagnostics);
});
}
+/**
+ * Removes any diagnostics in collection, where there is a diagnostic in
+ * newDiagnostics on the same line in fileUri.
+ */
+export function removeDuplicateDiagnostics(
+ collection: vscode.DiagnosticCollection,
+ fileUri: vscode.Uri,
+ newDiagnostics: vscode.Diagnostic[]
+) {
+ if (collection && collection.has(fileUri)) {
+ collection.set(
+ fileUri,
+ deDupeDiagnostics(newDiagnostics, collection.get(fileUri).slice())
+ );
+ }
+}
+
+/**
+ * Removes any diagnostics in otherDiagnostics, where there is a diagnostic in
+ * buildDiagnostics on the same line.
+ */
function deDupeDiagnostics(
buildDiagnostics: vscode.Diagnostic[],
otherDiagnostics: vscode.Diagnostic[]
diff --git a/test/integration/utils.test.ts b/test/integration/utils.test.ts
index 5e9a8a2..e82dfd6 100644
--- a/test/integration/utils.test.ts
+++ b/test/integration/utils.test.ts
@@ -4,7 +4,9 @@
*--------------------------------------------------------*/
import * as assert from 'assert';
-import { GoVersion, guessPackageNameFromFile, substituteEnv } from '../../src/util';
+import path = require('path');
+import * as vscode from 'vscode';
+import { GoVersion, guessPackageNameFromFile, handleDiagnosticErrors, removeDuplicateDiagnostics, substituteEnv } from '../../src/util';
suite('utils Tests', () => {
test('substituteEnv: default', () => {
@@ -132,3 +134,56 @@
.then(() => done(), done);
});
});
+
+suite('Duplicate Diagnostics Tests', () => {
+ test('remove duplicate diagnostics', async () => {
+ const fixturePath = path.join(__dirname, '..', '..', '..', 'test', 'testdata');
+ const uri1 = vscode.Uri.file(path.join(fixturePath, 'linterTest', 'linter_1.go'));
+ const uri2 = vscode.Uri.file(path.join(fixturePath, 'linterTest', 'linter_2.go'));
+
+ const diagnosticCollection = vscode.languages.createDiagnosticCollection('linttest');
+
+ // Populate the diagnostic collection
+ const diag1 = [
+ new vscode.Diagnostic(new vscode.Range(1, 2, 1, 3), 'first line diagnostic', vscode.DiagnosticSeverity.Warning),
+ new vscode.Diagnostic(new vscode.Range(2, 0, 2, 3), 'second line diagnostic', vscode.DiagnosticSeverity.Warning),
+ new vscode.Diagnostic(new vscode.Range(2, 3, 2, 5), 'second line error', vscode.DiagnosticSeverity.Error),
+ new vscode.Diagnostic(new vscode.Range(4, 0, 4, 3), 'fourth line diagnostic', vscode.DiagnosticSeverity.Warning),
+ ];
+ const diag2 = [
+ new vscode.Diagnostic(new vscode.Range(1, 2, 1, 3), 'first line diagnostic', vscode.DiagnosticSeverity.Warning),
+ new vscode.Diagnostic(new vscode.Range(2, 0, 2, 3), 'second line diagnostic', vscode.DiagnosticSeverity.Warning),
+ new vscode.Diagnostic(new vscode.Range(2, 3, 2, 5), 'second line error', vscode.DiagnosticSeverity.Error),
+ new vscode.Diagnostic(new vscode.Range(4, 0, 4, 3), 'fourth line diagnostic', vscode.DiagnosticSeverity.Warning),
+ ];
+ diagnosticCollection.set(uri1, diag1);
+ diagnosticCollection.set(uri2, diag2);
+
+ // After removing diagnostics from uri1, there should only be one diagnostic remaining, and
+ // the diagnostics for uri2 should not be changed.
+ const want1 = [
+ diag1[3],
+ ];
+ const want2: vscode.Diagnostic[] = [];
+ diag2.forEach((diag) => {
+ want2.push(diag);
+ });
+
+ const newDiagnostics: vscode.Diagnostic[] = [
+ new vscode.Diagnostic(new vscode.Range(1, 2, 1, 3), 'first line diagnostic', vscode.DiagnosticSeverity.Warning),
+ new vscode.Diagnostic(new vscode.Range(2, 3, 2, 5), 'second line error', vscode.DiagnosticSeverity.Error),
+ ];
+
+ removeDuplicateDiagnostics(diagnosticCollection, uri1, newDiagnostics);
+
+ assert.strictEqual(diagnosticCollection.get(uri1).length, want1.length);
+ for (let i = 0; i < want1.length; i ++) {
+ assert.strictEqual(diagnosticCollection.get(uri1)[i], want1[i]);
+ }
+
+ assert.strictEqual(diagnosticCollection.get(uri2).length, want2.length);
+ for (let i = 0; i < want2.length; i ++) {
+ assert.strictEqual(diagnosticCollection.get(uri2)[i], want2[i]);
+ }
+ });
+});
