[vscode-go] extension/src/diagnostics: create diagnostics static feature
0 views
Skip to first unread message
Hongxiang Jiang (Gerrit)
8:40 PM (2 hours ago) 8:40 PM
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Hongxiang Jiang has uploaded the change for review![]( "Open in Gerrit")
Commit message
extension/src/diagnostics: create diagnostics static feature
Using static feature allow us to keep the diagnostics related
functions in the same file / dir.
Move runtool function to diagnostics.
Change-Id: I75fbb182043f02e18e01fb0e95e43c01df626f15
Change diff
diff --git a/extension/src/diagnostics/diagnostics.ts b/extension/src/diagnostics/diagnostics.ts
index 5ec0590..c1f47b8 100644
--- a/extension/src/diagnostics/diagnostics.ts
+++ b/extension/src/diagnostics/diagnostics.ts
@@ -3,8 +3,15 @@
* Licensed under the MIT License. See LICENSE in the project root for license information.
*--------------------------------------------------------*/
+import cp = require('child_process');
+import path = require('path');
import vscode = require('vscode');
+import { FeatureState, HandleDiagnosticsSignature, LanguageClient, StaticFeature } from 'vscode-languageclient/node';
import { GoExtensionContext } from '../context';
+import { outputChannel } from '../goStatus';
+import { getBinPath } from '../util';
+import { fixDriveCasingInWindows } from '../utils/pathUtils';
+import { killProcessTree } from '../utils/processUtils';
export interface ICheckResult {
file: string;
@@ -135,3 +142,159 @@
const buildDiagnosticsLines = buildDiagnostics.map((x) => x.range.start.line);
return otherDiagnostics.filter((x) => buildDiagnosticsLines.indexOf(x.range.start.line) === -1);
}
+
+/**
+ * Runs given Go tool and returns errors/warnings that can be fed to the Problems Matcher
+ * @param args Arguments to be passed while running given tool
+ * @param cwd cwd that will passed in the env object while running given tool
+ * @param severity error or warning
+ * @param useStdErr If true, the stderr of the output of the given tool will be used, else stdout will be used
+ * @param toolName The name of the Go tool to run. If none is provided, the go runtime itself is used
+ * @param printUnexpectedOutput If true, then output that doesnt match expected format is printed to the output channel
+ */
+export function runTool(
+ args: string[],
+ cwd: string,
+ severity: string,
+ useStdErr: boolean,
+ toolName: string,
+ env: any,
+ printUnexpectedOutput: boolean,
+ token?: vscode.CancellationToken
+): Promise<ICheckResult[]> {
+ let cmd: string;
+ if (toolName) {
+ cmd = getBinPath(toolName);
+ } else {
+ const goRuntimePath = getBinPath('go');
+ if (!goRuntimePath) {
+ return Promise.reject(new Error('Cannot find "go" binary. Update PATH or GOROOT appropriately'));
+ }
+ cmd = goRuntimePath;
+ }
+
+ let p: cp.ChildProcess;
+ if (token) {
+ token.onCancellationRequested(() => {
+ if (p) {
+ void killProcessTree(p);
+ }
+ });
+ }
+ cwd = fixDriveCasingInWindows(cwd);
+ return new Promise((resolve, reject) => {
+ p = cp.execFile(cmd, args, { env, cwd }, (err, stdout, stderr) => {
+ try {
+ if (err && (<any>err).code === 'ENOENT') {
+ // Since the tool is run on save which can be frequent
+ // we avoid sending explicit notification if tool is missing
+ console.log(`Cannot find ${toolName ? toolName : 'go'}`);
+ return resolve([]);
+ }
+ if (err && stderr && !useStdErr) {
+ outputChannel.error(['Error while running tool:', cmd, ...args].join(' '));
+ outputChannel.error(stderr);
+ return resolve([]);
+ }
+ const lines = (useStdErr ? stderr : stdout).toString().split('\n');
+ outputChannel.info([cwd + '>Finished running tool:', cmd, ...args].join(' '));
+
+ const ret: ICheckResult[] = [];
+ let unexpectedOutput = false;
+ let atLeastSingleMatch = false;
+ for (const l of lines) {
+ if (l[0] === '\t' && ret.length > 0) {
+ ret[ret.length - 1].msg += '\n' + l;
+ continue;
+ }
+ const match = /^([^:]*: )?((.:)?[^:]*):(\d+)(:(\d+)?)?:(?:\w+:)? (.*)$/.exec(l);
+ if (!match) {
+ if (printUnexpectedOutput && useStdErr && stderr) {
+ unexpectedOutput = true;
+ }
+ continue;
+ }
+ atLeastSingleMatch = true;
+ const [, , file, , lineStr, , colStr, msg] = match;
+ const line = +lineStr;
+ const col = colStr ? +colStr : undefined;
+
+ // Building skips vendor folders,
+ // But vet and lint take in directories and not import paths, so no way to skip them
+ // So prune out the results from vendor folders here.
+ if (
+ !path.isAbsolute(file) &&
+ (file.startsWith(`vendor${path.sep}`) || file.indexOf(`${path.sep}vendor${path.sep}`) > -1)
+ ) {
+ continue;
+ }
+
+ const filePath = path.resolve(cwd, file);
+ ret.push({ file: filePath, line, col, msg, severity });
+ outputChannel.info(`${filePath}:${line}:${col ?? ''} ${msg}`);
+ }
+ if (!atLeastSingleMatch && unexpectedOutput && vscode.window.activeTextEditor) {
+ outputChannel.error(stderr);
+ if (err) {
+ ret.push({
+ file: vscode.window.activeTextEditor.document.fileName,
+ line: 1,
+ col: 1,
+ msg: stderr,
+ severity: 'error'
+ });
+ }
+ }
+ outputChannel.info('');
+ resolve(ret);
+ } catch (e) {
+ reject(e);
+ }
+ });
+ });
+}
+
+/**
+ * GoDiagnosticsFeature is a static feature that hooks into the language client's
+ * handleDiagnostics middleware to evict colliding diagnostics from lower-priority
+ * collections (build, vet, lint) whenever gopls publishes diagnostics.
+ */
+export class GoDiagnosticsFeature implements StaticFeature {
+ constructor(
+ private readonly client: LanguageClient,
+ private readonly goCtx: GoExtensionContext
+ ) {
+ this.addMiddleware();
+ }
+
+ public fillClientCapabilities(): void {}
+
+ public clear(): void {}
+
+ public getState(): FeatureState {
+ return { kind: 'static' };
+ }
+
+ public initialize(): void {}
+
+ private addMiddleware(): void {
+ const middleware = this.client.clientOptions.middleware ?? {};
+ const original = middleware.handleDiagnostics;
+
+ middleware.handleDiagnostics = (
+ uri: vscode.Uri,
+ diagnostics: vscode.Diagnostic[],
+ next: HandleDiagnosticsSignature
+ ) => {
+ const { buildDiagnosticCollection, lintDiagnosticCollection, vetDiagnosticCollection } = this.goCtx;
+ // Deduplicate diagnostics with those found by the other tools.
+ removeDuplicateDiagnostics(vetDiagnosticCollection, uri, diagnostics);
+ removeDuplicateDiagnostics(buildDiagnosticCollection, uri, diagnostics);
+ removeDuplicateDiagnostics(lintDiagnosticCollection, uri, diagnostics);
+
+ return original ? original(uri, diagnostics, next) : next(uri, diagnostics);
+ };
+
+ this.client.clientOptions.middleware = middleware;
+ }
+}
diff --git a/extension/src/diagnostics/goBuild.ts b/extension/src/diagnostics/goBuild.ts
index 34fbe81..01d9a5f 100644
--- a/extension/src/diagnostics/goBuild.ts
+++ b/extension/src/diagnostics/goBuild.ts
@@ -12,9 +12,9 @@
import { getNonVendorPackages } from '../goPackages';
import { diagnosticsStatusBarItem, outputChannel } from '../goStatus';
import { getTestFlags } from '../testUtils';
-import { getCurrentGoPath, getModuleCache, getTempFilePath, getWorkspaceFolderPath, runTool } from '../util';
+import { getCurrentGoPath, getModuleCache, getTempFilePath, getWorkspaceFolderPath } from '../util';
import { getCurrentGoWorkspaceFromGOPATH } from '../utils/pathUtils';
-import { handleErrors, ICheckResult } from './diagnostics';
+import { handleErrors, ICheckResult, runTool } from './diagnostics';
/**
* Builds current package or workspace.
diff --git a/extension/src/diagnostics/goLint.ts b/extension/src/diagnostics/goLint.ts
index f1c5e15..751885e 100644
--- a/extension/src/diagnostics/goLint.ts
+++ b/extension/src/diagnostics/goLint.ts
@@ -10,8 +10,8 @@
import { toolExecutionEnvironment } from '../goEnv';
import { diagnosticsStatusBarItem, outputChannel } from '../goStatus';
import { inspectGoToolVersion } from '../goInstallTools';
-import { getBinPath, getWorkspaceFolderPath, resolvePath, runTool } from '../util';
-import { handleErrors, ICheckResult } from './diagnostics';
+import { getBinPath, getWorkspaceFolderPath, resolvePath } from '../util';
+import { handleErrors, ICheckResult, runTool } from './diagnostics';
/**
* Runs linter on the current file, package or workspace.
diff --git a/extension/src/diagnostics/goVet.ts b/extension/src/diagnostics/goVet.ts
index 928d78a..9057c67 100644
--- a/extension/src/diagnostics/goVet.ts
+++ b/extension/src/diagnostics/goVet.ts
@@ -9,8 +9,8 @@
import { getGoConfig } from '../config';
import { toolExecutionEnvironment } from '../goEnv';
import { diagnosticsStatusBarItem, outputChannel } from '../goStatus';
-import { getGoVersion, getWorkspaceFolderPath, resolvePath, runTool } from '../util';
-import { handleErrors, ICheckResult } from './diagnostics';
+import { getGoVersion, getWorkspaceFolderPath, resolvePath } from '../util';
+import { handleErrors, ICheckResult, runTool } from './diagnostics';
/**
* Runs go vet in the current package or workspace.
diff --git a/extension/src/language/goLanguageServer.ts b/extension/src/language/goLanguageServer.ts
index 51497c7..cf00099 100644
--- a/extension/src/language/goLanguageServer.ts
+++ b/extension/src/language/goLanguageServer.ts
@@ -23,7 +23,6 @@
ExecuteCommandParams,
ExecuteCommandRequest,
ExecuteCommandSignature,
- HandleDiagnosticsSignature,
InitializeError,
InitializeResult,
LanguageClientOptions,
@@ -48,8 +47,7 @@
getCheckForToolsUpdatesConfig,
getCurrentGoPath,
getGoVersion,
- getWorkspaceFolderPath,
- removeDuplicateDiagnostics
+ getWorkspaceFolderPath
} from '../util';
import { getToolFromToolPath } from '../utils/pathUtils';
import fetch from 'node-fetch';
@@ -64,6 +62,7 @@
import { ActiveProgressTerminals, IProgressTerminal, ProgressTerminal } from '../progressTerminal';
import { createHash } from 'crypto';
import { GoExtensionContext } from '../context';
+import { GoDiagnosticsFeature } from '../diagnostics/diagnostics';
import { GoDocumentSelector } from '../goMode';
import { COMMAND as GOPLS_ADD_TEST_COMMAND } from '../goGenerateTests';
import { COMMAND as GOPLS_MODIFY_TAGS_COMMAND } from '../goModifytags';
@@ -692,19 +691,6 @@
// Otherwise, fall back to gopls.
return next(document, options, token);
},
- handleDiagnostics: (
- uri: vscode.Uri,
- diagnostics: vscode.Diagnostic[],
- next: HandleDiagnosticsSignature
- ) => {
- const { buildDiagnosticCollection, lintDiagnosticCollection, vetDiagnosticCollection } = goCtx;
- // 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,
@@ -840,6 +826,7 @@
);
c.registerFeature(new InteractiveFormsFeature(c));
c.registerFeature(new GoSemanticTokensFeature());
+ c.registerFeature(new GoDiagnosticsFeature(c, goCtx));
onDidChangeVulncheckResultEmitter.event(async (e: VulncheckEvent) => {
if (!govulncheckTerminal) {
return;
diff --git a/extension/src/util.ts b/extension/src/util.ts
index a657c21..2b812cc 100644
--- a/extension/src/util.ts
+++ b/extension/src/util.ts
@@ -23,9 +23,6 @@
getInferredGopath,
resolveHomeDir
} from './utils/pathUtils';
-import { killProcessTree } from './utils/processUtils';
-import { ICheckResult } from './diagnostics/diagnostics';
-export { ICheckResult, handleErrors, removeDuplicateDiagnostics } from './diagnostics/diagnostics';
export class GoVersion {
public sv?: semver.SemVer;
@@ -486,117 +483,6 @@
return '';
}
-/**
- * Runs given Go tool and returns errors/warnings that can be fed to the Problems Matcher
- * @param args Arguments to be passed while running given tool
- * @param cwd cwd that will passed in the env object while running given tool
- * @param severity error or warning
- * @param useStdErr If true, the stderr of the output of the given tool will be used, else stdout will be used
- * @param toolName The name of the Go tool to run. If none is provided, the go runtime itself is used
- * @param printUnexpectedOutput If true, then output that doesnt match expected format is printed to the output channel
- */
-export function runTool(
- args: string[],
- cwd: string,
- severity: string,
- useStdErr: boolean,
- toolName: string,
- env: any,
- printUnexpectedOutput: boolean,
- token?: vscode.CancellationToken
-): Promise<ICheckResult[]> {
- let cmd: string;
- if (toolName) {
- cmd = getBinPath(toolName);
- } else {
- const goRuntimePath = getBinPath('go');
- if (!goRuntimePath) {
- return Promise.reject(new Error('Cannot find "go" binary. Update PATH or GOROOT appropriately'));
- }
- cmd = goRuntimePath;
- }
-
- let p: cp.ChildProcess;
- if (token) {
- token.onCancellationRequested(() => {
- if (p) {
- void killProcessTree(p);
- }
- });
- }
- cwd = fixDriveCasingInWindows(cwd);
- return new Promise((resolve, reject) => {
- p = cp.execFile(cmd, args, { env, cwd }, (err, stdout, stderr) => {
- try {
- if (err && (<any>err).code === 'ENOENT') {
- // Since the tool is run on save which can be frequent
- // we avoid sending explicit notification if tool is missing
- console.log(`Cannot find ${toolName ? toolName : 'go'}`);
- return resolve([]);
- }
- if (err && stderr && !useStdErr) {
- outputChannel.error(['Error while running tool:', cmd, ...args].join(' '));
- outputChannel.error(stderr);
- return resolve([]);
- }
- const lines = (useStdErr ? stderr : stdout).toString().split('\n');
- outputChannel.info([cwd + '>Finished running tool:', cmd, ...args].join(' '));
-
- const ret: ICheckResult[] = [];
- let unexpectedOutput = false;
- let atLeastSingleMatch = false;
- for (const l of lines) {
- if (l[0] === '\t' && ret.length > 0) {
- ret[ret.length - 1].msg += '\n' + l;
- continue;
- }
- const match = /^([^:]*: )?((.:)?[^:]*):(\d+)(:(\d+)?)?:(?:\w+:)? (.*)$/.exec(l);
- if (!match) {
- if (printUnexpectedOutput && useStdErr && stderr) {
- unexpectedOutput = true;
- }
- continue;
- }
- atLeastSingleMatch = true;
- const [, , file, , lineStr, , colStr, msg] = match;
- const line = +lineStr;
- const col = colStr ? +colStr : undefined;
-
- // Building skips vendor folders,
- // But vet and lint take in directories and not import paths, so no way to skip them
- // So prune out the results from vendor folders here.
- if (
- !path.isAbsolute(file) &&
- (file.startsWith(`vendor${path.sep}`) || file.indexOf(`${path.sep}vendor${path.sep}`) > -1)
- ) {
- continue;
- }
-
- const filePath = path.resolve(cwd, file);
- ret.push({ file: filePath, line, col, msg, severity });
- outputChannel.info(`${filePath}:${line}:${col ?? ''} ${msg}`);
- }
- if (!atLeastSingleMatch && unexpectedOutput && vscode.window.activeTextEditor) {
- outputChannel.error(stderr);
- if (err) {
- ret.push({
- file: vscode.window.activeTextEditor.document.fileName,
- line: 1,
- col: 1,
- msg: stderr,
- severity: 'error'
- });
- }
- }
- outputChannel.info('');
- resolve(ret);
- } catch (e) {
- reject(e);
- }
- });
- });
-}
-
export function getWorkspaceFolderPath(fileUri?: vscode.Uri): string | undefined {
if (fileUri) {
const workspace = vscode.workspace.getWorkspaceFolder(fileUri);
diff --git a/extension/test/integration/extension.test.ts b/extension/test/integration/extension.test.ts
index b462919..595283a 100644
--- a/extension/test/integration/extension.test.ts
+++ b/extension/test/integration/extension.test.ts
@@ -20,7 +20,8 @@
import { buildLanguageServerConfig } from '../../src/language/goLanguageServer';
import { goPlay } from '../../src/goPlayground';
import { testCurrentFile } from '../../src/commands';
-import { getBinPath, getCurrentGoPath, getImportPath, ICheckResult } from '../../src/util';
+import { getBinPath, getCurrentGoPath, getImportPath } from '../../src/util';
+import { ICheckResult } from '../../src/diagnostics/diagnostics';
import cp = require('child_process');
import os = require('os');
import { MockExtensionContext } from '../mocks/MockContext';
diff --git a/extension/test/integration/linting.test.ts b/extension/test/integration/linting.test.ts
index f2e5ff8..25b505b 100644
--- a/extension/test/integration/linting.test.ts
+++ b/extension/test/integration/linting.test.ts
@@ -41,9 +41,9 @@
});
test('Linting - concurrent process cancelation', async () => {
- const util = require('../../src/util');
+ const diagnostics = require('../../src/diagnostics/diagnostics');
const processutil = require('../../src/utils/processUtils');
- const runToolSpy = sinon.spy(util, 'runTool');
+ const runToolSpy = sinon.spy(diagnostics, 'runTool');
const killProcessTreeSpy = sinon.spy(processutil, 'killProcessTree');
try {
@@ -79,7 +79,7 @@
test('Linting - lint errors with multiple open files', async () => {
try {
- // handleDiagnosticErrors may adjust the lint errors' ranges to make the error more visible.
+ // handleErrors may adjust the lint errors' ranges to make the error more visible.
// This adjustment applies only to the text documents known to vscode. This test checks
// the adjustment is made consistently across multiple open text documents.
const file1 = await vscode.workspace.openTextDocument(
diff --git a/extension/test/integration/utils.test.ts b/extension/test/integration/utils.test.ts
index d04ca1f..215ab0f 100644
--- a/extension/test/integration/utils.test.ts
+++ b/extension/test/integration/utils.test.ts
@@ -5,7 +5,8 @@
import assert from 'assert';
import * as vscode from 'vscode';
-import { GoVersion, removeDuplicateDiagnostics, substituteEnv } from '../../src/util';
+import { GoVersion, substituteEnv } from '../../src/util';
+import { removeDuplicateDiagnostics } from '../../src/diagnostics/diagnostics';
import path = require('path');
import { toolExecutionEnvironment } from '../../src/goEnv';
import sinon = require('sinon');
Change information
Files:
- M extension/src/diagnostics/diagnostics.ts
- M extension/src/diagnostics/goBuild.ts
- M extension/src/diagnostics/goLint.ts
- M extension/src/diagnostics/goVet.ts
- M extension/src/language/goLanguageServer.ts
- M extension/src/util.ts
- M extension/test/integration/extension.test.ts
- M extension/test/integration/linting.test.ts
- M extension/test/integration/utils.test.ts
Change size: L
Delta: 9 files changed, 179 insertions(+), 141 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hongxiang Jiang (Gerrit)
8:41 PM (2 hours ago) 8:41 PM
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Hongxiang Jiang voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages