extension/src/diagnostics: consider column upon de-duplication Based on my researches based on the last 5 minor versions of all third party linter we support, i.e. golangci-lint golangci-lint-v2, staticcheck, revive and firt party tool (i.e.t go vet, go build, gopls), all tools report diagnostics based on file + line + column. For golang/vscode-go#3511 Change-Id: I0470770918a4129591a30549d831b8db48cd771a Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/812380 Reviewed-by: Madeline Kalil <mkalil@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md index efcdd2c..40165fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md
@@ -5,9 +5,12 @@ ## Unreleased +* vscode-go now supports [v3.18](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification) of the Language Server Protocol. +* Improved diagnostic deduplication across `gopls`, build, vet, and linters ([#3511](https://github.com/golang/vscode-go/issues/3511)): + - Deduplication now checks start position (line and column), and severity level. + - Diagnostics on different columns of the same line can now coexist. * The "Go: Generate Interface Stubs" command (`go.impl.cursor`) is now powered by `gopls` by default using interactive refactoring. Place your cursor inside the target type declaration (e.g. `type Foo struct`), and `gopls` will prompt for the interface to implement and insert the method stubs after the type declaration. The previous implementation remains available via the "Go: Generate Interface Stubs (legacy)" command (`go.impl.cursor.legacy`), which inserts method stubs directly at the cursor location. -* vscode-go now supports [v3.18](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification) of the Language Server Protocol. ## v0.57.1 (prerelease)
diff --git a/extension/src/diagnostics/diagnostics.ts b/extension/src/diagnostics/diagnostics.ts index ee84980..f626db9 100644 --- a/extension/src/diagnostics/diagnostics.ts +++ b/extension/src/diagnostics/diagnostics.ts
@@ -17,16 +17,17 @@ * ------------------------------------------------------------------------------------- * ### Priority Hierarchy & Deduplication Rules * ------------------------------------------------------------------------------------- - * Diagnostics are deduplicated on a per-line basis using two symmetric rules: + * Diagnostics are deduplicated on a per-(line, start column) basis using two symmetric rules: * * 1. Upstream Filtering: - * When a lower-priority tool runs, any incoming diagnostic on a line that already - * contains a diagnostic from a higher-priority source with equal or higher severity - * is ignored. If the incoming diagnostic is strictly more severe, it is surfaced. + * When a lower-priority tool runs, any incoming diagnostic on a line and start column + * that already contains a diagnostic from a higher-priority source with equal or higher + * severity is ignored. If the incoming diagnostic is strictly more severe, it is surfaced. * * 2. Downstream Eviction: * When a higher-priority source publishes diagnostics, any existing diagnostics - * from lower-priority sources on those same lines with equal or lower severity are evicted. + * from lower-priority sources on those same line and start column positions with equal + * or lower severity are evicted. * * ------------------------------------------------------------------------------------- * ### Runtime Modes @@ -251,30 +252,33 @@ } /** - * Returns targetDiags with any diagnostics that coincide on the same line - * with a diagnostic in maskingDiags of equal or higher severity removed. + * Returns targetDiags with any diagnostics that coincide on the same line and + * start column with a diagnostic in maskingDiags of equal or higher severity + * removed. * * Diagnostics from targetDiags that are strictly more severe than all masking - * diagnostics on the same line are preserved. + * diagnostics at the same line and start column are preserved. */ -export function filterDiags( +function filterDiags( targetDiags: readonly vscode.Diagnostic[], maskingDiags: readonly vscode.Diagnostic[] ): vscode.Diagnostic[] { - // Max severity for each line number. - const maxSeverity = new Map<number, vscode.DiagnosticSeverity>(); + // Max severity for each (line, start column) position. + const maxSeverity = new Map<string, vscode.DiagnosticSeverity>(); for (const diag of maskingDiags) { - const line = diag.range.start.line; - const current = maxSeverity.get(line); + const key = `${diag.range.start.line}:${diag.range.start.character}`; + const current = maxSeverity.get(key); + // Lower numerical values represent higher severity (0 = Error, 1 = Warning, etc.). if (current === undefined || diag.severity < current) { - maxSeverity.set(line, diag.severity); + maxSeverity.set(key, diag.severity); } } const deduped: vscode.Diagnostic[] = []; for (const diag of targetDiags) { - const maxMaskingSeverity = maxSeverity.get(diag.range.start.line); + const key = `${diag.range.start.line}:${diag.range.start.character}`; + const maxMaskingSeverity = maxSeverity.get(key); if (maxMaskingSeverity === undefined || diag.severity < maxMaskingSeverity) { deduped.push(diag); }
diff --git a/extension/test/gopls/diagnostics.test.ts b/extension/test/gopls/diagnostics.test.ts index c83ee4e..423f814 100644 --- a/extension/test/gopls/diagnostics.test.ts +++ b/extension/test/gopls/diagnostics.test.ts
@@ -103,19 +103,23 @@ { line: 40, source: 'lint-test', severity: vscode.DiagnosticSeverity.Warning } ] }, - // TODO(hxjiang): update test case once dedup based on line and column { name: 'Same line, different columns, same severity', diags: { gopls: [{ file: filePath, line: 10, col: 5, msg: 'unmasked - highest priority', severity: 'error' }], build: [ { file: filePath, line: 10, col: 5, msg: 'masked by gopls', severity: 'error' }, - { file: filePath, line: 10, col: 15, msg: 'masked by gopls', severity: 'error' } + { file: filePath, line: 10, col: 15, msg: 'unmasked - no higher priority', severity: 'error' } ], - vet: [{ file: filePath, line: 10, col: 25, msg: 'masked by gopls', severity: 'error' }], - lint: [{ file: filePath, line: 10, col: 35, msg: 'masked by gopls', severity: 'error' }] + vet: [{ file: filePath, line: 10, col: 25, msg: 'unmasked - no higher priority', severity: 'error' }], + lint: [{ file: filePath, line: 10, col: 35, msg: 'unmasked - no higher priority', severity: 'error' }] }, - want: [{ line: 10, source: 'gopls-test', severity: vscode.DiagnosticSeverity.Error }] + want: [ + { line: 10, source: 'gopls-test', severity: vscode.DiagnosticSeverity.Error }, + { line: 10, source: 'build-test', severity: vscode.DiagnosticSeverity.Error }, + { line: 10, source: 'vet-test', severity: vscode.DiagnosticSeverity.Error }, + { line: 10, source: 'lint-test', severity: vscode.DiagnosticSeverity.Error } + ] }, { name: 'Same line and column, lower priority has higher severity',
diff --git a/extension/test/integration/utils.test.ts b/extension/test/integration/utils.test.ts index 907619e..139fa65 100644 --- a/extension/test/integration/utils.test.ts +++ b/extension/test/integration/utils.test.ts
@@ -4,9 +4,7 @@ *--------------------------------------------------------*/ import assert from 'assert'; -import * as vscode from 'vscode'; import { GoVersion, substituteEnv } from '../../src/util'; -import { filterDiags } from '../../src/diagnostics/diagnostics'; import { toolExecutionEnvironment } from '../../src/goEnv'; import sinon = require('sinon'); import { getGoConfig } from '../../src/config'; @@ -55,79 +53,6 @@ }); }); -suite('Diagnostic Deduplication Tests', () => { - test('filterDiags removes duplicate diagnostics on same line with equal or lower severity', async () => { - const targetDiagnostics = [ - 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 maskingDiagnostics = [ - 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) - ]; - - const result = filterDiags(targetDiagnostics, maskingDiagnostics); - - // Diagnostics on line 1 and 2 are masked; only line 4 remains. - assert.strictEqual(result.length, 1); - assert.strictEqual(result[0], targetDiagnostics[3]); - }); - - test('filterDiags preserves lower priority diagnostics if they have strictly higher severity', async () => { - const targetDiagnostics = [ - new vscode.Diagnostic( - new vscode.Range(1, 2, 1, 3), - 'first line error (lower priority, higher severity)', - vscode.DiagnosticSeverity.Error - ), - new vscode.Diagnostic( - new vscode.Range(2, 0, 2, 3), - 'second line warning (lower priority, equal severity)', - vscode.DiagnosticSeverity.Warning - ) - ]; - - const maskingDiagnostics = [ - new vscode.Diagnostic( - new vscode.Range(1, 0, 1, 5), - 'first line warning (higher priority, lower severity)', - vscode.DiagnosticSeverity.Warning - ), - new vscode.Diagnostic( - new vscode.Range(2, 0, 2, 5), - 'second line warning (higher priority, equal severity)', - vscode.DiagnosticSeverity.Warning - ) - ]; - - const result = filterDiags(targetDiagnostics, maskingDiagnostics); - - // Line 1 Error is preserved because Error (0) is more severe than Warning (1). - // Line 2 Warning is masked because Warning (1) is not more severe than Warning (1). - assert.strictEqual(result.length, 1); - assert.strictEqual(result[0], targetDiagnostics[0]); - }); -}); - suite('goEnv', () => { const config = require('../../src/config'); const sandbox = sinon.createSandbox();